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

[Michael Niedermayer]

On Tue, Dec 16, 2008 at 12:40:52PM -0500, Ronald S. Bultje wrote:
> Hi Michael,
> 
> On Tue, Dec 16, 2008 at 10:37 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Tue, Dec 16, 2008 at 09:00:34AM -0500, Ronald S. Bultje wrote:
> >> @@ -314,6 +314,7 @@
> >>                                                  NULL, NULL, NULL, NULL);
> >>          }
> >>      } else {
> >> +        rdt->audio_pkt_cnt[st->id] =
> >>          ff_rm_retrieve_cache (rdt->rmctx, rdt->rmctx->pb, st, pkt);
> >>          if (rdt->audio_pkt_cnt[st->id] == 0 &&
> >>              st->codec->codec_id == CODEC_ID_AAC)
> >
> > i see code like
> >  st->id = get_be16(pb);
> > in rmdec.c
> > thus this looks suspect, given that the array is not that large
> 
> It precedes the call to _read_mdpr(), so in rdt.c we don't use that
> value (that code is never executed). Right now it's always 0. In the
> future (=stream selection), it'll be set in rdt.c to a value between 0
> and n_streams for that particular set of streams.

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

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081216/82077a84/attachment.pgp>