[FFmpeg-devel] [PATCH] Fix incorrect detection of badly interleaved avi

[Nigel Touati-Evans]

On 4 July 2013 19:53, Michael Niedermayer <michaelni at gmx.at> wrote:

> On Thu, Jul 04, 2013 at 04:51:32PM +0100, Nigel Touati-Evans wrote:
> > The method guess_ni_flag needs to divide timestamps in the index
> > by sample_size if it is set in order to compare different streams
> correctly.
>
> commit message should start with "avidec:" or something like that
>
>
> > diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> > index f5c2345..a09bebd 100644
> > --- a/libavformat/avidec.c
> > +++ b/libavformat/avidec.c
> > @@ -1397,15 +1397,16 @@ static int guess_ni_flag(AVFormatContext *s){
> >
> >          for (i=0; i<s->nb_streams; i++) {
> >              AVStream *st = s->streams[i];
> > +            AVIStream *ast = st->priv_data;
> >              int n= st->nb_index_entries;
> >              while (idx[i]<n && st->index_entries[idx[i]].pos < pos)
> >                  idx[i]++;
> >              if (idx[i] < n) {
> > -                min_dts = FFMIN(min_dts,
> av_rescale_q(st->index_entries[idx[i]].timestamp, st->time_base,
> AV_TIME_BASE_Q));
> > +                min_dts = FFMIN(min_dts,
> av_rescale_q(st->index_entries[idx[i]].timestamp/FFMAX(ast->sample_size,
> 1), st->time_base, AV_TIME_BASE_Q));
>
> the rounding looks wrong
>
> (AVRational){FFMAX(1, ast->sample_size), AV_TIME_BASE}
> is used elsewhere in the file instead of AV_TIME_BASE_Q for this
> purpose
>
> also do you have a file or testcase that needs this ? i wasnt able to
> find one where this patch made a difference
>
> Thanks
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If you think the mosad wants you dead since a long time then you are either
> wrong or dead since a long time.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Sorry, didn't know about the commit message format, I will prefix with the
relevant filename this in the future.

I have done the rounding like that so that it is identical to the
corresponding test done during demuxing in avi_read_packet:

1209<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1209>
           if(ast->sample_size)
1210<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1210>
               pkt->dts /= ast->sample_size;

...

1262<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1262>
       if(!avi->non_interleaved && st->nb_index_entries>1 &&
avi->index_loaded>1){
1263<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1263>
           int64_t dts= av_rescale_q(pkt->dts, st->time_base,
AV_TIME_BASE_Q);
1264<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1264>
1265<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1265>
           if(avi->dts_max - dts > 2*AV_TIME_BASE){
1266<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1266>
               avi->non_interleaved= 1;
1267<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/avidec.c;h=f5c234552d0e11a66b29613edab20fadf114ece7;hb=HEAD#l1267>
               av_log(s, AV_LOG_INFO, "Switching to NI mode, due to
poor interleaving\n");

Here the division is done first, and then the conversion to AV_TIME_BASE. I
have no objection to updating the patch to do it as you have suggested and
combining the two though, if you think that would better.

Apart from the non_interleaved flag being set to 1 where is could be 0 on
any file with an index over a few seconds long and with streams having
different values for sample_size, you are correct that this doesn't make a
great deal of difference. However it is pretty clearly wrong at the moment,
as the index timestamp should be divided by the sample size to get the dts,
as it is elsewhere in the code.

Unfortunately the file I have been looking at does not belong to me, so I
don't think I can upload it at the moment.

Regards,

Nigel