[vlc-devel] [PATCH] input: rework delay handling

Thomas Guillem thomas at gllm.fr
Wed Jun 19 11:40:03 CEST 2019


The player is now the only one capable of changing the input delay. It doesn't
have to wait for the input thread to get the actual delay. This fixes
vlc_player_GetCategoryDelay() and the OSD returning/showing a previous delay
(TOCTOU issue).

"sub-delay" and "audio-desync" options are not handled by the player.

es_out: p_sys->i_pts_delay is now the base delay (it doesn't include
p_sys->i_pts_jitter delay).

The extra_pts_delay (delay from tracks) handling is moved from input.c to
es_out.c. This paves the way for handling delay from multiple tracks of the
same category: only EsOutGetExtraDelay() will need to handle that case.

Delays are not set by an input control anymore but by a new function:
input_SetCategoryDelay(). This function directly call the es_out control. It
doesn't have to be called from the Mainloop thread (es out controls are locked).

Maybe, it would be better to completely hide all input controls and let the
input implementation decide if a control need to be handled from the MainLoop
thread or can be handled directly.
---
 src/input/es_out.c         | 60 +++++++++++++++++++--------
 src/input/event.h          | 18 ---------
 src/input/input.c          | 49 ++++------------------
 src/input/input_internal.h | 30 +++++---------
 src/input/player.c         | 83 +++++++++++++++++---------------------
 5 files changed, 97 insertions(+), 143 deletions(-)

diff --git a/src/input/es_out.c b/src/input/es_out.c
index d68999096f..2e175e9d33 100644
--- a/src/input/es_out.c
+++ b/src/input/es_out.c
@@ -186,6 +186,7 @@ typedef struct
 
     /* Clock configuration */
     vlc_tick_t  i_pts_delay;
+    vlc_tick_t  i_extra_pts_delay;
     vlc_tick_t  i_pts_jitter;
     int         i_cr_average;
     float       rate;
@@ -232,6 +233,7 @@ static void EsOutDecodersStopBuffering( es_out_t *out, bool b_forced );
 static void EsOutGlobalMeta( es_out_t *p_out, const vlc_meta_t *p_meta );
 static void EsOutMeta( es_out_t *p_out, const vlc_meta_t *p_meta, const vlc_meta_t *p_progmeta );
 static int EsOutEsUpdateFmt(es_out_t *out, es_out_id_t *es, const es_format_t *fmt);
+static int EsOutControlLocked( es_out_t *out, int i_query, ... );
 
 static char *LanguageGetName( const char *psz_code );
 static char *LanguageGetCode( const char *psz_lang );
@@ -574,6 +576,10 @@ static void EsOutSetDelay( es_out_t *out, int i_cat, vlc_tick_t i_delay )
 
     foreach_es_then_es_slaves(es)
         EsOutDecoderChangeDelay(out, es);
+
+    /* Update the clock pts delay only if the extra tracks delay changed */
+    EsOutControlLocked(out, ES_OUT_SET_JITTER, p_sys->i_pts_delay,
+                       p_sys->i_pts_jitter, p_sys->i_cr_average);
 }
 
 static int EsOutSetRecord(  es_out_t *out, bool b_record )
@@ -775,6 +781,8 @@ static void EsOutDecodersStopBuffering( es_out_t *out, bool b_forced )
         i_preroll_duration = __MAX( p_sys->i_preroll_end - i_stream_start, 0 );
 
     const vlc_tick_t i_buffering_duration = p_sys->i_pts_delay +
+                                         p_sys->i_pts_jitter +
+                                         p_sys->i_extra_pts_delay +
                                          i_preroll_duration +
                                          p_sys->i_buffering_extra_stream - p_sys->i_buffering_extra_initial;
 
@@ -975,7 +983,10 @@ static void EsOutFrameNext( es_out_t *out )
         if( i_ret )
             return;
 
-        p_sys->i_buffering_extra_initial = 1 + i_stream_duration - p_sys->i_pts_delay; /* FIXME < 0 ? */
+        p_sys->i_buffering_extra_initial = 1 + i_stream_duration
+                                         - p_sys->i_pts_delay
+                                         - p_sys->i_pts_jitter
+                                         - p_sys->i_extra_pts_delay; /* FIXME < 0 ? */
         p_sys->i_buffering_extra_system =
         p_sys->i_buffering_extra_stream = p_sys->i_buffering_extra_initial;
     }
