[FFmpeg-devel] [PATCH] matroska read_seek: make max diff for last known subtitle configurable

[wm4]

On Thu, 24 Nov 2016 12:02:45 +0100
Rainer Hochecker <fernetmenta at kodi.tv> wrote:

> From: Rainer Hochecker <fernetmenta at online.de>
> 
> Allow players to reduce the time read_seek sets the file pos before desired position.
> Hard coded 30 seconds is a lot and takes several seconds when the source is on
> a network share.
> 
> 
> Signed-off-by: Rainer Hochecker <fernetmenta at online.de>
> ---
>  libavformat/matroskadec.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index f79511e..eb2bc31 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -353,6 +353,9 @@ typedef struct MatroskaDemuxContext {
>  
>      /* WebM DASH Manifest live flag/ */
>      int is_live;
> +
> +    /* set file pos up to 30 seconds before desired timestamp on seek */
> +    int subtitle_seek_diff;
>  } MatroskaDemuxContext;
>  
>  typedef struct MatroskaBlock {
> @@ -3407,7 +3410,7 @@ static int matroska_read_seek(AVFormatContext *s, int stream_index,
>              while (index_sub >= 0 &&
>                    index_min > 0 &&
>                    tracks[i].stream->index_entries[index_sub].pos < st->index_entries[index_min].pos &&
> -                  st->index_entries[index].timestamp - tracks[i].stream->index_entries[index_sub].timestamp < 30000000000 / matroska->time_scale)
> +                  st->index_entries[index].timestamp - tracks[i].stream->index_entries[index_sub].timestamp < matroska->subtitle_seek_diff * 1E9 / matroska->time_scale)
>                  index_min--;
>          }
>      }
> @@ -3813,9 +3816,17 @@ static int webm_dash_manifest_read_packet(AVFormatContext *s, AVPacket *pkt)
>  #define OFFSET(x) offsetof(MatroskaDemuxContext, x)
>  static const AVOption options[] = {
>      { "live", "flag indicating that the input is a live file that only has the headers.", OFFSET(is_live), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
> +    { "subtitle_seek_diff", "max difference in seconds between desired seek pos and last known subtitle pos.", OFFSET(subtitle_seek_diff), AV_OPT_TYPE_INT, {.i64 = 30}, 0, 30, AV_OPT_FLAG_DECODING_PARAM },
>      { NULL },
>  };
>  
> +static const AVClass matroska_class = {
> +    .class_name = "matroska,webm demuxer",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
>  static const AVClass webm_dash_class = {
>      .class_name = "WebM DASH Manifest demuxer",
>      .item_name  = av_default_item_name,
> @@ -3828,6 +3839,7 @@ AVInputFormat ff_matroska_demuxer = {
>      .long_name      = NULL_IF_CONFIG_SMALL("Matroska / WebM"),
>      .extensions     = "mkv,mk3d,mka,mks",
>      .priv_data_size = sizeof(MatroskaDemuxContext),
> +    .priv_class     = &matroska_class,
>      .read_probe     = matroska_probe,
>      .read_header    = matroska_read_header,
>      .read_packet    = matroska_read_packet,

Adding an option doesn't seem like a good approach to fix this.

This code isn't needed in the first place. Apparently it's for getting
subtitle packets which start before the seek target, but overlap with
the seek target. It only works for parts of the file which libavformat
has "indexed" (i.e. useless if you seek forward in a normal file).

As such it seems pretty useless. But there are two other things which
make it even worse:
- You could achieve the same thing outside of the API
  by seeking a bit before, and then skip data
- You could implement support for the new cue index elements, which
  were specifically designed to solve this problem

So I'd say just delete this code, instead of adding a new private
option that only 1 API user or so will use.