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

[Ronald S. Bultje]

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:

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.

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).

Ronald
-------------- next part --------------
Index: ffmpeg-svn/libavformat/rmdec.c
===================================================================
--- ffmpeg-svn.orig/libavformat/rmdec.c	2009-03-09 09:45:42.000000000 -0400
+++ ffmpeg-svn/libavformat/rmdec.c	2009-03-09 09:48:09.000000000 -0400
@@ -440,7 +440,15 @@
             state= (state<<8) + get_byte(pb);
 
             if(state == MKBETAG('I', 'N', 'D', 'X')){
-                len = get_be16(pb) - 6;
+                int size, n_pkts;
+                size = get_be32(pb);
+                if (size >= 28) {
+                    len = size - 8;
+                } else {
+                    url_fskip(pb, 2);
+                    n_pkts = get_be32(pb);
+                    len = size - 14 + n_pkts * 14;
+                }
                 if(len<0)
                     continue;
                 goto skip;