[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