[vlc-devel] [PATCH 1/2] Navigation enhancement for language learning

Rémi Denis-Courmont remi at remlab.net
Wed Mar 18 23:05:54 CET 2015


Le mercredi 18 mars 2015, 00:53:07 Goldy Liang a écrit :
> ---
>  include/vlc_demux.h        |   6 ++
>  include/vlc_input.h        |   8 +++
>  modules/demux/subtitle.c   | 159
> +++++++++++++++++++++++++++++++++++++++++++++

If there is a need to patch the subtitle demuxer, then it might be preferable 
to split that to a third patch. However I am not sure patching demuxers is at 
all necessary anymore.

>  src/input/control.c        |  35 ++++++++++
>  src/input/event.c          |   7 ++
>  src/input/event.h          |   1 +
>  src/input/input.c          |   8 +++
>  src/input/input_internal.h |   3 +
>  8 files changed, 227 insertions(+)
> 
> diff --git a/include/vlc_demux.h b/include/vlc_demux.h
> index 93197ad..99d7d85 100644
> --- a/include/vlc_demux.h
> +++ b/include/vlc_demux.h
> @@ -183,6 +183,12 @@ enum demux_query_e
>      DEMUX_NAV_DOWN,            /* res=can fail */
>      DEMUX_NAV_LEFT,            /* res=can fail */
>      DEMUX_NAV_RIGHT,           /* res=can fail */
> +
> +    DEMUX_IS_SUBTITLE_SEEKABLE,
> +    DEMUX_GET_SUBTITLE_CUR_SENTENCE_TIME,

Did you try to save the start and end time stamps of the last and previous 
subtitles of the active text track directly in the elementary stream output 
(EsOutSend() in src/input/es_out.c)? You would not need to keep track of the 
active subtitle, and you would not need to extend demuxer-specific changes.

