<!DOCTYPE html>
<html>
<head>
<title></title>
<style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style>
</head>
<body><div><br></div>
<div>On Tue, Jul 3, 2018, at 13:20, Rémi Denis-Courmont wrote:<br></div>
<blockquote type="cite"><div>There should not be a need to move the current state out of the stack, AFAIU.<br></div>
<div> <br></div>
<div> 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.<br></div>
</blockquote><div><br></div>
<div>Then, how do you fix what this commit is trying to fix ? With a reset bool that reset pause/rate from the DecoderThread stack ?<br></div>
<div><br></div>
<blockquote type="cite"><div><br></div>
<div defang_data-gmailquote="yes"><div>Le 3 juillet 2018 12:15:52 GMT+03:00, Thomas Guillem <thomas@gllm.fr> a écrit :<br></div>
<blockquote defang_data-gmailquote="yes" style="margin-top:0pt;margin-right:0pt;margin-bottom:0pt;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><pre><div>The aout/vout paused/rate state was not set if the output was restarted within<br></div>
<div>the lifetime of the decoder.<br></div>
<div>---<br></div>
<div> src/input/decoder.c | 66 +++++++++++++++++++++++++++------------------<br></div>
<div> 1 file changed, 40 insertions(+), 26 deletions(-)<br></div>
<div><br></div>
<div>diff --git a/src/input/decoder.c b/src/input/decoder.c<br></div>
<div>index 27dbfba50d..3e5d14814c 100644<br></div>
<div>--- a/src/input/decoder.c<br></div>
<div>+++ b/src/input/decoder.c<br></div>
<div>@@ -112,10 +112,12 @@ struct decoder_owner<br></div>
<div>     /* Preroll */<br></div>
<div>     vlc_tick_t i_preroll_end;<br></div>
<div>     /* Pause & Rate */<br></div>
<div>-    vlc_tick_t pause_date;<br></div>
<div>-    float rate;<br></div>
<div>+    vlc_tick_t request_pause_date;<br></div>
<div>+    float request_rate;<br></div>
<div>+    float output_rate;<br></div>
<div>     unsigned frames_countdown;<br></div>
<div>-    bool paused;<br></div>
<div>+    bool request_paused;<br></div>
<div>+    bool output_paused;<br></div>
<div> <br></div>
<div>     bool error;<br></div>
<div> <br></div>
<div>@@ -284,6 +286,17 @@ static void DecoderUpdateFormatLocked( decoder_t *p_dec )<br></div>
<div> /*****************************************************************************<br></div>
<div>  * Buffers allocation callbacks for the decoders<br></div>
<div>  *****************************************************************************/<br></div>
<div>+static void out_reset_state( decoder_t *p_dec )<br></div>
<div>+{<br></div>
<div>+    struct decoder_owner *p_owner = dec_get_owner( p_dec );<br></div>
<div>+<br></div>
<div>+    vlc_fifo_Lock( p_owner->p_fifo );<br></div>
<div>+    p_owner->output_rate = 1.f;<br></div>
<div>+    p_owner->output_paused = false;<br></div>
<div>+    vlc_fifo_Unlock( p_owner->p_fifo );<br></div>
<div>+<br></div>
<div>+}<br></div>
<div>+<br></div>
<div> static vout_thread_t *aout_request_vout( void *p_private,<br></div>
<div>                                          vout_thread_t *p_vout,<br></div>
<div>                                          const video_format_t *p_fmt, bool b_recyle )<br></div>
<div>@@ -406,6 +419,8 @@ static int aout_update_format( decoder_t *p_dec )<br></div>
<div>             p_owner->fmt.audio.i_bytes_per_frame;<br></div>
<div>         p_dec->fmt_out.audio.i_frame_length =<br></div>
<div>             p_owner->fmt.audio.i_frame_length;<br></div>
<div>+<br></div>
<div>+        out_reset_state( p_dec );<br></div>
<div>     }<br></div>
<div>     return 0;<br></div>
<div> }<br></div>
<div>@@ -538,6 +553,7 @@ static int vout_update_format( decoder_t *p_dec )<br></div>
<div>             msg_Err( p_dec, "failed to create video output" );<br></div>
<div>             return -1;<br></div>
<div>         }<br></div>
<div>+        out_reset_state( p_dec );<br></div>
<div>     }<br></div>
<div> <br></div>
<div>     if ( memcmp( &p_dec->fmt_out.video.mastering,<br></div>
<div>@@ -628,7 +644,7 @@ static vlc_tick_t DecoderGetDisplayDate( decoder_t *p_dec, vlc_tick_t i_ts )<br></div>
<div>     struct decoder_owner *p_owner = dec_get_owner( p_dec );<br></div>
<div> <br></div>
<div>     vlc_mutex_lock( &p_owner->lock );<br></div>
<div>-    if( p_owner->b_waiting || p_owner->paused )<br></div>
<div>+    if( p_owner->b_waiting || p_owner->request_paused )<br></div>
<div>         i_ts = VLC_TICK_INVALID;<br></div>
<div>     vlc_mutex_unlock( &p_owner->lock );<br></div>
<div> <br></div>
<div>@@ -1016,7 +1032,7 @@ static void DecoderPlayVideo( decoder_t *p_dec, picture_t *p_picture,<br></div>
<div>     /* FIXME: The *input* FIFO should not be locked here. This will not work<br></div>
<div>      * properly if/when pictures are queued asynchronously. */<br></div>
<div>     vlc_fifo_Lock( p_owner->p_fifo );<br></div>
<div>-    if( unlikely(p_owner->paused) && likely(p_owner->frames_countdown > 0) )<br></div>
<div>+    if( unlikely(p_owner->request_paused) && likely(p_owner->frames_countdown > 0) )<br></div>
<div>         p_owner->frames_countdown--;<br></div>
<div>     vlc_fifo_Unlock( p_owner->p_fifo );<br></div>
<div> <br></div>
<div>@@ -1561,8 +1577,6 @@ static void *DecoderThread( void *p_data )<br></div>
<div> {<br></div>
<div>     decoder_t *p_dec = (decoder_t *)p_data;<br></div>
<div>     struct decoder_owner *p_owner = dec_get_owner( p_dec );<br></div>
<div>-    float rate = 1.f;<br></div>
<div>-    bool paused = false;<br></div>
<div> <br></div>
<div>     /* The decoder's main loop */<br></div>
<div>     vlc_fifo_Lock( p_owner->p_fifo );<br></div>
<div>@@ -1591,11 +1605,11 @@ static void *DecoderThread( void *p_data )<br></div>
<div>             continue;<br></div>
<div>         }<br></div>
<div> <br></div>
<div>-        if( paused != p_owner->paused )<br></div>
<div>+        if( p_owner->output_paused != p_owner->request_paused )<br></div>
<div>         {   /* Update playing/paused status of the output */<br></div>
<div>             int canc = vlc_savecancel();<br></div>
<div>-            bool request_paused = p_owner->paused;<br></div>
<div>-            vlc_tick_t request_date = p_owner->pause_date;<br></div>
<div>+            bool request_paused = p_owner->request_paused;<br></div>
<div>+            vlc_tick_t request_date = p_owner->request_pause_date;<br></div>
<div> <br></div>
<div>             vlc_fifo_Unlock( p_owner->p_fifo );<br></div>
<div> <br></div>
<div>@@ -1605,14 +1619,14 @@ static void *DecoderThread( void *p_data )<br></div>
<div>             vlc_restorecancel( canc );<br></div>
<div>             vlc_fifo_Lock( p_owner->p_fifo );<br></div>
<div>             if (success)<br></div>
<div>-                paused = request_paused;<br></div>
<div>+                p_owner->output_paused = request_paused;<br></div>
<div>             continue;<br></div>
<div>         }<br></div>
<div> <br></div>
<div>-        if( rate != p_owner->rate )<br></div>
<div>+        if( p_owner->output_rate != p_owner->request_rate )<br></div>
<div>         {<br></div>
<div>             int canc = vlc_savecancel();<br></div>
<div>-            float request_rate = p_owner->rate;<br></div>
<div>+            float request_rate = p_owner->request_rate;<br></div>
<div> <br></div>
<div>             vlc_fifo_Unlock( p_owner->p_fifo );<br></div>
<div> <br></div>
<div>@@ -1622,10 +1636,10 @@ static void *DecoderThread( void *p_data )<br></div>
<div>             vlc_restorecancel( canc );<br></div>
<div>             vlc_fifo_Lock( p_owner->p_fifo );<br></div>
<div>             if (success)<br></div>
<div>-                rate = request_rate;<br></div>
<div>+                p_owner->output_rate = request_rate;<br></div>
<div>         }<br></div>
<div> <br></div>
<div>-        if( p_owner->paused && p_owner->frames_countdown == 0 )<br></div>
<div>+        if( p_owner->request_paused && p_owner->frames_countdown == 0 )<br></div>
<div>         {   /* Wait for resumption from pause */<br></div>
<div>             p_owner->b_idle = true;<br></div>
<div>             vlc_cond_signal( &p_owner->wait_acknowledge );<br></div>
<div>@@ -1746,9 +1760,9 @@ static decoder_t * CreateDecoder( vlc_object_t *p_parent,<br></div>
<div>     p_owner->b_fmt_description = false;<br></div>
<div>     p_owner->p_description = NULL;<br></div>
<div> <br></div>
<div>-    p_owner->rate = 1.f;<br></div>
<div>-    p_owner->paused = false;<br></div>
<div>-    p_owner->pause_date = VLC_TICK_INVALID;<br></div>
<div>+    p_owner->request_rate = p_owner->output_rate = 1.f;<br></div>
<div>+    p_owner->request_paused = p_owner->output_paused = false;<br></div>
<div>+    p_owner->request_pause_date = VLC_TICK_INVALID;<br></div>
<div>     p_owner->frames_countdown = 0;<br></div>
<div> <br></div>
<div>     p_owner->b_waiting = false;<br></div>
<div>@@ -2209,7 +2223,7 @@ void input_DecoderFlush( decoder_t *p_dec )<br></div>
<div> <br></div>
<div>     /* Flush video/spu decoder when paused: increment frames_countdown in order<br></div>
<div>      * to display one frame/subtitle */<br></div>
<div>-    if( p_owner->paused<br></div>
<div>+    if( p_owner->request_paused<br></div>
<div>      && ( p_owner->fmt.i_cat == VIDEO_ES || p_owner->fmt.i_cat == SPU_ES )<br></div>
<div>      && p_owner->frames_countdown == 0 )<br></div>
<div>         p_owner->frames_countdown++;<br></div>
<div>@@ -2330,8 +2344,8 @@ void input_DecoderChangePause( decoder_t *p_dec, bool b_paused, vlc_tick_t i_dat<br></div>
<div>      * while the input is paused (e.g. add sub file), then b_paused is<br></div>
<div>      * (incorrectly) false. FIXME: This is a bug in the decoder owner. */<br></div>
<div>     vlc_fifo_Lock( p_owner->p_fifo );<br></div>
<div>-    p_owner->paused = b_paused;<br></div>
<div>-    p_owner->pause_date = i_date;<br></div>
<div>+    p_owner->request_paused = b_paused;<br></div>
<div>+    p_owner->request_pause_date = i_date;<br></div>
<div>     p_owner->frames_countdown = 0;<br></div>
<div>     vlc_fifo_Signal( p_owner->p_fifo );<br></div>
<div>     vlc_fifo_Unlock( p_owner->p_fifo );<br></div>
<div>@@ -2342,7 +2356,7 @@ void input_DecoderChangeRate( decoder_t *dec, float rate )<br></div>
<div>     struct decoder_owner *owner = dec_get_owner( dec );<br></div>
<div> <br></div>
<div>     vlc_fifo_Lock( owner->p_fifo );<br></div>
<div>-    owner->rate = rate;<br></div>
<div>+    owner->request_rate = rate;<br></div>
<div>     vlc_fifo_Signal( owner->p_fifo );<br></div>
<div>     vlc_fifo_Unlock( owner->p_fifo );<br></div>
<div> }<br></div>
<div>@@ -2391,9 +2405,9 @@ void input_DecoderWait( decoder_t *p_dec )<br></div>
<div>     vlc_mutex_lock( &p_owner->lock );<br></div>
<div>     while( !p_owner->b_has_data )<br></div>
<div>     {<br></div>
<div>-        /* Don't need to lock p_owner->paused since it's only modified by the<br></div>
<div>-         * owner */<br></div>
<div>-        if( p_owner->paused )<br></div>
<div>+        /* Don't need to lock p_owner->request_paused since it's only modified<br></div>
<div>+         * by the owner */<br></div>
<div>+        if( p_owner->request_paused )<br></div>
<div>             break;<br></div>
<div>         vlc_fifo_Lock( p_owner->p_fifo );<br></div>
<div>         if( p_owner->b_idle && vlc_fifo_IsEmpty( p_owner->p_fifo ) )<br></div>
<div>@@ -2412,7 +2426,7 @@ void input_DecoderFrameNext( decoder_t *p_dec, vlc_tick_t *pi_duration )<br></div>
<div> {<br></div>
<div>     struct decoder_owner *p_owner = dec_get_owner( p_dec );<br></div>
<div> <br></div>
<div>-    assert( p_owner->paused );<br></div>
<div>+    assert( p_owner->request_paused );<br></div>
<div>     *pi_duration = 0;<br></div>
<div> <br></div>
<div>     vlc_fifo_Lock( p_owner->p_fifo );<br></div>
</pre></blockquote></div>
<div><br></div>
<div>--<br></div>
<div> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté. <br></div>
<div><u>_______________________________________________</u><br></div>
<div>vlc-devel mailing list<br></div>
<div>To unsubscribe or modify your subscription options:<br></div>
<div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div>
</blockquote><div><br></div>
</body>
</html>