[vlc-devel] [PATCH 1/2] Subtitle delay : fixed sync problem that cause audio to stop

Pascal Thomet pthomet at gmail.com
Thu May 29 19:43:04 CEST 2014


Bug reproduction step : *under linux* (will not happen under OSX)
- launch a video that has a subtitle,
- press Shift-H ("bookmark audio time"),
- wait 5 seconds
- press Shift-J ("bookmark subtitle time")
- press Shift-K ("sync bookmarks")
==> the audio is broken...

The subtitle delay ("spu-delay") was implemented by
  i) setting a variable spu-delay
     (var_SetTime(... , "spu-delay", ... ))
  ii) this change raised a callback that finaly lead to calling
    UpdatePtsDelay().

The reason UpdatePtsDelay() was called is that the subtitle delay
was programed the same way as the audio delay (for which it is required
to wait until the audio is actually demuxed)

*However* the situation for subtitles is quite different, as they are all in memory
(no need to wait until they are read !)

The code is now more simple (and the bug is fixed !) :
- no more spu-delay callbacks, everything is handled inside subtitle.c
- inside subtitle.c :
    * adjust_subtitle_time() receives a subtitle timestamp as input and returns
      that timestamp corrected by spu-delay
    * search_current_subtitle_if_necessary() is called whenever necessary
      (ie when spu-delay is changed)
---
 include/vlc_input.h               |   2 -
 modules/control/hotkeys.c         |  10 ----
 modules/demux/subtitle.c          | 109 +++++++++++++++++++++++++++++---------
 modules/gui/macosx/intf.m         |   1 -
 modules/gui/qt4/input_manager.cpp |   1 -
 src/input/event.c                 |  10 ----
 src/input/event.h                 |   1 -
 src/input/input.c                 |  10 +---
 src/input/input_internal.h        |   1 -
 src/input/var.c                   |   4 --
 10 files changed, 85 insertions(+), 64 deletions(-)

diff --git a/include/vlc_input.h b/include/vlc_input.h
index 88bfe65..043414e 100644
--- a/include/vlc_input.h
+++ b/include/vlc_input.h
@@ -380,8 +380,6 @@ typedef enum input_event_type_e
 
     /* "audio-delay" has changed */
     INPUT_EVENT_AUDIO_DELAY,
-    /* "spu-delay" has changed */
-    INPUT_EVENT_SUBTITLE_DELAY,
 
     /* "bookmark" has changed */
     INPUT_EVENT_BOOKMARK,
