[vlc-devel] [PATCH] decoder: read the old p_aout in the lock where we reset it

[Steve Lhomme]

On 2019-09-03 14:38, Thomas Guillem wrote:
> 
> On Tue, Sep 3, 2019, at 14:34, Rémi Denis-Courmont wrote:
>> Hi,
>>
>> AFAIK, it is, or at least was at some point, safe to read outside the lock because no other thread could write it.
> 
> If it safe to read outside the lock from the output thread (vout/aout callbacks). Cf my patch documentation.

The 2 functions from this patch are called each from a different thread.

ReloadDecoder() is called from the DecoderThread. But decoder_Clean() is 
called before so there is 0 chance that the unloaded decoder will 
create/modify a new one.

It should be safe to read from the aout_update_format() call. But if 
that's the case it should also be safe to set it to NULL without a lock.

>>
>> It's silly to maximize lock scope. Lock scope should be minimised.
>>
>> Le 3 septembre 2019 11:29:28 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>>> It's already done this way for p_vout. Since we're going to lock for writing,
>>> we might as well read the value in that lock. src/input/decoder.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>>> index 63f858c397..25aa1e0727 100644
>>> --- a/src/input/decoder.c
>>> +++ b/src/input/decoder.c
>>> @@ -220,9 +220,9 @@ static int ReloadDecoder( struct decoder_owner *p_owner, bool b_packetizer,
>>>       if( reload == RELOAD_DECODER_AOUT )
>>>       {
>>>           assert( p_owner->fmt.i_cat == AUDIO_ES );
>>> -        audio_output_t *p_aout = p_owner->p_aout;
>>>   
>>>           vlc_mutex_lock( &p_owner->lock );
>>> +        audio_output_t *p_aout = p_owner->p_aout;
>>>           p_owner->p_aout = NULL;
>>>           vlc_mutex_unlock( &p_owner->lock );
>>>           if( p_aout )
>>> @@ -301,10 +301,9 @@ static int aout_update_format( decoder_t *p_dec )
>>>            p_dec->fmt_out.i_codec != p_dec->fmt_out.audio.i_format ||
>>>            p_dec->fmt_out.i_profile != p_owner->fmt.i_profile ) )
>>>       {
>>> -        audio_output_t *p_aout = p_owner->p_aout;
>>> -
>>>           /* Parameters changed, restart the aout */
>>>           vlc_mutex_lock( &p_owner->lock );
>>> +        audio_output_t *p_aout = p_owner->p_aout;
>>>           p_owner->p_aout = NULL;
>>>           vlc_mutex_unlock( &p_owner->lock );
>>>           aout_DecDelete( p_aout );
>>
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>