[vlc-devel] [vlc-commits] auhal: implement drain

[David Fuhrmann]

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.

Hi Remi,

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,
David