[vlc-devel] [PATCH] Navigation based on subtitle data (next/prev sentence, ...
Rémi Denis-Courmont
remi at remlab.net
Thu Mar 5 21:25:28 CET 2015
Hello,
Comments inline...
Le jeudi 05 mars 2015, 21:57:59 Rémi Denis-Courmont a écrit :
> Navigation based on subtitle data (next/prev sentence,
> repeat sentence) New profile "Language learning style" for customize
> interface Added slower/faster fine buttons Handle interaction between AtoB
> and sentence repeat
Please stick to 72 characters for the summary.
> More detail:
details
> - Escalated the subtitle data structure into vlc_subtitle.h
> - Added a field in the input thread internal data to point to the
> main subtitle data
> - Introduced API to set/reset/get the main subtitle data
> - Subtitle demux module invoked set to expose the subtitle data
> Invoked reset if subtitle unloaded (module closed)
> - qt4 interface module invoked get to access subtitle
> - qt4 interface enhanced to navigate via subtitle data, including
> * prev/next sentence
> * repeat current sentence
UI changes should be a separate patch.
> - Menu enhanced to add related new entries
> - Toolbar buttons added for related functions
> - A new profile of customize interface named "Language learning style"
> - Added slower/faster fine buttons
> - Interaction between AtoB and sentence repeat mode
> ---
> diff --git a/include/vlc_input.h b/include/vlc_input.h
> index c18bea6..0cc1685 100644
> --- a/include/vlc_input.h
> +++ b/include/vlc_input.h
> @@ -37,6 +37,7 @@
> #include <vlc_events.h>
> #include <vlc_input_item.h>
> #include <vlc_vout_osd.h>
> +#include <vlc_subtitle.h>
Please don't worsen header headers inclusion creep.
You can use an unspecified struct type here, e.g.:
struct vlc_subtitles_t;
>
> #include <string.h>
>
> @@ -392,6 +393,9 @@ typedef enum input_event_type_e
> /* A vout_thread_t object has been created/deleted by *the input* */
> INPUT_EVENT_VOUT,
>
> + /* A main subtitle data has been loaded */
> + INPUT_EVENT_MAIN_SUBTITLE_LOADED,
> +
> } input_event_type_e;
>
> /**
> @@ -571,6 +575,38 @@ static inline int input_AddSubtitleOSD( input_thread_t
> *p_input, const char *psz
> }
> #define input_AddSubtitle(a, b, c) input_AddSubtitleOSD(a, b, c, false)
>
> +/**
> + * Set the main subtitle data which can be accessed by other
> + * modules later on. Only the first subtitle passed by this API
> + * will be set as the main, all later subtitles will be ignored.
> + * Should be called by the subtitle demux module(s).
> + * @param p_input an input thread
> + * @param p_sub the subtitle data pointer to be set
> + * @return true if the main subtitle is set as p_sub, false
> + * if main subtitle is already set to others prior to this call
> + */
> +VLC_API bool input_SetMainSubtitle (input_thread_t *p_input,
> + vlc_subtitles_t * p_sub);
It's a bit odd for a demuxer to call a function on the input thread because it
breaks encapsulation. It also brings concurrency and threading problems.
Normally, the input thread controls the demuxer, not the other way around. It
seems you want to seek to the previous/current/next subtitle start time. So
you can just add demux.pf_control requests to retrieve the time from the
subtitle demux.
For instance, in the input thread call:
demux_Control(..., DEMUX_GET_PREV_TEXT_TIME, &pts);
or
demux_Control(..., DEMUX_GET_CUR_TEXT_TIME, &pts);
or
demux_Control(..., DEMUX_GET_NEXT_TEXT_TIME, &pts);
then seek to pts, and in the demuxer:
Control(...)
{
...
case DEMUX_GET_PREV_TEXT_TIME:
*va_arg(ap, mtime_t *) = time_of_previous_sub;
...
}
Or something along those lines anyway.
This also avoids exposing the subtitle demuxer internals to the GUI.
> +/**
> + * Reset (clear) the main subtitle data in an input thread.
> + * The main subtitle is cleared only if the provided subtitle
> + * data is the same one that was set before.
> + * Should be called by the subtitle demux module(s).
> + * @param p_input an input thread
> + * @param p_sub the subtitle data pointer to be reset
> + * @return true if the subtitle is cleared, false if not cleared
> + * because the provided subtitle is not the same one.
> + */
> +VLC_API bool input_ResetMainSubtitle (input_thread_t *p_input,
> + vlc_subtitles_t * p_sub);
> +
> +/**
> + * Get the main subtitle data from an input thread
> + * @param p_input an input thread
> + * @return the pointer to the subtitle data, or NULL if the main
> + * subtitle not already set.
> + */
> +VLC_API vlc_subtitles_t * input_GetMainSubtitle (input_thread_t *p_input);
>
> /**
> * Return the audio output (if any) associated with an input.
> diff --git a/include/vlc_subtitle.h b/include/vlc_subtitle.h
> new file mode 100644
> index 0000000..b1d5c0e
> --- /dev/null
> +++ b/include/vlc_subtitle.h
> @@ -0,0 +1,43 @@
> +/**************************************************************************
> *** + * vlc_subtitle.h: Global types for subtitle data
> +
> ****************************************************************************
> * + * Copyright (C) 1998-2009 VLC authors and VideoLAN
Wrong years.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or + *
> (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA. +
> ****************************************************************************
> */ +
> +/**
> + * \file
> + * This file provides type definition for preloaded subtitle data
> + */
> +
> +#ifndef VLC_SUBTITLE_H
> +# define VLC_SUBTITLE_H 1
> +
> +typedef struct
> +{
> + int64_t i_start;
> + int64_t i_stop;
> +
> + char *psz_text;
> +} subtitle_t;
> +
> +typedef struct
> +{
> + int i_subtitles;
> + subtitle_t *subtitle;
> +} vlc_subtitles_t;
> +
> +#endif /* !VLC_SUBTITLE_H */
> diff --git a/modules/demux/subtitle.c b/modules/demux/subtitle.c
> index d8b6dc5..788371f 100644
> --- a/modules/demux/subtitle.c
> +++ b/modules/demux/subtitle.c
> @@ -42,6 +42,8 @@
> #include <vlc_demux.h>
> #include <vlc_charset.h>
>
> +#include <vlc_input.h>
> +
> /**************************************************************************
> *** * Module descriptor
>
> ***************************************************************************
> **/ @@ -126,15 +128,6 @@ typedef struct
> static int TextLoad( text_t *, stream_t *s );
> static void TextUnload( text_t * );
>
> -typedef struct
> -{
> - int64_t i_start;
> - int64_t i_stop;
> -
> - char *psz_text;
> -} subtitle_t;
> -
> -
> struct demux_sys_t
> {
> int i_type;
> @@ -146,8 +139,8 @@ struct demux_sys_t
>
> char *psz_header;
> int i_subtitle;
> - int i_subtitles;
> - subtitle_t *subtitle;
> +
> + vlc_subtitles_t sub;
>
> int64_t i_length;
>
> @@ -253,8 +246,8 @@ static int Open ( vlc_object_t *p_this )
>
> p_sys->psz_header = NULL;
> p_sys->i_subtitle = 0;
> - p_sys->i_subtitles = 0;
> - p_sys->subtitle = NULL;
> + p_sys->sub.i_subtitles = 0;
> + p_sys->sub.subtitle = NULL;
> p_sys->i_microsecperframe = 40000;
>
> p_sys->jss.b_inited = false;
> @@ -500,10 +493,10 @@ static int Open ( vlc_object_t *p_this )
> /* Parse it */
> for( i_max = 0;; )
> {
> - if( p_sys->i_subtitles >= i_max )
> + if( p_sys->sub.i_subtitles >= i_max )
> {
> i_max += 500;
> - if( !( p_sys->subtitle = realloc_or_free( p_sys->subtitle,
> + if( !( p_sys->sub.subtitle = realloc_or_free( p_sys-
> >sub.subtitle,
> sizeof(subtitle_t) * i_max )
> ) )
> {
> TextUnload( &p_sys->txt );
> @@ -512,26 +505,26 @@ static int Open ( vlc_object_t *p_this )
> }
> }
>
> - if( pf_read( p_demux, &p_sys->subtitle[p_sys->i_subtitles],
> - p_sys->i_subtitles ) )
> + if( pf_read( p_demux, &p_sys->sub.subtitle[p_sys->sub.i_subtitles],
> + p_sys->sub.i_subtitles ) )
> break;
>
> - p_sys->i_subtitles++;
> + p_sys->sub.i_subtitles++;
> }
> /* Unload */
> TextUnload( &p_sys->txt );
>
> - msg_Dbg(p_demux, "loaded %d subtitles", p_sys->i_subtitles );
> + msg_Dbg(p_demux, "loaded %d subtitles", p_sys->sub.i_subtitles );
>
> /* Fix subtitle (order and time) *** */
> p_sys->i_subtitle = 0;
> p_sys->i_length = 0;
> - if( p_sys->i_subtitles > 0 )
> + if( p_sys->sub.i_subtitles > 0 )
> {
> - p_sys->i_length = p_sys->subtitle[p_sys->i_subtitles-1].i_stop;
> + p_sys->i_length = p_sys->sub.subtitle[p_sys-
>
> >sub.i_subtitles-1].i_stop;
>
> /* +1 to avoid 0 */
> if( p_sys->i_length <= 0 )
> - p_sys->i_length = p_sys->subtitle[p_sys-
>
> >i_subtitles-1].i_start+1;
>
> + p_sys->i_length = p_sys->sub.subtitle[p_sys-
>
> >sub.i_subtitles-1].i_start+1;
>
> }
>
> /* *** add subtitle ES *** */
> @@ -570,6 +563,11 @@ static int Open ( vlc_object_t *p_this )
> p_sys->es = es_out_Add( p_demux->out, &fmt );
> es_format_Clean( &fmt );
>
> + // set the main subtitle data so that other
> + // modules can access
> + input_SetMainSubtitle (p_demux->p_input, &p_sys->sub);
> +
> +
> return VLC_SUCCESS;
> }
>
> @@ -582,9 +580,12 @@ static void Close( vlc_object_t *p_this )
> demux_sys_t *p_sys = p_demux->p_sys;
> int i;
>
> - for( i = 0; i < p_sys->i_subtitles; i++ )
> - free( p_sys->subtitle[i].psz_text );
> - free( p_sys->subtitle );
> + // reset the main subtitle data in the input thread
> + input_ResetMainSubtitle (p_demux->p_input, &p_sys->sub);
> +
> + for( i = 0; i < p_sys->sub.i_subtitles; i++ )
> + free( p_sys->sub.subtitle[i].psz_text );
> + free( p_sys->sub.subtitle );
> free( p_sys->psz_header );
>
> free( p_sys );
> @@ -608,9 +609,9 @@ static int Control( demux_t *p_demux, int i_query,
> va_list args )
>
> case DEMUX_GET_TIME:
> pi64 = (int64_t*)va_arg( args, int64_t * );
> - if( p_sys->i_subtitle < p_sys->i_subtitles )
> + if( p_sys->i_subtitle < p_sys->sub.i_subtitles )
> {
> - *pi64 = p_sys->subtitle[p_sys->i_subtitle].i_start;
> + *pi64 = p_sys->sub.subtitle[p_sys->i_subtitle].i_start;
> return VLC_SUCCESS;
> }
> return VLC_EGENERIC;
> @@ -618,9 +619,9 @@ static int Control( demux_t *p_demux, int i_query,
> va_list args )
> 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 )
> + while( p_sys->i_subtitle < p_sys->sub.i_subtitles )
> {
> - const subtitle_t *p_subtitle = &p_sys->subtitle[p_sys-
>
> >i_subtitle];
>
> + const subtitle_t *p_subtitle = &p_sys->sub.subtitle[p_sys-
>
> >i_subtitle];
>
> if( p_subtitle->i_start > i64 )
> break;
> @@ -630,19 +631,19 @@ static int Control( demux_t *p_demux, int i_query,
> va_list args )
> p_sys->i_subtitle++;
> }
>
> - if( p_sys->i_subtitle >= p_sys->i_subtitles )
> + if( p_sys->i_subtitle >= p_sys->sub.i_subtitles )
> return VLC_EGENERIC;
> return VLC_SUCCESS;
>
> case DEMUX_GET_POSITION:
> pf = (double*)va_arg( args, double * );
> - if( p_sys->i_subtitle >= p_sys->i_subtitles )
> + if( p_sys->i_subtitle >= p_sys->sub.i_subtitles )
> {
> *pf = 1.0;
> }
> - else if( p_sys->i_subtitles > 0 )
> + else if( p_sys->sub.i_subtitles > 0 )
> {
> - *pf = (double)p_sys->subtitle[p_sys->i_subtitle].i_start /
> + *pf =
> (double)p_sys->sub.subtitle[p_sys->i_subtitle].i_start /
> (double)p_sys->i_length;
> }
> else
> @@ -656,12 +657,12 @@ static int Control( demux_t *p_demux, int i_query,
> va_list args )
> i64 = f * p_sys->i_length;
>
> p_sys->i_subtitle = 0;
> - while( p_sys->i_subtitle < p_sys->i_subtitles &&
> - p_sys->subtitle[p_sys->i_subtitle].i_start < i64 )
> + while( p_sys->i_subtitle < p_sys->sub.i_subtitles &&
> + p_sys->sub.subtitle[p_sys->i_subtitle].i_start < i64 )
> {
> p_sys->i_subtitle++;
> }
> - if( p_sys->i_subtitle >= p_sys->i_subtitles )
> + if( p_sys->i_subtitle >= p_sys->sub.i_subtitles )
> return VLC_EGENERIC;
> return VLC_SUCCESS;
>
> @@ -692,20 +693,20 @@ static int Demux( demux_t *p_demux )
> demux_sys_t *p_sys = p_demux->p_sys;
> int64_t i_maxdate;
>
> - if( p_sys->i_subtitle >= p_sys->i_subtitles )
> + if( p_sys->i_subtitle >= p_sys->sub.i_subtitles )
> return 0;
>
> i_maxdate = p_sys->i_next_demux_date - var_GetTime( p_demux->p_parent,
> "spu-delay" );;
> - if( i_maxdate <= 0 && p_sys->i_subtitle < p_sys->i_subtitles )
> + if( i_maxdate <= 0 && p_sys->i_subtitle < p_sys->sub.i_subtitles )
> {
> /* Should not happen */
> - i_maxdate = p_sys->subtitle[p_sys->i_subtitle].i_start + 1;
> + i_maxdate = p_sys->sub.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 )
> + while( p_sys->i_subtitle < p_sys->sub.i_subtitles &&
> + p_sys->sub.subtitle[p_sys->i_subtitle].i_start < i_maxdate )
> {
> - const subtitle_t *p_subtitle = &p_sys->subtitle[p_sys->i_subtitle];
> + const subtitle_t *p_subtitle = &p_sys->sub.subtitle[p_sys-
> >i_subtitle];
>
> block_t *p_block;
> int i_len = strlen( p_subtitle->psz_text ) + 1;
> @@ -755,19 +756,19 @@ static void Fix( demux_t *p_demux )
> do
> {
> b_done = true;
> - for( int i_index = 1; i_index < p_sys->i_subtitles; i_index++ )
> + for( int i_index = 1; i_index < p_sys->sub.i_subtitles; i_index++ )
> {
> - if( p_sys->subtitle[i_index].i_start <
> - p_sys->subtitle[i_index - 1].i_start )
> + if( p_sys->sub.subtitle[i_index].i_start <
> + p_sys->sub.subtitle[i_index - 1].i_start )
> {
> subtitle_t sub_xch;
> memcpy( &sub_xch,
> - p_sys->subtitle + i_index - 1,
> + p_sys->sub.subtitle + i_index - 1,
> sizeof( subtitle_t ) );
> - memcpy( p_sys->subtitle + i_index - 1,
> - p_sys->subtitle + i_index,
> + memcpy( p_sys->sub.subtitle + i_index - 1,
> + p_sys->sub.subtitle + i_index,
> sizeof( subtitle_t ) );
> - memcpy( p_sys->subtitle + i_index,
> + memcpy( p_sys->sub.subtitle + i_index,
> &sub_xch,
> sizeof( subtitle_t ) );
> b_done = false;
> diff --git a/src/input/event.c b/src/input/event.c
> index e32ca8a..e49b535 100644
> --- a/src/input/event.c
> +++ b/src/input/event.c
> @@ -191,6 +191,13 @@ void input_SendEventCache( input_thread_t *p_input,
> double f_level )
> Trigger( p_input, INPUT_EVENT_CACHE );
> }
>
> +/* Send a signal notifying a main subtitle loaded */
> +void input_SendEventMainSubLoaded( input_thread_t *p_input )
> +{
> + Trigger( p_input, INPUT_EVENT_MAIN_SUBTITLE_LOADED);
> +}
> +
> +
> /* FIXME: review them because vlc_event_send might be
> * moved inside input_item* functions.
> */
> diff --git a/src/input/event.h b/src/input/event.h
> index e91ad11..b176d5b 100644
> --- a/src/input/event.h
> +++ b/src/input/event.h
> @@ -42,6 +42,7 @@ void input_SendEventSeekpoint( input_thread_t *p_input,
> int i_title, int i_seekp
> void input_SendEventSignal( input_thread_t *p_input, double f_quality,
> double f_strength );
> void input_SendEventState( input_thread_t *p_input, int i_state );
> void input_SendEventCache( input_thread_t *p_input, double f_level );
> +void input_SendEventMainSubLoaded( input_thread_t *p_input );
>
> /* TODO rename Item* */
> void input_SendEventMeta( input_thread_t *p_input );
> diff --git a/src/input/input.c b/src/input/input.c
> index 7de30ec..11376c8 100644
> --- a/src/input/input.c
> +++ b/src/input/input.c
> @@ -359,6 +359,7 @@ static input_thread_t *Create( vlc_object_t *p_parent,
> input_item_t *p_item,
> p_input->p->input.b_can_rate_control = true;
> p_input->p->input.b_rescale_ts = true;
> p_input->p->input.b_eof = false;
> + p_input->p->p_sub = NULL;
>
> vlc_mutex_lock( &p_item->lock );
>
> @@ -3092,3 +3093,61 @@ char *input_CreateFilename( input_thread_t *input,
> const char *psz_path, const c
> return psz_file;
> }
> }
> +
> +/**
> + * Set the main subtitle data which can be accessed by other
> + * modules later on. Only the first subtitle passed by this API
> + * will be set as the main, all later subtitles will be ignored.
> + * Should be called by the subtitle demux module(s).
> + * @param p_input an input thread
> + * @param p_sub the subtitle data pointer to be set
> + * @return true if the main subtitle is set as p_sub, false
> + * if main subtitle is already set to others prior to this call
> + */
> +VLC_API bool input_SetMainSubtitle (input_thread_t *p_input,
> + vlc_subtitles_t * p_sub)
> +{
> + if (p_input && !p_input->p->p_sub)
> + {
> + p_input->p->p_sub = p_sub;
This is racy. The GUI and the demuxer run in different threads.
> +
> + input_SendEventMainSubLoaded(p_input);
> +
> + return true;
> + } else return false;
> +}
> +
> +/**
> + * Reset (clear) the main subtitle data in an input thread.
> + * The main subtitle is cleared only if the provided subtitle
> + * data is the same one that was set before.
> + * Should be called by the subtitle demux module(s).
> + * @param p_input an input thread
> + * @param p_sub the subtitle data pointer to be reset
> + * @return true if the subtitle is cleared, false if not cleared
> + * because the provided subtitle is not the same one.
> + */
> +VLC_API bool input_ResetMainSubtitle (input_thread_t *p_input,
> + vlc_subtitles_t * p_sub)
> +{
> + if (p_input && p_input->p->p_sub == p_sub)
> + {
> + p_input->p->p_sub = NULL;
> + return true;
> + } else return false;
> +}
> +
> +/**
> + * Get the main subtitle data from an input thread
> + * @param p_input an input thread
> + * @return the pointer to the subtitle data, or NULL if the main
> + * subtitle not already set.
> + */
> +vlc_subtitles_t * input_GetMainSubtitle (input_thread_t *p_input)
> +{
> + if (p_input && p_input->p)
> + return p_input->p->p_sub;
Ditto. If the demuxer terminates, the pointer will become invalid and the GUI
will crash.
> + else
> + return NULL;
> +}
> +
> diff --git a/src/input/input_internal.h b/src/input/input_internal.h
> index fa8c2b1..a53b2e2 100644
> --- a/src/input/input_internal.h
> +++ b/src/input/input_internal.h
> @@ -161,6 +161,9 @@ struct input_thread_private_t
> int i_control;
> input_control_t control[INPUT_CONTROL_FIFO_SIZE];
>
> + /* Main subtitle for navigation purpose */
> + vlc_subtitles_t *p_sub;
> +
> bool is_running;
> vlc_thread_t thread;
> };
> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
> index 7f06ed6..cb2f2d0 100644
> --- a/src/libvlccore.sym
> +++ b/src/libvlccore.sym
> @@ -180,6 +180,9 @@ input_DecoderDecode
> input_DecoderDelete
> input_DecoderCreate
> input_GetItem
> +input_SetMainSubtitle
> +input_ResetMainSubtitle
> +input_GetMainSubtitle
> input_item_AddInfo
> input_item_AddOption
> input_item_Copy
--
Rémi Denis-Courmont
http://www.remlab.net/
More information about the vlc-devel
mailing list