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

Thomas Guillem thomas at gllm.fr
Tue Jul 3 13:22:30 CEST 2018


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 ?
> 
> Le 3 juillet 2018 12:15:52 GMT+03:00, Thomas Guillem <thomas at gllm.fr>
> a écrit :>> The aout/vout paused/rate state was not set if the output was
>> restarted within
>>>> the lifetime of the decoder.
>> 
>> ---
>> 
>>  src/input/decoder.c | 66 +++++++++++++++++++++++++++------------------
>>>>  1 file changed, 40 insertions(+), 26 deletions(-)
>> 
>> 
>> 
>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>> 
>> index 27dbfba50d..3e5d14814c 100644
>> 
>> --- a/src/input/decoder.c
>> 
>> +++ b/src/input/decoder.c
>> 
>> @@ -112,10 +112,12 @@ struct decoder_owner
>> 
>>      /* Preroll */
>> 
>>      vlc_tick_t i_preroll_end;
>> 
>>      /* Pause & Rate */
>> 
>> -    vlc_tick_t pause_date;
>> 
>> -    float rate;
>> 
>> +    vlc_tick_t request_pause_date;
>> 
>> +    float request_rate;
>> 
>> +    float output_rate;
>> 
>>      unsigned frames_countdown;
>> 
>> -    bool paused;
>> 
>> +    bool request_paused;
>> 
>> +    bool output_paused;
>> 
>>  
>> 
>>      bool error;
>> 
>>  
>> 
>> @@ -284,6 +286,17 @@ static void DecoderUpdateFormatLocked( decoder_t
>> *p_dec )
>>>>  /******************************************************************-
>>  ***********
>>>>   * Buffers allocation callbacks for the decoders
>> 
>>   ******************************************************************-
>>   ***********/
>>>> +static void out_reset_state( decoder_t *p_dec )
>> 
>> +{
>> 
>> +    struct decoder_owner *p_owner = dec_get_owner( p_dec );
>> 
>> +
>> 
>> +    vlc_fifo_Lock( p_owner->p_fifo );
>> 
>> +    p_owner->output_rate = 1.f;
>> 
>> +    p_owner->output_paused = false;
>> 
>> +    vlc_fifo_Unlock( p_owner->p_fifo );
>> 
>> +
>> 
>> +}
>> 
>> +
>> 
>>  static vout_thread_t *aout_request_vout( void *p_private,
>> 
>>                                           vout_thread_t *p_vout,
>>>>                                           const video_format_t
>>                                           *p_fmt, bool b_recyle )
>>>> @@ -406,6 +419,8 @@ static int aout_update_format( decoder_t *p_dec )
>>>>              p_owner->fmt.audio.i_bytes_per_frame;
>> 
>>          p_dec->fmt_out.audio.i_frame_length =
>> 
>>              p_owner->fmt.audio.i_frame_length;
>> 
>> +
>> 
>> +        out_reset_state( p_dec );
>> 
>>      }
>> 
>>      return 0;
>> 
>>  }
>> 
>> @@ -538,6 +553,7 @@ static int vout_update_format( decoder_t *p_dec )
>>>>              msg_Err( p_dec, "failed to create video output" );
>> 
>>              return -1;
>> 
>>          }
>> 
>> +        out_reset_state( p_dec );
>> 
>>      }
>> 
>>  
>> 
>>      if ( memcmp( &p_dec->fmt_out.video.mastering,
>> 
>> @@ -628,7 +644,7 @@ static vlc_tick_t DecoderGetDisplayDate(
>> decoder_t *p_dec, vlc_tick_t i_ts )
>>>>      struct decoder_owner *p_owner = dec_get_owner( p_dec );
>> 
>>  
>> 
>>      vlc_mutex_lock( &p_owner->lock );
>> 
>> -    if( p_owner->b_waiting || p_owner->paused )
>> 
>> +    if( p_owner->b_waiting || p_owner->request_paused )
>> 
>>          i_ts = VLC_TICK_INVALID;
>> 
>>      vlc_mutex_unlock( &p_owner->lock );
>> 
>>  
>> 
>> @@ -1016,7 +1032,7 @@ static void DecoderPlayVideo( decoder_t *p_dec,
>> picture_t *p_picture,
>>>>      /* FIXME: The *input* FIFO should not be locked here. This will
>>      not work
>>>>       * properly if/when pictures are queued asynchronously. */
>> 
>>      vlc_fifo_Lock( p_owner->p_fifo );
>> 
>> -    if( unlikely(p_owner->paused) && likely(p_owner-
>>      >frames_countdown > 0) )
>>>> +    if( unlikely(p_owner->request_paused) && likely(p_owner-
>>      >frames_countdown > 0) )
>>>>          p_owner->frames_countdown--;
>> 
>>      vlc_fifo_Unlock( p_owner->p_fifo );
>> 
>>  
>> 
>> @@ -1561,8 +1577,6 @@ static void *DecoderThread( void *p_data )
>>>>  {
>> 
>>      decoder_t *p_dec = (decoder_t *)p_data;
>> 
>>      struct decoder_owner *p_owner = dec_get_owner( p_dec );
>> 
>> -    float rate = 1.f;
>> 
>> -    bool paused = false;
>> 
>>  
>> 
>>      /* The decoder's main loop */
>> 
>>      vlc_fifo_Lock( p_owner->p_fifo );
>> 
>> @@ -1591,11 +1605,11 @@ static void *DecoderThread( void *p_data )
>>>>              continue;
>> 
>>          }
>> 
>>  
>> 
>> -        if( paused != p_owner->paused )
>> 
>> +        if( p_owner->output_paused != p_owner->request_paused )
>>>>          {   /* Update playing/paused status of the output */
>> 
>>              int canc = vlc_savecancel();
>> 
>> -            bool request_paused = p_owner->paused;
>> 
>> -            vlc_tick_t request_date = p_owner->pause_date;
>> 
>> +            bool request_paused = p_owner->request_paused;
>> 
>> +            vlc_tick_t request_date = p_owner->request_pause_date;
>>>>  
>> 
>>              vlc_fifo_Unlock( p_owner->p_fifo );
>> 
>>  
>> 
>> @@ -1605,14 +1619,14 @@ static void *DecoderThread( void *p_data )
>>>>              vlc_restorecancel( canc );
>> 
>>              vlc_fifo_Lock( p_owner->p_fifo );
>> 
>>              if (success)
>> 
>> -                paused = request_paused;
>> 
>> +                p_owner->output_paused = request_paused;
>> 
>>              continue;
>> 
>>          }
>> 
>>  
>> 
>> -        if( rate != p_owner->rate )
>> 
>> +        if( p_owner->output_rate != p_owner->request_rate )
>> 
>>          {
>> 
>>              int canc = vlc_savecancel();
>> 
>> -            float request_rate = p_owner->rate;
>> 
>> +            float request_rate = p_owner->request_rate;
>> 
>>  
>> 
>>              vlc_fifo_Unlock( p_owner->p_fifo );
>> 
>>  
>> 
>> @@ -1622,10 +1636,10 @@ static void *DecoderThread( void *p_data )
>>>>              vlc_restorecancel( canc );
>> 
>>              vlc_fifo_Lock( p_owner->p_fifo );
>> 
>>              if (success)
>> 
>> -                rate = request_rate;
>> 
>> +                p_owner->output_rate = request_rate;
>> 
>>          }
>> 
>>  
>> 
>> -        if( p_owner->paused && p_owner->frames_countdown == 0 )
>>>> +        if( p_owner->request_paused && p_owner->frames_countdown ==
>>          0 )
>>>>          {   /* Wait for resumption from pause */
>> 
>>              p_owner->b_idle = true;
>> 
>>              vlc_cond_signal( &p_owner->wait_acknowledge );
>> 
>> @@ -1746,9 +1760,9 @@ static decoder_t * CreateDecoder( vlc_object_t
>> *p_parent,
>>>>      p_owner->b_fmt_description = false;
>> 
>>      p_owner->p_description = NULL;
>> 
>>  
>> 
>> -    p_owner->rate = 1.f;
>> 
>> -    p_owner->paused = false;
>> 
>> -    p_owner->pause_date = VLC_TICK_INVALID;
>> 
>> +    p_owner->request_rate = p_owner->output_rate = 1.f;
>> 
>> +    p_owner->request_paused = p_owner->output_paused = false;
>> 
>> +    p_owner->request_pause_date = VLC_TICK_INVALID;
>> 
>>      p_owner->frames_countdown = 0;
>> 
>>  
>> 
>>      p_owner->b_waiting = false;
>> 
>> @@ -2209,7 +2223,7 @@ void input_DecoderFlush( decoder_t *p_dec )
>>>>  
>> 
>>      /* Flush video/spu decoder when paused: increment
>>      frames_countdown in order
>>>>       * to display one frame/subtitle */
>> 
>> -    if( p_owner->paused
>> 
>> +    if( p_owner->request_paused
>> 
>>       && ( p_owner->fmt.i_cat == VIDEO_ES || p_owner->fmt.i_cat ==
>>       SPU_ES )
>>>>       && p_owner->frames_countdown == 0 )
>> 
>>          p_owner->frames_countdown++;
>> 
>> @@ -2330,8 +2344,8 @@ void input_DecoderChangePause( decoder_t
>> *p_dec, bool b_paused, vlc_tick_t i_dat
>>>>       * while the input is paused (e.g. add sub file), then b_paused
>>         is
>>>>       * (incorrectly) false. FIXME: This is a bug in the decoder
>>         owner. */
>>>>      vlc_fifo_Lock( p_owner->p_fifo );
>> 
>> -    p_owner->paused = b_paused;
>> 
>> -    p_owner->pause_date = i_date;
>> 
>> +    p_owner->request_paused = b_paused;
>> 
>> +    p_owner->request_pause_date = i_date;
>> 
>>      p_owner->frames_countdown = 0;
>> 
>>      vlc_fifo_Signal( p_owner->p_fifo );
>> 
>>      vlc_fifo_Unlock( p_owner->p_fifo );
>> 
>> @@ -2342,7 +2356,7 @@ void input_DecoderChangeRate( decoder_t *dec,
>> float rate )
>>>>      struct decoder_owner *owner = dec_get_owner( dec );
>> 
>>  
>> 
>>      vlc_fifo_Lock( owner->p_fifo );
>> 
>> -    owner->rate = rate;
>> 
>> +    owner->request_rate = rate;
>> 
>>      vlc_fifo_Signal( owner->p_fifo );
>> 
>>      vlc_fifo_Unlock( owner->p_fifo );
>> 
>>  }
>> 
>> @@ -2391,9 +2405,9 @@ void input_DecoderWait( decoder_t *p_dec )
>>>>      vlc_mutex_lock( &p_owner->lock );
>> 
>>      while( !p_owner->b_has_data )
>> 
>>      {
>> 
>> -        /* Don't need to lock p_owner->paused since it's only
>>          modified by the
>>>> -         * owner */
>> 
>> -        if( p_owner->paused )
>> 
>> +        /* Don't need to lock p_owner->request_paused since it's
>>          only modified
>>>> +         * by the owner */
>> 
>> +        if( p_owner->request_paused )
>> 
>>              break;
>> 
>>          vlc_fifo_Lock( p_owner->p_fifo );
>> 
>>          if( p_owner->b_idle && vlc_fifo_IsEmpty( p_owner->p_fifo ) )
>>>> @@ -2412,7 +2426,7 @@ void input_DecoderFrameNext( decoder_t *p_dec,
>> vlc_tick_t *pi_duration )
>>>>  {
>> 
>>      struct decoder_owner *p_owner = dec_get_owner( p_dec );
>> 
>>  
>> 
>> -    assert( p_owner->paused );
>> 
>> +    assert( p_owner->request_paused );
>> 
>>      *pi_duration = 0;
>> 
>>  
>> 
>>      vlc_fifo_Lock( p_owner->p_fifo );
>> 
> 
> --
>  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/20180703/563fe1c2/attachment.html>


More information about the vlc-devel mailing list