@@ -1029,7 +1040,8 @@ static vlc_tick_t EsOutGetBuffering( es_out_t *out )
         }
 
         const vlc_tick_t i_consumed = i_system_duration * p_sys->rate - i_stream_duration;
-        i_delay = p_sys->i_pts_delay - i_consumed;
+        i_delay = p_sys->i_pts_delay + p_sys->i_pts_jitter
+                + p_sys->i_extra_pts_delay - i_consumed;
     }
     if( i_delay < 0 )
         return 0;
@@ -1167,8 +1179,10 @@ static es_out_pgrm_t *EsOutProgramAdd( es_out_t *out, int i_group )
     }
     if( p_sys->b_paused )
         input_clock_ChangePause( p_pgrm->p_input_clock, p_sys->b_paused, p_sys->i_pause_date );
-    input_clock_SetJitter( p_pgrm->p_input_clock, p_sys->i_pts_delay, p_sys->i_cr_average );
-    vlc_clock_main_SetInputDejitter( p_pgrm->p_main_clock, p_sys->i_pts_delay );
+    const vlc_tick_t pts_delay = p_sys->i_pts_delay + p_sys->i_pts_jitter
+                               + p_sys->i_extra_pts_delay;
+    input_clock_SetJitter( p_pgrm->p_input_clock, pts_delay, p_sys->i_cr_average );
+    vlc_clock_main_SetInputDejitter( p_pgrm->p_main_clock, pts_delay );
 
     /* Append it */
     vlc_list_append(&p_pgrm->node, &p_sys->programs);
@@ -2439,6 +2453,14 @@ static int EsOutControlLocked( es_out_t *out, int i_query, ... )
     return ret;
 }
 
+static vlc_tick_t EsOutGetExtraDelay(es_out_t *out)
+{
+    es_out_sys_t *p_sys = container_of(out, es_out_sys_t, out);
+    const vlc_tick_t extra_delay = __MIN(p_sys->i_audio_delay,
+                                         p_sys->i_spu_delay);
+    return extra_delay < 0 ? -extra_delay : 0;
+}
+
 /**
  * Control query handler
  *
@@ -2755,20 +2777,20 @@ static int EsOutVaControlLocked( es_out_t *out, int i_query, va_list args )
             if( b_late && ( !input_priv(p_sys->p_input)->p_sout ||
                             !input_priv(p_sys->p_input)->b_out_pace_control ) )
             {
-                const vlc_tick_t i_pts_delay_base = p_sys->i_pts_delay - p_sys->i_pts_jitter;
                 vlc_tick_t i_pts_delay = input_clock_GetJitter( p_pgrm->p_input_clock );
 
                 /* Avoid dangerously high value */
                 const vlc_tick_t i_jitter_max =
                         VLC_TICK_FROM_MS(var_InheritInteger( p_sys->p_input, "clock-jitter" ));
-                if( i_pts_delay > __MIN( i_pts_delay_base + i_jitter_max, INPUT_PTS_DELAY_MAX ) )
+                if( i_pts_delay > __MIN( p_sys->i_pts_delay + i_jitter_max, INPUT_PTS_DELAY_MAX ) )
                 {
                     es_out_pgrm_t *pgrm;
 
                     msg_Err( p_sys->p_input,
                              "ES_OUT_SET_(GROUP_)PCR  is called too late (jitter of %d ms ignored)",
-                             (int)MS_FROM_VLC_TICK(i_pts_delay - i_pts_delay_base) );
-                    i_pts_delay = p_sys->i_pts_delay;
+                             (int)MS_FROM_VLC_TICK(i_pts_delay - p_sys->i_pts_delay) );
+                    i_pts_delay = p_sys->i_pts_delay + p_sys->i_pts_jitter
+                                + p_sys->i_extra_pts_delay;
 
                     /* reset clock */
                     vlc_list_foreach(pgrm, &p_sys->programs, node)
