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

[Ronald S. Bultje]

Hi,

On Tue, Dec 30, 2008 at 2:31 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> you silently remove a validity check, do you just not give a damn if the

Sorry, I thought I had made them unsigned, that was the intention.
When unsigned, the check makes no sense (because the value is always
positive). I don't care if we seek (unsigned int)-1 forward. If you
prefer signed (?), I can add the check back.

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

You want me to say the original code is wrong? I mean come on, it
fails to skip the index and results in garbage at the end of a file,
it was obviously never tested. Need I say more?

Ok, proof 1: all other level-1 IDs have a 32-bit size, it's
incomprehensible that real would choose a 16-bit one for INDX.

proof 2: mplayer/libmpdemux/demux_real.c:generate_index()
    i = stream_read_dword_le(demuxer->stream);
    if ((i == -256) || (i != MKTAG('I', 'N', 'D', 'X')))
    {
	mp_msg(MSGT_DEMUX, MSGL_WARN,"Something went wrong, no index chunk
found on given address (%d)\n",
	    next_header_pos);
	index_mode = -1;
        if (i == -256)
	    stream_reset(demuxer->stream);
    	stream_seek(demuxer->stream, origpos);
	return 0;
	//goto end;
    }

    mp_msg(MSGT_DEMUX, MSGL_V,"Reading index table from index chunk (%d)\n",
	next_header_pos);

    i = stream_read_dword(demuxer->stream);
    mp_msg(MSGT_DEMUX, MSGL_V,"size: %d bytes\n", i);

In other words, a dword (32-bit) object size.

proof 3: gst-plugins-ugly/gst/realmedia/rmdemux.c:gst_rmdemux_chain()

        rmdemux->object_id = RMDEMUX_FOURCC_GET (data + 0);
        rmdemux->size = RMDEMUX_GUINT32_GET (data + 4) - HEADER_SIZE;
[..]
        switch (rmdemux->object_id) {
[..]
          case GST_MAKE_FOURCC ('I', 'N', 'D', 'X'):
            rmdemux->state = RMDEMUX_STATE_HEADER_INDX;
            break;
[..]

Shows a 32-bit object size.

Conclusion: the original code is wrong.

Ronald