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

Paul B Mahol onemda at gmail.com
Sun Jul 29 18:11:41 CEST 2012


On 7/29/12, Reimar Doeffinger <Reimar.Doeffinger at gmx.de> wrote:
> 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.

This is wrong.....
>
> bytestream2 costs performance, so one might not want to use it for parts
> that are critical and can't overread enough to cause issues.

You have evidence for that? Whole file use bytestream2 functions.

This file mix unchecked and checked variants depending on decoder.
Unchecked variants are used for cases where it is know that overreads are
not possible because nb_samples are calculated depending on size of packet.

Thus no overreads should happen at all, if they happen it is bug in code -
perfect place for assert.

> Though I agree that this seems a bit fishy, it would be necessary to
> figure out what goes on.


More information about the ffmpeg-devel mailing list