[vlc-devel] [vlc-commits] auhal: implement drain
david.fuhrmann at gmail.com
Tue Mar 26 19:06:12 CET 2013
Am 24.03.2013 um 22:04 schrieb Rémi Denis-Courmont <remi at remlab.net>:
> Le dimanche 24 mars 2013 23:00:14, David Fuhrmann a écrit :
>> Am 24.03.2013 um 21:56 schrieb "Rémi Denis-Courmont" <remi at remlab.net>:
>>> Le dimanche 24 mars 2013 22:49:46, David Fuhrmann a écrit :
>>>> @@ -1287,6 +1299,10 @@ static OSStatus RenderCallbackAnalog(vlc_object_t
>>>> *p_obj, VLC_UNUSED(inNumberFrames);
>>>> + vlc_mutex_lock(&p_sys->lock);
>>>> + vlc_cond_signal(&p_sys->cond);
>>>> + vlc_mutex_unlock(&p_sys->lock);
>>> Please tell me, where the hell did you copy this (wrong) code from? Why
>>> does everybody developer make the same mistake in turn, inspite of the
>>> documentation and so many negative review comments??
>> I saw this in at least 2 or 3 audio output plugins.
> KAI is the only other audio output plugin calling vlc_cond_signal(), and does
> so correctly and quite differently. Well, WaveOut uses vlc_cond_broadcast() but
> also correctly and not like this patch.
If I parsed the relevant parts of your emails in the right way, I assume that you want to complain that I did not extended the scope of the mutex lock to the manipulation of the buffer.
Please note that I already thought in doing so, before committing.
But the CircularBuffer already has some thread safety mechanisms using atomic operations. For this scenario I assumed that these will suffice for proper thread synchronization.
Please take a closer look of the implementation of the circular buffer, and give some concrete remarks why you believe that current mechanisms are not sufficient.
I am always happy when I learn something due to remarks from you (or from others).
With best regards,
More information about the vlc-devel