diff --git a/modules/control/hotkeys.c b/modules/control/hotkeys.c
index 1e7fcdf..c284383 100644
--- a/modules/control/hotkeys.c
+++ b/modules/control/hotkeys.c
@@ -433,16 +433,6 @@ static int PutAction( intf_thread_t *p_intf, int i_action )
             break;
         case ACTIONID_SUBSYNC_APPLY:
         {
-            /* Warning! Can yield a pause in the playback.
-             * For example, the following succession of actions will yield a 5 second delay :
-             * - Pressing Shift-H (ACTIONID_SUBSYNC_MARKAUDIO)
-             * - wait 5 second
-             * - Press Shift-J (ACTIONID_SUBSYNC_MARKSUB)
-             * - Press Shift-K (ACTIONID_SUBSYNC_APPLY)
-             * --> 5 seconds pause
-             * This is due to var_SetTime() (and ultimately UpdatePtsDelay())
-             * which causes the video to pause for an equivalent duration
-             * (This problem is also present in the "Track synchronization" window) */
             if ( p_input )
             {
                 if ( (p_sys->subtitle_delaybookmarks.i_time_audio == 0) || (p_sys->subtitle_delaybookmarks.i_time_subtitle == 0) )
diff --git a/modules/demux/subtitle.c b/modules/demux/subtitle.c
index 29922cc..322ddb5 100644
--- a/modules/demux/subtitle.c
+++ b/modules/demux/subtitle.c
@@ -127,6 +127,16 @@ static void TextUnload( text_t * );
 
 typedef struct
 {
+    /* 
+     * i_start and i_stop are the original subtitle timestamps.
+     * 
+     * In order to take into account the subtitle delay (spu-delay), 
+     * please use 
+     *   adjust_subtitle_time(p_demux, my_subtitle_t.i_start)
+     * instead of 
+     *   my_subtitle_t.i_start 
+     * (same goes for i_stop)
+     */
     int64_t i_start;
     int64_t i_stop;
 
@@ -224,6 +234,7 @@ static int Control( demux_t *, int, va_list );
 
 static void Fix( demux_t * );
 static char * get_language_from_filename( const char * );
+static int64_t adjust_subtitle_time( demux_t *, int64_t);
 
 /*****************************************************************************
  * Module initializer
@@ -249,6 +260,9 @@ static int Open ( vlc_object_t *p_this )
     p_demux->p_sys = p_sys = malloc( sizeof( demux_sys_t ) );
     if( p_sys == NULL )
         return VLC_ENOMEM;
+    
+    /* reset spu-delay at Open*/
+    var_SetTime( p_demux->p_parent, "spu-delay", 0 );
 
     p_sys->psz_header         = NULL;
     p_sys->i_subtitle         = 0;
@@ -591,6 +605,31 @@ static void Close( vlc_object_t *p_this )
 /*****************************************************************************
  * Control:
  *****************************************************************************/
+
+/* Utlity function : sets the current subtitle index (p_sys->i_subtitle)
+ * based on the time 
+ */
+static int set_current_subtitle_by_time(demux_t *p_demux, int64_t i64_when)
+{
+    demux_sys_t *p_sys = p_demux->p_sys;
+
+    p_sys->i_subtitle = 0;
+    while( p_sys->i_subtitle < p_sys->i_subtitles )
+    {
+        const subtitle_t *p_subtitle = &p_sys->subtitle[p_sys->i_subtitle];
+        if( adjust_subtitle_time(p_demux, p_subtitle->i_start) > i64_when )
+            break;
+        if( p_subtitle->i_stop > p_subtitle->i_start && adjust_subtitle_time(p_demux, p_subtitle->i_stop) > i64_when )
+            break;
+
+        p_sys->i_subtitle++;
+    }
+
+    if( p_sys->i_subtitle >= p_sys->i_subtitles )
+        return VLC_EGENERIC;
+    return VLC_SUCCESS;
+}
+
 static int Control( demux_t *p_demux, int i_query, va_list args )
 {
     demux_sys_t *p_sys = p_demux->p_sys;
@@ -608,29 +647,14 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
             pi64 = (int64_t*)va_arg( args, int64_t * );
             if( p_sys->i_subtitle < p_sys->i_subtitles )
             {
-                *pi64 = p_sys->subtitle[p_sys->i_subtitle].i_start;
+                *pi64 = adjust_subtitle_time(p_demux, p_sys->subtitle[p_sys->i_subtitle].i_start);
                 return VLC_SUCCESS;
             }
             return VLC_EGENERIC;
 
         case DEMUX_SET_TIME:
             i64 = (int64_t)va_arg( args, int64_t );
-            p_sys->i_subtitle = 0;
-            while( p_sys->i_subtitle < p_sys->i_subtitles )
-            {
-                const subtitle_t *p_subtitle = &p_sys->subtitle[p_sys->i_subtitle];
-
-                if( p_subtitle->i_start > i64 )
-                    break;
-                if( p_subtitle->i_stop > p_subtitle->i_start && p_subtitle->i_stop > i64 )
-                    break;
-
-                p_sys->i_subtitle++;
-            }
-
-            if( p_sys->i_subtitle >= p_sys->i_subtitles )
-                return VLC_EGENERIC;
-            return VLC_SUCCESS;
+            return set_current_subtitle_by_time(p_demux, i64);
 
         case DEMUX_GET_POSITION:
             pf = (double*)va_arg( args, double * );
@@ -640,7 +664,8 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
             }
             else if( p_sys->i_subtitles > 0 )
             {
-                *pf = (double)p_sys->subtitle[p_sys->i_subtitle].i_start /
+                int64_t i_start_adjust = adjust_subtitle_time(p_demux, p_sys->subtitle[p_sys->i_subtitle].i_start);
+                *pf = (double) ( i_start_adjust ) /
                       (double)p_sys->i_length;
             }
             else
@@ -655,7 +680,7 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
 
             p_sys->i_subtitle = 0;
             while( p_sys->i_subtitle < p_sys->i_subtitles &&
-                   p_sys->subtitle[p_sys->i_subtitle].i_start < i64 )
+                   adjust_subtitle_time(p_demux, p_sys->subtitle[p_sys->i_subtitle].i_start) < i64 )
             {
                 p_sys->i_subtitle++;
             }
@@ -682,6 +707,22 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
     }
 }
 
