[vlc-devel] [V3 PATCH 3/3] dec: update pause/rate when the output is restarted

[Thomas Guillem]

On Wed, Jul 4, 2018, at 15:33, Rémi Denis-Courmont wrote:
> Well it wouldn't surprise me if it created races or inversions. but it
> would surprise me if doing it one way would avoid races and the other
> way not.
I don't see races with this current set.

> 
> Le 4 juillet 2018 11:37:57 GMT+03:00, Thomas Guillem <thomas at gllm.fr>
> a écrit :>> 
>> 
>> On Tue, Jul 3, 2018, at 17:58, Rémi Denis-Courmont wrote:
>> 
>>>  Le tiistaina 3. heinäkuuta 2018, 14.22.30 EEST Thomas Guillem a
>>>  écrit :
>>>>>>>  On Tue, Jul 3, 2018, at 13:20, Rémi Denis-Courmont wrote:
>>>> 
>>>>>  There should not be a need to move the current state out of the
>>>>>>>>>>  stack, AFAIU.>
>>>>> 
>>>>>  
>>>>> 
>>>>>   I like to keep thread variables on stack because it's then
>>>>>   obvious
>>>>>>>>>>   what the access rule is (read/write for the thread) and if the
>>>>>>>>>>   variable becomes unused.
>>>>> 
>>>>  
>>>> 
>>>>  Then, how do you fix what this commit is trying to fix ? With a
>>>>  reset
>>>>>>>>  bool that reset pause/rate from the DecoderThread stack ?
>>>> 
>>>  
>>> 
>>>  Set the correct values when you get the xout ?
>>> 
>> 
>> 
>> I implemented it (badly), pushed it, and force reverted it.
>> 
>> It's more difficult than expected. With the 2 different locks and
>> thread, it's hard (impossible?) to never miss a rate/paused state.
>>>> 
>> 
>> I explain, cf. aout diff (the same for vout):
>> 
>> 
>> 
>> @@ -384,6 +384,17 @@ static int aout_update_format( decoder_t
>> *p_dec )
>>>>              }
>> 
>>          }
>> 
>> 
>> 
>> +        vlc_fifo_Lock( p_owner->p_fifo );
>> 
>> +        float rate = p_owner->rate;
>> 
>> +        vlc_tick_t pause_date = p_owner->pause_date;
>> 
>> +        bool paused = p_owner->paused;
>> 
>> +        vlc_fifo_Unlock( p_owner->p_fifo );
>> 
>> +
>> 
>> +        if( paused )
>> 
>> +            aout_DecChangePause( p_aout, paused, pause_date );
>> 
>> +        if( rate != 1.f )
>> 
>> +            aout_DecChangeRate( p_aout, rate );
>> 
>> +
>> 
>>          vlc_mutex_lock( &p_owner->lock );
>> 
>>          p_owner->p_aout = p_aout;
>> 
>> 
>> 
>> Here I update the aout state before writing it to p_owner->p_aout so
>> that the DecoderThread won't change these states on its own. But if a
>> new pause/rate is sent from the input between vlc_fifo_Unlock( p_owner-
>> >p_fifo ) and vlc_mutex_lock( &p_owner->lock ), and if the
>> DecoderThread already process it in the meantime (and ignore it since
>> since there is no aout), the state will be missed.
>>>> 
>> 
>> 
>> 
>> Therefore, I prefer doing what this V3 patch set is doing.
>> 
>> 
>> 
>>>  
>>> 
>>>  -- 
>>> 
>>>  Реми Дёни-Курмон
>>> 
>>>  http://www.remlab.net/
>>> 
>>>  
>>> 
>>>  
>>> 
>>>  
>>> 
>>> 
>>> 
>>>  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
>> 
> 
> --
>  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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180704/09a20106/attachment.html>