[FFmpeg-devel] [PATCH 03/19] adpcm: fix return value for small overreads.

[Reimar Döffinger]

On Sun, Jul 29, 2012 at 02:49:04PM +0000, Paul B Mahol wrote:
> On 7/29/12, Nicolas George <nicolas.george at normalesup.org> wrote:
> >
> > Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> > ---
> >  libavcodec/adpcm.c |    5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> >
> > Comment for this one and the next few:
> >
> > I believe it is better to know what decoders rely on
> > FF_INPUT_BUFFER_PADDING_SIZE rather than catch everything in a common test,
> > but if people prefer the opposite I can drop these cases.
> >
> >
> > diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
> > index b86168a..6706b07 100644
> > --- a/libavcodec/adpcm.c
> > +++ b/libavcodec/adpcm.c
> > @@ -1265,7 +1265,10 @@ static int adpcm_decode_frame(AVCodecContext *avctx,
> > void *data,
> >      *got_frame_ptr   = 1;
> >      *(AVFrame *)data = c->frame;
> >
> > -    return bytestream2_tell(&gb);
> > +    ret = bytestream2_tell(&gb);
> > +    if (ret > avpkt->size && ret <= avpkt->size +
> > FF_INPUT_BUFFER_PADDING_SIZE)
> > +        ret = avpkt->size; /* small overreads are acceptable */
> > +    return ret;
> >  }
> >
> 
> Isn't point of bytestream2 to not overread?
> So I'm against applying this hack.
> 
> Instead all cases where unchecked bytestream2 functions are used
> (get_be16u and similar)
> should have bytestream2_get_bytes_left.. before them.

bytestream2 costs performance, so one might not want to use it for parts
that are critical and can't overread enough to cause issues.
Though I agree that this seems a bit fishy, it would be necessary to
figure out what goes on.