@@ -2787,8 +2809,8 @@ static int EsOutVaControlLocked( es_out_t *out, int i_query, va_list args )
                     EsOutControlLocked( out, ES_OUT_RESET_PCR );
                 }
 
-                EsOutControlLocked( out, ES_OUT_SET_JITTER, i_pts_delay_base,
-                                    i_pts_delay - i_pts_delay_base,
+                EsOutControlLocked( out, ES_OUT_SET_JITTER, p_sys->i_pts_delay,
+                                    i_pts_delay - p_sys->i_pts_delay,
                                     p_sys->i_cr_average );
             }
         }
@@ -3041,23 +3063,27 @@ static int EsOutVaControlLocked( es_out_t *out, int i_query, va_list args )
         int     i_cr_average = va_arg( args, int );
         es_out_pgrm_t *pgrm;
 
+        const vlc_tick_t i_extra_pts_delay = EsOutGetExtraDelay(out);
         bool b_change_clock =
-            i_pts_delay + i_pts_jitter != p_sys->i_pts_delay ||
-            i_cr_average != p_sys->i_cr_average;
+            i_pts_delay != p_sys->i_pts_delay ||
+            i_cr_average != p_sys->i_cr_average ||
+            i_extra_pts_delay != p_sys->i_extra_pts_delay;
 
         assert( i_pts_jitter >= 0 );
-        p_sys->i_pts_delay  = i_pts_delay + i_pts_jitter;
+        p_sys->i_pts_delay  = i_pts_delay;
         p_sys->i_pts_jitter = i_pts_jitter;
         p_sys->i_cr_average = i_cr_average;
+        p_sys->i_extra_pts_delay = i_extra_pts_delay;
 
         if (b_change_clock)
         {
+            i_pts_delay += i_pts_jitter + i_extra_pts_delay;
+
             vlc_list_foreach(pgrm, &p_sys->programs, node)
             {
-                input_clock_SetJitter(pgrm->p_input_clock, i_pts_delay
-                                      + i_pts_jitter, i_cr_average);
-                vlc_clock_main_SetInputDejitter(pgrm->p_main_clock,
-                                                i_pts_delay + i_pts_jitter);
+                input_clock_SetJitter(pgrm->p_input_clock, i_pts_delay,
+                                      i_cr_average);
+                vlc_clock_main_SetInputDejitter(pgrm->p_main_clock, i_pts_delay);
             }
         }
         return VLC_SUCCESS;
diff --git a/src/input/event.h b/src/input/event.h
index 5da39053d2..01163c479a 100644
--- a/src/input/event.h
+++ b/src/input/event.h
@@ -91,24 +91,6 @@ static inline void input_SendEventRate(input_thread_t *p_input, float rate)
     });
 }
 
-static inline void input_SendEventAudioDelay(input_thread_t *p_input,
-                                              vlc_tick_t i_delay)
-{
-    input_SendEvent(p_input, &(struct vlc_input_event) {
-        .type = INPUT_EVENT_AUDIO_DELAY,
-        .audio_delay = i_delay,
-    });
-}
-
-static inline void input_SendEventSubtitleDelay(input_thread_t *p_input,
-                                                 vlc_tick_t i_delay)
-{
-    input_SendEvent(p_input, &(struct vlc_input_event) {
-        .type = INPUT_EVENT_SUBTITLE_DELAY,
-        .subtitle_delay = i_delay,
-    });
-}
-
 static inline void input_SendEventRecord(input_thread_t *p_input,
                                          bool b_recording)
 {
diff --git a/src/input/input.c b/src/input/input.c
index 0e7b8e9699..6ab5233653 100644
--- a/src/input/input.c
+++ b/src/input/input.c
@@ -237,6 +237,13 @@ void input_SetPosition( input_thread_t *p_input, float f_position, bool b_fast )
     input_ControlPush( p_input, INPUT_CONTROL_SET_POSITION, &param );
 }
 
