Rockbox.org home
release
dev builds
extras
themes manual
wiki
device status forums
mailing lists
IRC bugs
patches
dev guide



Rockbox mail archive

Subject: Re: Performance regression

Re: Performance regression

From: Thomas Jarosch <tomj_at_simonv.com>
Date: Sat, 3 Dec 2011 11:36:29 +0100

Am Donnerstag, 1. Dezember 2011, 22:31:25 schrieb Michael Sevakis:
> > Actually I did a test build back then (still floating around on my phone)
> > and everything was working fine. I didn't notice the performance drop.
> > Maemo doesn't need pcm_play_lock() as the locking is done
> > on the gstreamer object level.
>
> It for locking out the callback for the rest of the system, like the mixer,
> pcmbuffer, etc. when it would not be desireable for callback to happen.
> With the higher callback frequency, it's much more likely to happen when
> other code in the system expects an atomic operation. I don't see how the
> current code performs the intended function other than for its own
> internal integrity. The flag setting that I remember seeing is racy too.

You probably remember the "inside_feed_data" flag. That's
in there for a different reason: We mustn't call any gstreamer
function while we are inside a gstreamer callback.
gstreamer's internal lock is not recursive so it would deadlock.
Probably the flag needs to use atomic operations, too. I'll revisit it.

I think I understood the pcm_play_lock() mechanism now
and will change the maemo 5 implementation. Thanks.

Speaking of racy, this code in pcm-sdl.c / pcm-android.c
doesn't look very safe:
------------------------------------
void pcm_play_lock(void)
{
    if (++audio_locked == 1)
        SDL_LockMutex(audio_lock);
}
------------------------------------

We should use gcc atomic operations for "audio_locked". The SDL_LockMutex()
will issue a full memory barrier but if two pcm_play_lock() functions are
called at the same time, this will fail badly.

Thomas
Received on 2011-12-03


Page was last modified "Jan 10 2012" The Rockbox Crew
aaa