+
+/*****************************************************************************
+ * search_current_subtitle_if_necessary : reinits the current subtitle index 
+ * (aka p_sys->i_subtitle) if spu-delay has changed
+ *****************************************************************************/
+static void search_current_subtitle_if_necessary(demux_t *p_demux, int64_t when)
+{
+    static int64_t last_spu_delay = 0;
+    int64_t spu_delay = var_GetTime( p_demux->p_parent, "spu-delay" );
+    if (spu_delay != last_spu_delay)
+    {
+        set_current_subtitle_by_time(p_demux, when);    
+        last_spu_delay = spu_delay;
+    }
+}
+
 /*****************************************************************************
  * Demux: Send subtitle to decoder
  *****************************************************************************/
@@ -693,15 +734,17 @@ static int Demux( demux_t *p_demux )
     if( p_sys->i_subtitle >= p_sys->i_subtitles )
         return 0;
 
-    i_maxdate = p_sys->i_next_demux_date - var_GetTime( p_demux->p_parent, "spu-delay" );;
+    i_maxdate = p_sys->i_next_demux_date;
     if( i_maxdate <= 0 && p_sys->i_subtitle < p_sys->i_subtitles )
     {
         /* Should not happen */
-        i_maxdate = p_sys->subtitle[p_sys->i_subtitle].i_start + 1;
+        i_maxdate = adjust_subtitle_time(p_demux, p_sys->subtitle[p_sys->i_subtitle].i_start) + 1;
     }
-
+    
+    search_current_subtitle_if_necessary(p_demux, i_maxdate);
+    
     while( p_sys->i_subtitle < p_sys->i_subtitles &&
-           p_sys->subtitle[p_sys->i_subtitle].i_start < i_maxdate )
+           adjust_subtitle_time(p_demux, p_sys->subtitle[p_sys->i_subtitle].i_start) < i_maxdate )
     {
         const subtitle_t *p_subtitle = &p_sys->subtitle[p_sys->i_subtitle];
 
@@ -721,9 +764,9 @@ static int Demux( demux_t *p_demux )
         }
 
         p_block->i_dts =
-        p_block->i_pts = VLC_TS_0 + p_subtitle->i_start;
+        p_block->i_pts = VLC_TS_0 + adjust_subtitle_time(p_demux, p_subtitle->i_start);
         if( p_subtitle->i_stop >= 0 && p_subtitle->i_stop >= p_subtitle->i_start )
-            p_block->i_length = p_subtitle->i_stop - p_subtitle->i_start;
+            p_block->i_length = adjust_subtitle_time(p_demux, p_subtitle->i_stop) - adjust_subtitle_time(p_demux, p_subtitle->i_start);
 
         memcpy( p_block->p_buffer, p_subtitle->psz_text, i_len );
 
@@ -738,6 +781,22 @@ static int Demux( demux_t *p_demux )
     return 1;
 }
 
+
+/*****************************************************************************
+ * adjust_subtitle_time : receives a subtitle timestamp as input 
+ *                     (p_subtitle->i_start or p_subtitle->i_stop) 
+ *                      and returns that timestamp corrected 
+ *                      by spu-delay
+ *****************************************************************************/
+static int64_t adjust_subtitle_time( demux_t * p_demux, int64_t i64_when)
+{
+    int64_t spu_delay = var_GetTime( p_demux->p_parent, "spu-delay" );
+    int64_t i64_adjust = i64_when + spu_delay;
+    return i64_adjust;
+}
+
+
+
 /*****************************************************************************
  * Fix: fix time stamp and order of subtitle
  *****************************************************************************/
