On Sun, Jun 28, 2015 at 01:39:41PM +0200, Andreas Cadhalpun wrote:
> On 28.06.2015 13:13, Ronald S. Bultje wrote:
> > On Sun, Jun 28, 2015 at 5:28 AM, Andreas Cadhalpun
> >> Treating one like an error, but not the other seems strange as well.
> >> One could add an explode mode for both. Would that be better?
> >
> >
> > In the first case, it's an error. If the frame size is 2 bits, the header
> > is 1, and it specifies a spillover bits of 2, then the frame is clearly
> > corrupt. Returning an error is fine. The ffmin() isn't necessary. I also
> > don't think an explode mode check is necessary here, it's a clear error
> > that is unrecoverable for this frame.
>
> I've no strong preference. Attached is a variant just returning an error.
>
> > In the second case, does that actually happen?
>
> Yes.
>
> > Wmavoice is one of the
> > limited number of decoders that internally checks for overreads.
> > get_bits_count() should never overread.
>
> It doesn't check everywhere:
> /* Statistics? FIXME - we don't check for length, a slight overrun
> * will be caught by internal buffer padding, and anything else
> * will be skipped, not read. */
> if (get_bits1(gb)) {
> res = get_bits(gb, 4);
> skip_bits(gb, 10 * (res + 1));
> }
>
> > Do you have samples for which this happens?
>
> Yes.
>
> > We currently basically return an error on any possible overread
> > signified in the bitstream (without actually overreading), so doing so here
> > also would make sense (if it really happens at all).
>
> OK.
>
> > (We could also remove all the overread checks in the decoder, make it use
> > the safe bitstream reader mode, and then check for overreads at the end of
> > synth_superframe or in the caller, and then return an error. I have no
> > specific preference, and this may lead to less code overall.)
>
> That should work as well, but would be a larger change.
>
> Best regards,
> Andreas> wmavoice.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > 6c2b085ae9d64d9d8ae33efa4f554f23df98922c > 0001-wmavoice-limit-wmavoice_decode_packet-return-value-t.patch > From 34aa9dd95c57039d56a07d0c9dfc837dd2199af8 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com> > Date: Sun, 28 Jun 2015 12:40:12 +0200 > Subject: [PATCH] wmavoice: limit wmavoice_decode_packet return value to packet > size > > Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com> LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Into a blind darkness they enter who follow after the Ignorance, they as if into a greater darkness enter who devote themselves to the Knowledge alone. -- Isha Upanishad
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
