[vlc-devel] [PATCH 1/5] Subtitle delay : fixed sync problem that breaks audio under linux
Pascal Thomet
pthomet at gmail.com
Mon Jun 9 21:54:58 CEST 2014
On 09 Jun 2014, at 21:26, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le lundi 9 juin 2014, 21:00:23 Pascal Thomet a écrit :
>> @@ -1067,7 +1011,30 @@ static int ActionEvent( vlc_object_t *libvlc, char
>> const *psz_var, (void)psz_var;
>> (void)oldval;
>>
>> - return PutAction( p_intf, newval.i_int );
>> + if ( strcmp( psz_var, "key-osdmessage") == 0)
>> + {
>> + if ( strlen(newval.psz_string) > 0 )
>> + {
>> + playlist_t *p_playlist = pl_Get( p_intf );
>> + input_thread_t *p_input = playlist_CurrentInput( p_playlist );
>> + if (p_input)
>> + {
>> + vout_thread_t *p_vout = p_input ? input_GetVout( p_input )
>> : NULL;// XXXXDZFEEGVDS
>> + if( p_vout )
>> + {
>> + DisplayMessage(p_vout, "%s", newval.psz_string);
>> + vlc_object_release( p_vout );
>> + }
>> + vlc_object_release(p_input);
>> + }
>> +
>> + }
>> + return VLC_SUCCESS;
>> + }
>> + else
>> + {
>> + return PutAction( p_intf, newval.i_int );
>> + }
>
> Seriously, WTH is this? That seems completely out of place.
>
Concerning the //XXXXDZ, this is a typo. Sorry for that.
If you mean that hotkey.c shall not expose a callback that displays OSD messages, I get it.
>> }
>>
>> static void PlayBookmark( intf_thread_t *p_intf, int i_num )
>> diff --git a/modules/demux/subtitle.c b/modules/demux/subtitle.c
>> index 29922cc..81d0a76 100644
>> --- a/modules/demux/subtitle.c
>> +++ b/modules/demux/subtitle.c
>> @@ -35,6 +35,7 @@
>> #include <vlc_plugin.h>
>> #include <vlc_input.h>
>> #include <vlc_memory.h>
>> +#include <vlc_interface.h>
>>
>> #include <ctype.h>
>>
>> @@ -127,6 +128,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;
>>
>> @@ -141,6 +152,7 @@ struct demux_sys_t
>> es_out_id_t *es;
>>
>> int64_t i_next_demux_date;
>> + int64_t i_last_demux_date;
>> int64_t i_microsecperframe;
>>
>> char *psz_header;
>> @@ -166,6 +178,14 @@ struct demux_sys_t
>> float f_total;
>> float f_factor;
>> } mpsub;
>> +
>> + /*subtitle_delaybookmarks: placeholder for storing subtitle sync
>> timestamps*/ + struct
>> + {
>> + int64_t i_time_subtitle;
>> + int64_t i_time_audio;
>> + } subtitle_delaybookmarks;
>> +
>> };
>>
>> static int ParseMicroDvd ( demux_t *, subtitle_t *, int );
>> @@ -224,6 +244,76 @@ 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);
>> +static int set_current_subtitle_by_time(demux_t *p_demux, int64_t
>> i64_when); +
>> +
>> +/**************************************************************************
>> *** + * external callbacks
>> +
>> ***************************************************************************
>> **/ +int subtitle_external_callback ( vlc_object_t * ,char const *,
>> vlc_value_t old_value, vlc_value_t new_value, void * callback_data); +int
>> subtitle_external_callback (
>> + vlc_object_t * object,
>> + char const * variable_name,
>> + vlc_value_t old_value,
>> + vlc_value_t new_value,
>> + void * callback_data_p_demux)
>> +{
>> + demux_t *p_demux = (demux_t*)callback_data_p_demux;
>> + (void)object;
>> + (void)old_value;
>> + (void)new_value;
>> +
>> + demux_sys_t *p_sys = p_demux->p_sys;
>> +
>> + if ( ! strcmp(variable_name, "sub-bookmarkaudio") )
>> + {
>> + p_sys->subtitle_delaybookmarks.i_time_audio =
>> p_sys->i_last_demux_date;
>> + var_SetString(p_demux->p_libvlc,
>> "key-osdmessage", _("Sub sync: bookmarked audio time"));
>> + }
>> + if ( ! strcmp(variable_name, "sub-bookmarksubtitle") )
>> + {
>> + p_sys->subtitle_delaybookmarks.i_time_subtitle =
>> p_sys->i_last_demux_date; + var_SetString(p_demux->p_libvlc,
>> "key-osdmessage", _("Sub sync: bookmarked subtitle time")); + }
>> + if ( ! strcmp(variable_name, "sub-syncbookmarks") )
>> + {
>> + if ( (p_sys->subtitle_delaybookmarks.i_time_audio == 0) ||
>> (p_sys->subtitle_delaybookmarks.i_time_subtitle == 0) ) + {
>> + var_SetString(p_demux->p_libvlc, "key-osdmessage", _("Sub sync:
>> set bookmarks first!")); + }
>> + else
>> + {
>> + int64_t i_current_subdelay = var_GetTime( p_demux->p_parent,
>> "spu-delay" ); + int64_t i_additional_subdelay =
>> p_sys->subtitle_delaybookmarks.i_time_audio -
>> p_sys->subtitle_delaybookmarks.i_time_subtitle; + int64_t
>> i_total_subdelay = i_current_subdelay + i_additional_subdelay; +
>> var_SetTime( p_demux->p_parent, "spu-delay", i_total_subdelay); +
>> char message[150];
>> + snprintf(message, 150,
>> + _( "Sub sync: corrected %i ms (total delay = %i ms)" ),
>> + (int)(i_additional_subdelay / 1000), +
>> (int)(i_total_subdelay / 1000) ); +
>> var_SetString(p_demux->p_libvlc, "key-osdmessage", message); +
>> p_sys->subtitle_delaybookmarks.i_time_audio = 0;
>> + p_sys->subtitle_delaybookmarks.i_time_subtitle = 0;
>> + }
>> + }
>> + if ( ! strcmp(variable_name, "sub-syncreset") )
>> + {
>> + var_SetTime( p_demux->p_parent, "spu-delay", 0);
>> + p_sys->subtitle_delaybookmarks.i_time_audio = 0;
>> + p_sys->subtitle_delaybookmarks.i_time_subtitle = 0;
>> + var_SetString(p_demux->p_libvlc, "key-osdmessage", _("Sub sync:
>> delay reset")); + return VLC_SUCCESS;
>> + }
>> + if ( ! strcmp(variable_name, "spu-delau") )
>> + {
>> + set_current_subtitle_by_time(p_demux, p_sys->i_last_demux_date);
>> + }
>> + return VLC_SUCCESS;
>> +}
>> +
>> +
>
> Demuxer accessing libvlc cannot be right.
Would it be better to store/access vars in p_demux->p_parent ? (i.e. var_SetString(p_demux->p_libvlc, …)
> Also memory accesses seem to violate
> the concurrency model.
???
>
>>
>> /**************************************************************************
>> *** * Module initializer
>> @@ -249,7 +339,29 @@ 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 );
>> + /* this is a file subtitle*/
>> + var_SetInteger(p_demux->p_parent, "sub-isfilesub", 1);
>> +
>> +
>> + p_sys->subtitle_delaybookmarks.i_time_audio = 0;
>> + p_sys->subtitle_delaybookmarks.i_time_subtitle = 0;
>> +
>> +
>> + /* Add callbacks*/
>> + var_Create(p_demux->p_parent, "sub-bookmarkaudio", VLC_VAR_INTEGER);
>> + var_Create(p_demux->p_parent, "sub-bookmarksubtitle", VLC_VAR_INTEGER);
>> + var_Create(p_demux->p_parent, "sub-syncbookmarks", VLC_VAR_INTEGER); +
>> var_Create(p_demux->p_parent, "sub-syncreset", VLC_VAR_INTEGER); +
>> var_AddCallback( p_demux->p_parent, "sub-bookmarkaudio",
>> &subtitle_external_callback, p_this );
>> + var_AddCallback(
>> p_demux->p_parent, "sub-bookmarksubtitle", &subtitle_external_callback,
>> p_this );
>> + var_AddCallback( p_demux->p_parent, "sub-syncbookmarks",
>> &subtitle_external_callback, p_this );
>> + var_AddCallback(
>> p_demux->p_parent, "sub-syncreset", &subtitle_external_callback, p_this );
>> + var_AddCallback( p_demux->p_parent, "spu-delay",
>> &subtitle_external_callback, p_this );
>> +
>> +
>
> Hell no. That's not how the demuxer interface works.
Any chance someone could provide some hints about how the demuxer is supposed to work?
The videolan wiki is not helpful about all these architecture aspects. How is someone supposed to get it?
The only document that I found gives which some hints on the architecture is not located on the wiki, but at :
http://www.enjoythearchitecture.com/vlc-architecture
>
>> p_sys->psz_header = NULL;
>> p_sys->i_subtitle = 0;
>> p_sys->i_subtitles = 0;
>> @@ -579,8 +691,23 @@ static void Close( vlc_object_t *p_this )
>> {
>> demux_t *p_demux = (demux_t*)p_this;
>> demux_sys_t *p_sys = p_demux->p_sys;
>> - int i;
>>
>> + var_SetInteger(p_demux->p_parent, "sub-isfilesub", 0);
>> +
>> + /* Remove callbacks*/
>> + var_DelCallback( p_demux->p_parent, "sub-bookmarkaudio",
>> &subtitle_external_callback, p_this ); + var_DelCallback(
>> p_demux->p_parent, "sub-bookmarksubtitle", &subtitle_external_callback,
>> p_this ); + var_DelCallback( p_demux->p_parent, "sub-syncbookmarks",
>> &subtitle_external_callback, p_this ); + var_DelCallback(
>> p_demux->p_parent, "sub-syncreset", &subtitle_external_callback, p_this );
>> +
>> + var_DelCallback( p_demux->p_parent, "spu-delay",
>> &subtitle_external_callback, p_this ); + var_Destroy(p_demux->p_parent,
>> "sub-bookmarkaudio");
>> + var_Destroy(p_demux->p_parent, "sub-bookmarksubtitle");
>> + var_Destroy(p_demux->p_parent, "sub-syncbookmarks");
>> + var_Destroy(p_demux->p_parent, "sub-syncreset");
>> +
>> +
>> + int i;
>> for( i = 0; i < p_sys->i_subtitles; i++ )
>> free( p_sys->subtitle[i].psz_text );
>> free( p_sys->subtitle );
>> @@ -591,6 +718,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 +760,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 +777,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 +793,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++;
>> }
>> @@ -693,15 +831,15 @@ 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; }
>> -
>> +
>> 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 +859,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 );
>>
>> @@ -733,11 +871,28 @@ static int Demux( demux_t *p_demux )
>> }
>>
>> /* */
>> + p_sys->i_last_demux_date = p_sys->i_next_demux_date;
>> p_sys->i_next_demux_date = 0;
>>
>> 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/src/input/input.c b/src/input/input.c
>> index 7e71d4e..ab34401 100644
>> --- a/src/input/input.c
>> +++ b/src/input/input.c
>> @@ -1103,7 +1103,14 @@ 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 );
>> + int isfilesub =var_GetInteger(p_input, "sub-isfilesub");
>> +
>> + mtime_t i_extra_delay;
>> + if ( isfilesub )
>> + i_extra_delay = i_audio_delay;
>> + else
>> + i_extra_delay = __MIN( i_audio_delay, i_spu_delay );
>> +
>> if( i_extra_delay < 0 )
>> i_pts_delay -= i_extra_delay;
>>
>> diff --git a/src/input/var.c b/src/input/var.c
>> index 3e722f4..be3ef2a 100644
>> --- a/src/input/var.c
>> +++ b/src/input/var.c
>> @@ -195,6 +195,14 @@ void input_ControlVarInit ( input_thread_t *p_input )
>> val.i_time = 0;
>> var_Change( p_input, "spu-delay", VLC_VAR_SETVALUE, &val, NULL );
>>
>> + /*sub-isfilesub will be
>> + * - equal to 1 when the subtitle is read from a file (for example a
>> .srt file), + * - equal to 0 otherwise (for example dvd subtitles)*/
>> + var_Create( p_input, "sub-isfilesub", VLC_VAR_INTEGER );
>> + val.i_int = 0;
>> + var_Change( p_input, "sub-isfilesub", VLC_VAR_SETVALUE, &val, 0 );
>> +
>> +
>> /* Video ES */
>> var_Create( p_input, "video-es", VLC_VAR_INTEGER | VLC_VAR_HASCHOICE );
>> text.psz_string = _("Video Track");
>> @@ -789,7 +797,9 @@ static int EsDelayCallback ( vlc_object_t *p_this, char
>> const *psz_cmd, }
>> else if( !strcmp( psz_cmd, "spu-delay" ) )
>> {
>> - input_ControlPush( p_input, INPUT_CONTROL_SET_SPU_DELAY, &newval );
>> + int isfilesub = var_GetInteger(p_input, "sub-isfilesub"); +
>> if ( ! isfilesub )
>> + input_ControlPush( p_input, INPUT_CONTROL_SET_SPU_DELAY,
>> &newval ); }
>> return VLC_SUCCESS;
>> }
>
> --
> Rémi Denis-Courmont
> http://www.remlab.net/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140609/eaef9d5c/attachment.html>
More information about the vlc-devel
mailing list