[FFmpeg-devel] [PATCH] MLP/TrueHD decoder

Ian Caulfield ian.caulfield
Thu Nov 29 11:55:56 CET 2007


On Nov 12, 2007 1:25 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> Hi
>
>
> On Thu, Nov 08, 2007 at 01:33:46PM +0000, Ian Caulfield wrote:
> > >
> > > so after this max_channel and max_matrix_channel are invalid, is there
> > > anything that stops them from being used? if no then this
> > > check is as good as no check at all
> >
> > The first case sets max_decoded_substream so that these parameters
> > won't be used, while the second case returns an error - however I seem
>
> i dont see how returing an error will prevent the use of the invalid value
> in future frames
> leaving variables in the context which are used to prevent array overflows
> at invalid values is fragile, what if the code is changed slightly in the
> future and they do become used? (that is assuming they wouldnt be used
> currently already in some cases)

As well as an error being returned, the "have parameters" flag in the
context is cleared, causing all packets to be rejected until we get a
(valid) sync packet.

> >
> > > [...]
> > > > +    if (!m->in_sync)
> > > > +        return length * 2;
> > >
> > > please explain this, didnt your parser already drop all "not in sync" parts?
> >
> > in_sync is probably a bad name - the parser does drop frames when it's
> > not in sync. However, the sync frames contain decoding parameters
> > needed for all packets until the next sync frame, so if there's an
> > error decoding the sync frame, the decoder can't decode any further
> > packets until another one comes along, regardless of whether the
> > parser is synced up.
>
> do the parameters change between syncs in practice? or would using the
> parameters of the last undamaged sync work as well?

Unfortunately so - the sync packets contain not just things like "6
channels, 48KHz", but also encoding parameters which change throughout
the stream to improve compression - an earlier version of the decoder
had a bug where one of the parameters wasn't reset properly and it
sounded awful. I think trying to fudge by on the last set of
parameters is likely to produce nasty output...

New version of the patch attached.

Ian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mlpdec.patch
Type: text/x-diff
Size: 41702 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071129/bdf6de9e/attachment.patch>



More information about the ffmpeg-devel mailing list