[FFmpeg-devel] [PATCH] oops I broke rdt.c

[Ronald S. Bultje]

Hi Michael,

On Tue, Dec 16, 2008 at 1:03 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
p,,[
> this sounds fragile, someone just needs to move some inocent looking
> code around or find a way that does not reset the value after
> _read_mdpr()
>
> please decide what the valid range of st->id can be
> and enforce it both in code writing it as well as ensuring that
> all code using it does not blow up with values within that range

I'll add checks for that.

> besides, thinking about it, your code looks just wrong
> the array size of MAX_STREAMS just is in no way related to a 16bit
> id read from the file.
> are you sure you are not misusing the variable.
> each variable has one and only one semantic meaning, either it is a
> stream index or a 16bit id it can absolutely not be both nor
> can that change after a function call. This is a recipe for a
> unmaintainable mess

st->id is always the index number of that stream in the media (as
opposed to in the array of AVFormatContext->streams, that is
st->index). In .rm files, this is a 16-bit integer. In RTSP, the value
was unused.

Therefore, for RDT, I decided to use it to specify the index of the
stream within the "set of streams with identical content". I can make
a new variable for that but it sounded kind of unnecessary. st->id is
ensured (I can assert() that if you wish) to be st->index -
first_st_of_set->index and therefore automatically is always smaller
than MAX_STREAMS, and is more specifically within the range of 0 and
RDTDemuxContext->n_streams. I think the assert() would be the easiest
way to ensure this / make it clear without making the code more
complex. If you're affraid that RTSP might some day start using
st->id, then I'll make a new variable or just use st->index -
first_st_of_set->index.

Ronald