+void input_SetCategoryDelay(input_thread_t *input, enum es_format_category_e cat,
+                            vlc_tick_t delay)
+{
+    assert(cat == AUDIO_ES || cat == SPU_ES);
+    es_out_SetDelay(input_priv(input)->p_es_out_display, cat, delay);
+}
+
 /**
  * Get the item from an input thread
  * FIXME it does not increase ref count of the item.
@@ -396,10 +403,6 @@ static input_thread_t *Create( vlc_object_t *p_parent,
     /* Create Object Variables for private use only */
     input_ConfigVarInit( p_input );
 
-    priv->i_audio_delay =
-        VLC_TICK_FROM_MS( var_GetInteger( p_input, "audio-desync" ) );
-    priv->i_spu_delay = 0;
-
     /* Remove 'Now playing' info as it is probably outdated */
     input_item_SetNowPlaying( p_item, NULL );
     input_item_SetESNowPlaying( p_item, NULL );
@@ -946,8 +949,6 @@ static void RequestSubRate( input_thread_t *p_input, float f_slave_fps )
 
 static void SetSubtitlesOptions( input_thread_t *p_input )
 {
-    input_thread_private_t *priv = input_priv(p_input);
-
     /* Get fps and set it if not already set */
     const float f_fps = input_priv(p_input)->master->f_fps;
     if( f_fps > 1.f )
@@ -955,14 +956,6 @@ static void SetSubtitlesOptions( input_thread_t *p_input )
         var_SetFloat( p_input, "sub-original-fps", f_fps );
         RequestSubRate( p_input, var_InheritFloat( p_input, "sub-fps" ) );
     }
-
-    int64_t sub_delay = var_InheritInteger( p_input, "sub-delay" );
-    if( sub_delay != 0 )
-    {
-        priv->i_spu_delay = vlc_tick_from_samples(sub_delay, 10);
-        input_SendEventSubtitleDelay( p_input, priv->i_spu_delay );
-        /* UpdatePtsDelay will be called next by InitPrograms */
-    }
 }
 
 static void GetVarSlaves( input_thread_t *p_input,
@@ -1178,19 +1171,9 @@ static void UpdatePtsDelay( input_thread_t *p_input )
     if( i_pts_delay < 0 )
         i_pts_delay = 0;
 
-    /* Take care of audio/spu delay */
-    const vlc_tick_t i_extra_delay = __MIN( p_sys->i_audio_delay, p_sys->i_spu_delay );
-    if( i_extra_delay < 0 )
-        i_pts_delay -= i_extra_delay;
-
     /* Update cr_average depending on the caching */
     const int i_cr_average = var_GetInteger( p_input, "cr-average" ) * i_pts_delay / DEFAULT_PTS_DELAY;
 
-    /* */
-    es_out_SetDelay( input_priv(p_input)->p_es_out_display, AUDIO_ES,
-                     p_sys->i_audio_delay );
-    es_out_SetDelay( input_priv(p_input)->p_es_out_display, SPU_ES,
-                     p_sys->i_spu_delay );
     es_out_SetJitter( input_priv(p_input)->p_es_out, i_pts_delay, 0, i_cr_average );
 }
 
