[FFmpeg-devel] [PATCH] lavf/matroskadec: work around additional broken index cases

[wm4]

On Sun, 23 Aug 2015 02:47:39 -0500
Rodger Combs <rodger.combs at gmail.com> wrote:

> ---
>  libavformat/matroskadec.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index f97ae16..7f4a52b 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1520,8 +1520,10 @@ static void matroska_add_index_entries(MatroskaDemuxContext *matroska)
>  
>      index_list = &matroska->index;
>      index      = index_list->elem;
> -    if (index_list->nb_elem &&
> -        index[0].time > 1E14 / matroska->time_scale) {
> +    if ((index_list->nb_elem &&
> +         index[0].time > 1E14 / matroska->time_scale) ||
> +        (index_list->nb_elem > 1 &&
> +         index[1].time > 1E14 / matroska->time_scale)) {
>          av_log(matroska->ctx, AV_LOG_WARNING, "Working around broken index.\n");
>          index_scale = matroska->time_scale;
>      }

I looked at the sample file you gave me privately. The index was indeed
very broken. Even with this work around, it had only 4 quite useless
entries. There was also a legacy index (a SeekHead element linking all
cluster elements; useless for seeking). The header identified the muxer
as:

| + Muxing application: libmkv 0.6.1.2 at 2367
| + Writing application: HandBrake 0.9.1 at 2384

I'm not sure if this really should be worked around. We'd get better
results by ignoring the index completely. Now the question is if there
are files that are broken in this way, but actually give good results
with the workaround applied. (The original workaround goes back to
8f569ed08faa39414ebd65c2e530a87dc9007d5b, referencing an issue on an
older bug tracker.)

Anyway, personally I'd prefer to just drop the workaround, and throw
away the index if it looks broken. Any other opinions?

If we want to accept this workaround, I think it could be simplified.
Throw away the index if it has less than 2 entries. Check only the 2nd
entry (entries are not necessarily sorted, but checking the 2nd entry
is going to catch most or all cases). Well, just a suggestion.