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

Rémi Denis-Courmont remi at remlab.net
Tue Jul 3 13:20:29 CEST 2018


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.

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 );
>-- 
>2.18.0
>
>_______________________________________________
>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é.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180703/91fee6e8/attachment.html>


More information about the vlc-devel mailing list