@@ -2060,24 +2043,6 @@ static bool Control( input_thread_t *p_input,
             ViewpointApply( p_input );
             break;
 
-        case INPUT_CONTROL_SET_AUDIO_DELAY:
-            if( param.delay.b_absolute )
-                priv->i_audio_delay = param.delay.i_val;
-            else
-                priv->i_audio_delay += param.delay.i_val;
-            input_SendEventAudioDelay( p_input, priv->i_audio_delay );
-            UpdatePtsDelay( p_input );
-            break;
-
-        case INPUT_CONTROL_SET_SPU_DELAY:
-            if( param.delay.b_absolute )
-                priv->i_spu_delay = param.delay.i_val;
-            else
-                priv->i_spu_delay += param.delay.i_val;
-            input_SendEventSubtitleDelay( p_input, priv->i_spu_delay );
-            UpdatePtsDelay( p_input );
-            break;
-
         case INPUT_CONTROL_SET_TITLE:
         case INPUT_CONTROL_SET_TITLE_NEXT:
         case INPUT_CONTROL_SET_TITLE_PREV:
diff --git a/src/input/input_internal.h b/src/input/input_internal.h
index a2295420b7..96357200be 100644
--- a/src/input/input_internal.h
+++ b/src/input/input_internal.h
@@ -160,11 +160,6 @@ typedef enum input_event_type_e
     /* At least one of "signal-quality" or "signal-strength" has changed */
     INPUT_EVENT_SIGNAL,
 
-    /* "audio-delay" has changed */
-    INPUT_EVENT_AUDIO_DELAY,
-    /* "spu-delay" has changed */
-    INPUT_EVENT_SUBTITLE_DELAY,
-
     /* "bookmark" has changed */
     INPUT_EVENT_BOOKMARK,
 
@@ -305,10 +300,6 @@ struct vlc_input_event
         const struct input_stats_t *stats;
         /* INPUT_EVENT_SIGNAL */
         struct vlc_input_event_signal signal;
-        /* INPUT_EVENT_AUDIO_DELAY */
-        vlc_tick_t audio_delay;
-        /* INPUT_EVENT_SUBTITLE_DELAY */
-        vlc_tick_t subtitle_delay;
         /* INPUT_EVENT_CACHE */
         float cache;
         /* INPUT_EVENT_VOUT */
@@ -371,6 +362,16 @@ void input_SetTime( input_thread_t *, vlc_tick_t i_time, bool b_fast );
 
 void input_SetPosition( input_thread_t *, float f_position, bool b_fast );
 
+/**
+ * Set the delay of an ES category
+ *
+ * If called before input_Start(), the delay will be applied for next ES
+ * tracks. If called after input_Start(), the delay will be applied for all
+ * tracks of the category (and all future tracks).
+ */
+void input_SetCategoryDelay(input_thread_t *input, enum es_format_category_e cat,
+                            vlc_tick_t delay);
+
 /**
  * Get the input item for an input thread
  *
@@ -435,10 +436,6 @@ typedef union
         bool b_fast_seek;
         float f_val;
     } pos;
-    struct {
-        bool b_absolute;
-        vlc_tick_t i_val;
-    } delay;
     struct {
         vlc_es_id_t *id;
         unsigned page;
@@ -482,10 +479,6 @@ typedef struct input_thread_private_t
     vlc_tick_t  i_stop;     /* :stop-time, 0 if none */
     vlc_tick_t  i_time;     /* Current time */
 
-    /* Delays */
-    vlc_tick_t  i_audio_delay;
-    vlc_tick_t  i_spu_delay;
-
     /* Output */
     bool            b_out_pace_control; /* XXX Move it ot es_sout ? */
     sout_instance_t *p_sout;            /* Idem ? */
@@ -588,9 +581,6 @@ enum input_control_e
     INPUT_CONTROL_SET_INITIAL_VIEWPOINT, // set initial viewpoint (generally from video)
     INPUT_CONTROL_UPDATE_VIEWPOINT, // update viewpoint relative to current
 
-    INPUT_CONTROL_SET_AUDIO_DELAY,
-    INPUT_CONTROL_SET_SPU_DELAY,
-
     INPUT_CONTROL_ADD_SLAVE,
     INPUT_CONTROL_SET_SUBS_FPS,
 
diff --git a/src/input/player.c b/src/input/player.c
index 68305deb41..c4104710f9 100644
--- a/src/input/player.c
+++ b/src/input/player.c
@@ -115,8 +115,7 @@ struct vlc_player_input
 
     struct input_stats_t stats;
 
-    vlc_tick_t audio_delay;
-    vlc_tick_t subtitle_delay;
+    vlc_tick_t cat_delays[DATA_ES];
 
     vlc_player_program_vector program_vector;
     vlc_player_track_vector video_track_vector;
@@ -665,8 +664,6 @@ vlc_player_input_New(vlc_player_t *player, input_item_t *item)
 
     memset(&input->stats, 0, sizeof(input->stats));
 
-    input->audio_delay = input->subtitle_delay = 0;
-
     vlc_vector_init(&input->program_vector);
     vlc_vector_init(&input->video_track_vector);
     vlc_vector_init(&input->audio_track_vector);
@@ -688,6 +685,25 @@ vlc_player_input_New(vlc_player_t *player, input_item_t *item)
         free(input);
         return NULL;
     }
