|
Rockbox mail archiveSubject: Re: Performance regressionRe: 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 template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy |