[vlc-devel] [PATCH] input: fix restart of all ES when changing renderer

[Thomas Guillem]

The sout was destroyed before decoders were deleted. This could lead to a
use-after-free of the sout from the DecoderThread.

To fix this issue, we stop all ES, save the stopped state of all ES, change the
renderer/sout configuration and restart all previously stopped ES.

Deprecate ES_OUT_RESTART_ALL_ES since it was only used for this case. And
replace it by ES_OUT_STOP_ALL_ES/ES_OUT_START_ALL_ES, but in private headers.
---
 include/vlc_es_out.h         |  4 +---
 src/input/es_out.c           | 27 +++++++++++++++++++++++++--
 src/input/es_out.h           |  6 ++++++
 src/input/es_out_timeshift.c |  2 ++
 src/input/input.c            |  6 +++++-
 5 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/include/vlc_es_out.h b/include/vlc_es_out.h
index fbed8bd3ae..fe53c2a3d3 100644
--- a/include/vlc_es_out.h
+++ b/include/vlc_es_out.h
@@ -38,9 +38,7 @@ enum es_out_query_e
     /* set or change the selected ES in its category (audio/video/spu) */
     ES_OUT_SET_ES,      /* arg1= es_out_id_t*                   */
     ES_OUT_RESTART_ES,  /* arg1= es_out_id_t*                   */
-    /* Restart all ES, destroying and recreating decoder/sout and potential
-     * video/audio outputs. This is not recommended and might not even work */
-    ES_OUT_RESTART_ALL_ES, /* No arg */
+    ES_OUT_RESTART_ALL_ES, /* Deprecated, no effect */
 
     /* set 'default' tag on ES (copied across from container) */
     ES_OUT_SET_ES_DEFAULT, /* arg1= es_out_id_t*                */
diff --git a/src/input/es_out.c b/src/input/es_out.c
index d25020f1dd..f44c532119 100644
--- a/src/input/es_out.c
+++ b/src/input/es_out.c
@@ -2383,16 +2383,39 @@ static int EsOutControlLocked( es_out_t *out, int i_query, va_list args )
         }
         return VLC_SUCCESS;
     }
-    case ES_OUT_RESTART_ALL_ES:
+    case ES_OUT_STOP_ALL_ES:
     {
+        int *selected_es = vlc_alloc(p_sys->i_es + 1, sizeof(int));
+        if (!selected_es)
+            return VLC_EGENERIC;
+        selected_es[0] = p_sys->i_es;
         for( int i = 0; i < p_sys->i_es; i++ )
         {
             if( EsIsSelected( p_sys->es[i] ) )
             {
                 EsDestroyDecoder( out, p_sys->es[i] );
-                EsCreateDecoder( out, p_sys->es[i] );
+                selected_es[i + 1] = p_sys->es[i]->i_id;
+            }
+            else
+                selected_es[i + 1] = -1;
+        }
+        *va_arg( args, void **) = selected_es;
+        return VLC_SUCCESS;
+    }
+    case ES_OUT_START_ALL_ES:
+    {
+        int *selected_es = va_arg( args, void * );
+        int count = selected_es[0];
+        for( int i = 0; i < count; ++i )
+        {
+            int i_id = selected_es[i + 1];
+            if( i_id != -1 )
+            {
+                es_out_id_t *p_es = EsOutGetFromID( out, i_id );
+                EsCreateDecoder( out, p_es );
             }
         }
+        free(selected_es);
         return VLC_SUCCESS;
     }
 
diff --git a/src/input/es_out.h b/src/input/es_out.h
index 56cefed7c9..8f202266e8 100644
--- a/src/input/es_out.h
+++ b/src/input/es_out.h
@@ -50,6 +50,12 @@ enum es_out_query_private_e
     ES_OUT_SET_ES_DEFAULT_BY_ID,
     ES_OUT_GET_ES_OBJECTS_BY_ID,                    /* arg1=int id, vlc_object_t **dec, vout_thread_t **, audio_output_t ** res=can fail*/
 
+    /* Stop all selected ES and save the stopped state in a context. free the
+     * context or call ES_OUT_STOP_ALL_ES */
+    ES_OUT_STOP_ALL_ES,                             /* arg1=void ** */
+    /* Start all ES from the context returned by ES_OUT_STOP_ALL_ES */
+    ES_OUT_START_ALL_ES,                            /* arg1=void * */
+
     /* Get buffering state */
     ES_OUT_GET_BUFFERING,                           /* arg1=bool*               res=cannot fail */
 
diff --git a/src/input/es_out_timeshift.c b/src/input/es_out_timeshift.c
index c6dd83ebb8..e30f923958 100644
--- a/src/input/es_out_timeshift.c
+++ b/src/input/es_out_timeshift.c
@@ -743,6 +743,8 @@ static int ControlLocked( es_out_t *p_out, int i_query, va_list args )
     case ES_OUT_RESTART_ES_BY_ID:
     case ES_OUT_SET_ES_DEFAULT_BY_ID:
     case ES_OUT_GET_ES_OBJECTS_BY_ID:
+    case ES_OUT_STOP_ALL_ES:
+    case ES_OUT_START_ALL_ES:
     case ES_OUT_SET_DELAY:
     case ES_OUT_SET_RECORD_STATE:
     default:
diff --git a/src/input/input.c b/src/input/input.c
index c1beba330e..665391a02a 100644
--- a/src/input/input.c
+++ b/src/input/input.c
@@ -1809,7 +1809,6 @@ static void ControlUpdateRenderer( input_thread_t *p_input, bool b_enable )
                                     input_priv(p_input)->p_sout, NULL );
         input_priv(p_input)->p_sout = NULL;
     }
-    es_out_Control( input_priv(p_input)->p_es_out, ES_OUT_RESTART_ALL_ES );
 }
 #endif
 
@@ -2238,6 +2237,9 @@ static bool Control( input_thread_t *p_input,
             if ( p_item == NULL && p_priv->p_renderer == NULL )
                 break;
 
+            void *context;
+            es_out_Control( input_priv(p_input)->p_es_out_display, ES_OUT_STOP_ALL_ES,
+                            &context );
             if ( p_priv->p_renderer )
             {
                 ControlUpdateRenderer( p_input, false );
@@ -2258,6 +2260,8 @@ static bool Control( input_thread_t *p_input,
                 }
                 input_resource_TerminateVout( p_priv->p_resource );
             }
+            es_out_Control( input_priv(p_input)->p_es_out_display, ES_OUT_START_ALL_ES,
+                            context );
 #endif
             break;
         }
-- 
2.11.0