diff --git a/modules/gui/macosx/intf.m b/modules/gui/macosx/intf.m
index 8f46abb..26b6675 100644
--- a/modules/gui/macosx/intf.m
+++ b/modules/gui/macosx/intf.m
@@ -358,7 +358,6 @@ static int InputEvent(vlc_object_t *p_this, const char *psz_var,
             break;
 
         case INPUT_EVENT_AUDIO_DELAY:
-        case INPUT_EVENT_SUBTITLE_DELAY:
             [[VLCMain sharedInstance] performSelectorOnMainThread:@selector(updateDelays) withObject:nil waitUntilDone:NO];
             break;
 
diff --git a/modules/gui/qt4/input_manager.cpp b/modules/gui/qt4/input_manager.cpp
index c07f712..ef9ec66 100644
--- a/modules/gui/qt4/input_manager.cpp
+++ b/modules/gui/qt4/input_manager.cpp
@@ -377,7 +377,6 @@ static int InputEvent( vlc_object_t *p_this, const char *,
         break;
 
     case INPUT_EVENT_AUDIO_DELAY:
-    case INPUT_EVENT_SUBTITLE_DELAY:
         event = new IMEvent( IMEvent::SynchroChanged );
         break;
 
diff --git a/src/input/event.c b/src/input/event.c
index 453e04b..f4ca0d6 100644
--- a/src/input/event.c
+++ b/src/input/event.c
@@ -112,16 +112,6 @@ void input_SendEventAudioDelay( input_thread_t *p_input, mtime_t i_delay )
     Trigger( p_input, INPUT_EVENT_AUDIO_DELAY );
 }
 
-void input_SendEventSubtitleDelay( input_thread_t *p_input, mtime_t i_delay )
-{
-    vlc_value_t val;
-
-    val.i_time = i_delay;
-    var_Change( p_input, "spu-delay", VLC_VAR_SETVALUE, &val, NULL );
-
-    Trigger( p_input, INPUT_EVENT_SUBTITLE_DELAY );
-}
-
 /* TODO and file name ? */
 void input_SendEventRecord( input_thread_t *p_input, bool b_recording )
 {
diff --git a/src/input/event.h b/src/input/event.h
index c8f1071..b3c4b56 100644
--- a/src/input/event.h
+++ b/src/input/event.h
@@ -36,7 +36,6 @@ void input_SendEventLength( input_thread_t *p_input, mtime_t i_length );
 void input_SendEventStatistics( input_thread_t *p_input );
 void input_SendEventRate( input_thread_t *p_input, int i_rate );
 void input_SendEventAudioDelay( input_thread_t *p_input, mtime_t i_delay );
-void input_SendEventSubtitleDelay( input_thread_t *p_input, mtime_t i_delay );
 void input_SendEventRecord( input_thread_t *p_input, bool b_recording );
 void input_SendEventTitle( input_thread_t *p_input, int i_title );
 void input_SendEventSeekpoint( input_thread_t *p_input, int i_title, int i_seekpoint );
diff --git a/src/input/input.c b/src/input/input.c
index 7e71d4e..6d4bc4a 100644
--- a/src/input/input.c
+++ b/src/input/input.c
@@ -1102,8 +1102,7 @@ static void UpdatePtsDelay( input_thread_t *p_input )
 
     /* Take care of audio/spu delay */
     const mtime_t i_audio_delay = var_GetTime( p_input, "audio-delay" );
-    const mtime_t i_spu_delay   = var_GetTime( p_input, "spu-delay" );
-    const mtime_t i_extra_delay = __MIN( i_audio_delay, i_spu_delay );
+    const mtime_t i_extra_delay = i_audio_delay;
     if( i_extra_delay < 0 )
         i_pts_delay -= i_extra_delay;
 
@@ -1112,7 +1111,6 @@ static void UpdatePtsDelay( input_thread_t *p_input )
 
     /* */
     es_out_SetDelay( p_input->p->p_es_out_display, AUDIO_ES, i_audio_delay );
-    es_out_SetDelay( p_input->p->p_es_out_display, SPU_ES, i_spu_delay );
     es_out_SetJitter( p_input->p->p_es_out, i_pts_delay, 0, i_cr_average );
 }
 
@@ -1871,12 +1869,6 @@ static bool Control( input_thread_t *p_input,
             input_SendEventAudioDelay( p_input, val.i_time );
             UpdatePtsDelay( p_input );
             break;
-
-        case INPUT_CONTROL_SET_SPU_DELAY:
-            input_SendEventSubtitleDelay( p_input, val.i_time );
-            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 9f71294..9e534d9 100644
--- a/src/input/input_internal.h
+++ b/src/input/input_internal.h
@@ -202,7 +202,6 @@ enum input_control_e
     INPUT_CONTROL_RESTART_ES,
 
     INPUT_CONTROL_SET_AUDIO_DELAY,
-    INPUT_CONTROL_SET_SPU_DELAY,
 
     INPUT_CONTROL_ADD_SLAVE,
 
diff --git a/src/input/var.c b/src/input/var.c
index 3e722f4..dea46c8 100644
--- a/src/input/var.c
+++ b/src/input/var.c
@@ -787,10 +787,6 @@ static int EsDelayCallback ( vlc_object_t *p_this, char const *psz_cmd,
     {
         input_ControlPush( p_input, INPUT_CONTROL_SET_AUDIO_DELAY, &newval );
     }
-    else if( !strcmp( psz_cmd, "spu-delay" ) )
-    {
-        input_ControlPush( p_input, INPUT_CONTROL_SET_SPU_DELAY, &newval );
-    }
     return VLC_SUCCESS;
 }
 
-- 
1.9.1




More information about the vlc-devel mailing list