> +    DEMUX_GET_SUBTITLE_NEXT_SENTENCE_TIME,
> +    DEMUX_GET_SUBTITLE_PREV_SENTENCE_TIME,
> +
>  };
> 
>  VLC_API int demux_vaControlHelper( stream_t *, int64_t i_start, int64_t
> i_end, int64_t i_bitrate, int i_align, int i_query, va_list args );
> diff --git a/include/vlc_input.h b/include/vlc_input.h
> index c18bea6..a774acc 100644
> --- a/include/vlc_input.h
> +++ b/include/vlc_input.h
> @@ -392,6 +392,9 @@ typedef enum input_event_type_e
>      /* A vout_thread_t object has been created/deleted by *the input* */
>      INPUT_EVENT_VOUT,
> 
> +    /* A default subtitle data has been loaded */
> +    INPUT_EVENT_DEFAULT_SUBTITLE_LOADED,
> +
>  } input_event_type_e;
> 
>  /**
> @@ -478,6 +481,11 @@ enum input_query_e
>      /* External clock managments */
>      INPUT_GET_PCR_SYSTEM,   /* arg1=mtime_t *, arg2=mtime_t *       res=can
> fail */
>      INPUT_MODIFY_PCR_SYSTEM,/* arg1=int absolute, arg2=mtime_t      res=can
> fail */
> +
> +    INPUT_GET_CUR_SENTENCE_TIME,
> +    INPUT_GET_NEXT_SENTENCE_TIME,
> +    INPUT_GET_PREV_SENTENCE_TIME,
> +    INPUT_GET_SUBTITLE_SEEKABLE,
>  };
> 
>  /** @}*/
> diff --git a/modules/demux/subtitle.c b/modules/demux/subtitle.c
> index d8b6dc5..380242d 100644
> --- a/modules/demux/subtitle.c
> +++ b/modules/demux/subtitle.c
> @@ -226,6 +226,25 @@ static int Control( demux_t *, int, va_list );
>  static void Fix( demux_t * );
>  static char * get_language_from_filename( const char * );
> 
> +/*
> + * Get the subtitle sentence at a specific time
> + * @param i_subtitles total number of subtitles regarding to *subtitle
> + * @param *subtitle  pointer of array of the subtitle data
> + * @param i_time the specific time (in microsecond)
> + * @param i_subtitle return the number of subtitle sentence
> + *  -1 if before the first subtitle
> + *  0 ~ i_subtitles-1 if falls into the time
> + *      between the start of this sentence and the start of the next (or
> the end time)
> + * @param b_between return whether the time is between two sentence.
> + *   true if between the i_subtitle and the next one
> + *   false if not between, but falls in the time of i_subtitle.
> + */
> +static void getSubtitleAtTime (int i_subtitles,
> +                        subtitle_t  *subtitle,
> +                        int64_t i_time,
> +                        int * i_subtitle,
> +                        bool * b_between);
> +
>  /**************************************************************************
> *** * Module initializer
>  
> ***************************************************************************
> **/ @@ -590,6 +609,74 @@ static void Close( vlc_object_t *p_this )
>      free( p_sys );
>  }
> 
> +/*
> + * Get the subtitle sentence at a specific time
> + * @param i_subtitles total number of subtitles regarding to *subtitle
> + * @param *subtitle  pointer of array of the subtitle data
> + * @param i_time the specific time (in microsecond)
> + * @param i_subtitle return the number of subtitle sentence
> + *  -1 if before the first subtitle
> + *  0 ~ i_subtitles-1 if falls into the time
> + *      between the start of this sentence and the start of the next (or
> the end time)
> + * @param b_between return whether the time is between two sentence.
> + *   true if between the i_subtitle and the next one
> + *   false if not between, but falls in the time of i_subtitle.
> + */
> +static void getSubtitleAtTime (int i_subtitles,
> +                        subtitle_t  *subtitle,
> +                        int64_t i_time,
> +                        int * i_subtitle,
> +                        bool * b_between)
> +{
> +    if (!subtitle || i_subtitles<=0)
> +    {
> +        *i_subtitle = -1; *b_between = false;
> +        return;
> +    }
> +
> +    // binary search
> +    int ll=0, rr=i_subtitles-1;
> +
> +    while (ll<=rr)
> +    {
> +        int mm= (ll+rr) / 2;
> +
> +        if ( i_time < subtitle[mm].i_start)
> +        {
> +            // if the timestamp is before the start of the middle node
> +            // continue the search in the lower half
> +            rr = mm-1;
> +        } else if ( mm<rr && i_time >= subtitle[mm+1].i_start)
> +        {
> +            // if the middle node is not the right node, and the timestamp
> +            // is after the start of the node next to the middle
> +            // continue the search in the higher half
> +            ll = mm+1;
> +
> +        } else
> +        {
> +            // otherwise the timestamp is between
> +            // the start of #mm and the start of #mm+1
> +            *i_subtitle = mm;
> +            *b_between = i_time > subtitle[mm].i_stop;
> +            return;
> +        }
> +    }
> +
> +    if (rr<0)
> +    {
> +        // timestamp before the first subtitle sentence
> +        *i_subtitle = -1; *b_between = true;
> +    }
> +    else
> +    {
> +        //should not be here
> +        *i_subtitle = -1; *b_between = true;
> +    }

Did you try to use the built-in bsearch()?

> +}
> +
> +
> +
>  /**************************************************************************
> *** * Control:
>  
> ***************************************************************************
> **/ @@ -678,6 +765,78 @@ static int Control( demux_t *p_demux, int i_query,
> va_list args )
>          case DEMUX_CAN_RECORD:
>              return VLC_EGENERIC;
> 
> +        case DEMUX_IS_SUBTITLE_SEEKABLE:
> +        {
> +            bool *b = (bool*) va_arg (args, bool*);
> +            *b = true;
> +            return VLC_SUCCESS;
> +        }
> +        case DEMUX_GET_SUBTITLE_CUR_SENTENCE_TIME:
> +        {
> +            int64_t* pstart = (int64_t*)va_arg( args, int64_t * );
> +            int64_t* pend   = (int64_t*)va_arg( args, int64_t * );
> +
> +            bool b;
> +
> +            int64_t i_time = var_GetTime( p_demux->p_input, "time" );
> +            int i_subtitle;

In principles, p_demux->p_input can be NULL, so you should not blindly assume 
you can call var_GetTime() here. More generally, I may sound like a broken 
record, but it is not advisable to manipulate the input from the demuxer, not 
even as read-only.
If nothing else, you could probably call var_GetTime() in input.c and pass the 
result as a parameter.

That said, if at all possible, I think patching the ES output is even better.

> +
> +            getSubtitleAtTime (p_sys->i_subtitles,
> +                    p_sys->subtitle, i_time, &i_subtitle, &b);
> +            *pstart = p_sys->subtitle[i_subtitle].i_start;
> +            *pend   = p_sys->subtitle[i_subtitle].i_stop;
> +
> +            return VLC_SUCCESS;
> +        }
> +
> +        case DEMUX_GET_SUBTITLE_NEXT_SENTENCE_TIME:
> +        {
> +            int64_t* pstart = (int64_t*)va_arg( args, int64_t * );
> +            int64_t* pend   = (int64_t*)va_arg( args, int64_t * );
> +
> +            bool b;
> +
> +            int64_t i_time = var_GetTime( p_demux->p_input, "time" );

Ditto.

> +            int i_subtitle;
> +
> +            getSubtitleAtTime (p_sys->i_subtitles,
> +                    p_sys->subtitle, i_time, &i_subtitle, &b);
> +
> +            if (i_subtitle < p_sys->i_subtitles - 1)
> +                i_subtitle ++;
> +
> +            *pstart = p_sys->subtitle[i_subtitle].i_start;
> +            *pend   = p_sys->subtitle[i_subtitle].i_stop;
> +
> +            return VLC_SUCCESS;
> +        }
> +
> +        case DEMUX_GET_SUBTITLE_PREV_SENTENCE_TIME:
> +        {
> +            int64_t* pstart = (int64_t*)va_arg( args, int64_t * );
> +            int64_t* pend   = (int64_t*)va_arg( args, int64_t * );
> +
> +            bool b;
> +
> +            int64_t i_time = var_GetTime( p_demux->p_input, "time" );

Ditto.

> +            int i_subtitle;
> +
> +            getSubtitleAtTime (p_sys->i_subtitles,
> +                    p_sys->subtitle, i_time, &i_subtitle, &b);
> +
> +            /* If currently between two sentence (b=true),
> +             *    just jump to the "current" sentence before the break;
> +             * Otherwise
> +             *    jump to the sentence before
> +            */
> +            if (i_subtitle > 0 && !b)
> +                i_subtitle --;
> +
> +            *pstart = p_sys->subtitle[i_subtitle].i_start;
> +            *pend   = p_sys->subtitle[i_subtitle].i_stop;
> +
> +            return VLC_SUCCESS;
> +        }
>          default:
>              msg_Err( p_demux, "unknown query %d in subtitle control",
> i_query );
>              return VLC_EGENERIC;
> diff --git a/src/input/control.c b/src/input/control.c
> index 2e4633f..c8296a0 100644
> --- a/src/input/control.c
> +++ b/src/input/control.c
> @@ -35,6 +35,7 @@
>  #include "event.h"
>  #include "resource.h"
>  #include "es_out.h"
> +#include "demux.h"
> 
> 
>  static void UpdateBookmarksOption( input_thread_t * );
> @@ -494,6 +495,40 @@ int input_vaControl( input_thread_t *p_input, int
> i_query, va_list args )
>              return es_out_ControlModifyPcrSystem( p_input->p-
> 
> >p_es_out_display, b_absolute, i_system );
> 
>          }
> 
> +        case INPUT_GET_CUR_SENTENCE_TIME:
> +        case INPUT_GET_NEXT_SENTENCE_TIME:
> +        case INPUT_GET_PREV_SENTENCE_TIME:
> +        {
> +           input_source_t * p_sub = p_input->p->p_def_sub;

p_def_sub is written in the input thread. You cannot blindly read it from 
another thread.

> +
> +           if (!p_sub)
> +               return VLC_EGENERIC;
> +
> +            int64_t* pstart = (int64_t*)va_arg( args, int64_t * );
> +            int64_t* pend = (int64_t*)va_arg( args, int64_t * );
> +
> +            if (!demux_Control (p_sub->p_demux ,
> +                    DEMUX_GET_SUBTITLE_CUR_SENTENCE_TIME +
> +                    i_query - INPUT_GET_CUR_SENTENCE_TIME,
> +                    pstart, pend))
> +                return VLC_SUCCESS;
> +            else
> +                return VLC_EGENERIC;

This assumes that the subtitle demux is re-entrant - which it is not.

Compare INPUT_ADD_SUBTITLE / INPUT_CONTROL_ADD_SUBTITLE,

> +        }
> +        case INPUT_GET_SUBTITLE_SEEKABLE:
> +        {
> +            bool * p_seekable = (bool*)va_arg( args, bool * );
> +            input_source_t * p_sub = p_input->p->p_def_sub;
> +
> +            *p_seekable = false;
> +
> +            if (p_sub)
> +                demux_Control (p_sub->p_demux,
> +                               DEMUX_IS_SUBTITLE_SEEKABLE,
> +                               p_seekable);
> +
> +            return VLC_SUCCESS;
> +        }