+
+    /* Initial sub/audio delay */
+    const vlc_tick_t cat_delays[DATA_ES] = {
+        [AUDIO_ES] =
+            VLC_TICK_FROM_MS(var_InheritInteger(player, "audio-desync")),
+        [SPU_ES] =
+            vlc_tick_from_samples(var_InheritInteger(player, "sub-delay"), 10),
+    };
+
+    for (enum es_format_category_e i = UNKNOWN_ES; i < DATA_ES; ++i)
+    {
+        input->cat_delays[i] = cat_delays[i];
+        if (cat_delays[i] != 0)
+        {
+            input_SetCategoryDelay(input->thread, i, cat_delays[i]);
+            vlc_player_SendEvent(player, on_category_delay_changed, i,
+                                 cat_delays[i]);
+        }
+    }
     return input;
 }
 
@@ -2053,16 +2069,6 @@ input_thread_Events(input_thread_t *input_thread,
             vlc_player_SendEvent(player, on_signal_changed,
                                  input->signal_quality, input->signal_strength);
             break;
-        case INPUT_EVENT_AUDIO_DELAY:
-            input->audio_delay = event->audio_delay;
-            vlc_player_SendEvent(player, on_category_delay_changed, AUDIO_ES,
-                                 input->audio_delay);
-            break;
-        case INPUT_EVENT_SUBTITLE_DELAY:
-            input->subtitle_delay = event->subtitle_delay;
-            vlc_player_SendEvent(player, on_category_delay_changed, SPU_ES,
-                                 input->subtitle_delay);
-            break;
         case INPUT_EVENT_CACHE:
             input->cache = event->cache;
             vlc_player_SendEvent(player, on_buffering_changed, event->cache);
@@ -2834,34 +2840,23 @@ vlc_player_SetCategoryDelay(vlc_player_t *player, enum es_format_category_e cat,
     if (!input)
         return VLC_EGENERIC;
 
-    switch (cat)
+    if (cat != AUDIO_ES && cat != SPU_ES)
+        return VLC_EGENERIC;
+    vlc_tick_t *cat_delay = &input->cat_delays[cat];
+
+    if (absolute)
+        *cat_delay = delay;
+    else
     {
-        case AUDIO_ES:
-            input_ControlPush(input->thread, INPUT_CONTROL_SET_AUDIO_DELAY,
-                &(input_control_param_t) {
-                    .delay = {
-                        .b_absolute = absolute,
-                        .i_val = delay,
-                    },
-            });
-            break;
-        case SPU_ES:
-            input_ControlPush(input->thread, INPUT_CONTROL_SET_SPU_DELAY,
-                &(input_control_param_t) {
-                    .delay = {
-                        .b_absolute = absolute,
-                        .i_val = delay,
-                    },
-            });
-            break;
-        default:
-            return VLC_EGENERIC;
+        *cat_delay += delay;
+        delay = *cat_delay;
     }
 
-    vlc_player_vout_OSDMessage(player, _("%s delay: %s%i ms"),
+    input_SetCategoryDelay(input->thread, cat, delay);
+    vlc_player_vout_OSDMessage(player, _("%s delay: %i ms"),
                                es_format_category_to_string(cat),
-                               absolute ? "" : "+",
                                (int)MS_FROM_VLC_TICK(delay));
+    vlc_player_SendEvent(player, on_category_delay_changed, cat, delay);
     return VLC_SUCCESS;
 }
 
@@ -2871,15 +2866,11 @@ vlc_player_GetCategoryDelay(vlc_player_t *player, enum es_format_category_e cat)
     struct vlc_player_input *input = vlc_player_get_input_locked(player);
     if (!input)
         return 0;
-    switch (cat)
-    {
-        case AUDIO_ES:
-            return input->audio_delay;
-        case SPU_ES:
-            return input->subtitle_delay;
-        default:
-            return 0;
-    }
+
+    if (cat != AUDIO_ES && cat != SPU_ES)
+        return 0;
+
+    return input->cat_delays[cat];
 }
 
 static struct {
-- 
2.20.1



More information about the vlc-devel mailing list