[FFmpeg-devel] [PATCH] rmdec.c: correctly skip indexes

[Kostya]

On Mon, Mar 09, 2009 at 09:49:42AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> 2008/12/30 Michael Niedermayer <michaelni at gmx.at>:
> > On Tue, Dec 30, 2008 at 02:07:31PM -0500, Ronald S. Bultje wrote:
> >> @@ -445,9 +445,11 @@
> >> ? ? ? ? ? ? ?state= (state<<8) + get_byte(pb);
> >>
> >> ? ? ? ? ? ? ?if(state == MKBETAG('I', 'N', 'D', 'X')){
> >> - ? ? ? ? ? ? ? ?len = get_be16(pb) - 6;
> >> - ? ? ? ? ? ? ? ?if(len<0)
> >> - ? ? ? ? ? ? ? ? ? ?continue;
> >> + ? ? ? ? ? ? ? ?int size, n_pkts;
> >> + ? ? ? ? ? ? ? ?size = get_be32(pb);
> >> + ? ? ? ? ? ? ? ?url_fskip(pb, 4);
> >> + ? ? ? ? ? ? ? ?n_pkts = get_be16(pb);
> >> + ? ? ? ? ? ? ? ?len = size - 14 + n_pkts * 14;
> >> ? ? ? ? ? ? ? ? ?goto skip;
> >> ? ? ? ? ? ? ?}
> [..]
> > besides you change a 16bit BE to a 32bit BE read, its hard to imagine how
> > both could have made sense, you should explain this assuming your code
> > does make any sense.
> 
> So I wanted to look at this again. So the question here is, why does
> index-skipping not work for some files? Here's the two typical
> examples I've found (both taken from the same fle, the first DBZ movie
> on samples.mplayerhq.hu):
> 
> 1:
> 024935E0   3B B9 C9 57  D2 C2 7E 06  1D 5A FA 90  B9 FC 49 4E  ;..W..~..Z....IN
> 024935F0   44 58 00 00  1D 1E 00 00  00 00 02 13  00 00 02 49  DX.............I
> 02493600   53 0C 00 00  00 00 00 00  00 00 01 B2  00 00 00 00  S...............
> 
> Index size here is clearly not the two-bytes after INDEX, so I'm quite
> sure that's a typo in the current code. It's always 32-bit (see
> mplayer, gst, etc., as referenced before). However, the size is the
> size of the total code, because the next index starts exactly at this
> offset (24935EE+1D1E=249530C). Let's look at this one:

We have some docs:
https://common.helixcommunity.org/2003/HCS_SDK_r5/htmfiles/rmff.htm

and it says that index size is 32-bit indeed 

> 2:
> 02495300   00 12 67 1C  02 48 DA 8D  00 01 0F 76  49 4E 44 58  ..g..H.....vINDX
> 02495310   00 00 00 14  00 00 00 00  03 0C 00 01  00 00 00 00  ................
> 02495320   00 00 00 00  00 21 00 00  25 F4 00 00  00 0E 00 00  .....!..%.......
> 
> The length of all examples I've looked at so far is 0x14, which is not
> even enough space to fit a single INDEX chunk (14 bytes header minimum
> + 14 bytes per chunk). It seems that for any of these chunks where the
> "size" is 0x14, the actual size is 0x14 (index header) + n_pkts * 14.

0x14 is full index header indeed but docs say:
"Note that the size field of the Index Chunk Header is the size of the
entire index chunk, including the index records."

I suspect it was not widely available and is violated though.

> Attached patch implements this and correctly skips indexes for me.
> Behaviour before at end of file (add a printf directly inside the if
> state == MKTAG(INDX), and another printf directly after the skip:
> tag):
> 
> ==
> Index, len=-6
> Skip
> Skip
> Skip
> Skip
> Skip
> Skip
> Skip
> Skip
> EOF reached
> ==
> 
> After:
> ==
> Index, len=7446
> Skip
> Index, len=10926
> Skip
> EOF reached
> ==
> 
> I'll apply the patch unless people have obvious complaints. I'll do
> actual index reading in a separate patch, I'll probably implement
> seeking along with it (not yet done).

Please read that doc before applying that patch (it looks more or less ok though).
 
> Ronald