Ditto.

>          default:
>              msg_Err( p_input, "unknown query in input_vaControl" );
>              return VLC_EGENERIC;
> diff --git a/src/input/event.c b/src/input/event.c
> index e32ca8a..f887021 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 default subtitle loaded */
> +void input_SendEventDefSubLoaded( input_thread_t *p_input )
> +{
> +    Trigger( p_input, INPUT_EVENT_DEFAULT_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..4a63661 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_SendEventDefSubLoaded( 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..2c2a55a 100644
> --- a/src/input/input.c
> +++ b/src/input/input.c
> @@ -369,6 +369,9 @@ static input_thread_t *Create( vlc_object_t *p_parent,
> input_item_t *p_item,
>      /* No slave */
>      p_input->p->i_slave = 0;
>      p_input->p->slave   = NULL;
> +
> +    /* No default subtitle */
> +    p_input->p->p_def_sub = NULL;
> 
>      /* */
>      if( p_resource )
> @@ -2982,6 +2985,11 @@ static void input_SubtitleAdd( input_thread_t
> *p_input,
> 
>          es_out_Control( p_input->p->p_es_out_display,
> ES_OUT_SET_ES_DEFAULT_BY_ID, i_id );
>          es_out_Control( p_input->p->p_es_out_display, ES_OUT_SET_ES_BY_ID,
> i_id );
> +
> +        /* set default subtitle input source and
> +         * send the notification event*/
> +        p_input->p->p_def_sub = sub;
> +        input_SendEventDefSubLoaded(p_input);
>      }
>      var_FreeList( &list, NULL );
>  }
> diff --git a/src/input/input_internal.h b/src/input/input_internal.h
> index fa8c2b1..98fca02 100644
> --- a/src/input/input_internal.h
> +++ b/src/input/input_internal.h
> @@ -129,6 +129,9 @@ struct input_thread_private_t
>      int            i_slave;
>      input_source_t **slave;
> 
> +    /* Default subtitle input source */
> +    input_source_t *p_def_sub;
> +
>      /* Resources */
>      input_resource_t *p_resource;
>      input_resource_t *p_resource_private;

-- 
Rémi Denis-Courmont
http://www.remlab.net/




More information about the vlc-devel mailing list