[FFmpeg-devel] [PATCH 06/16] vmdaudio: correct the silent chunk count in the first block.

[Reimar Döffinger]

On Tue, Feb 22, 2011 at 10:49:01PM +0100, Kostya wrote:
> On Tue, Feb 22, 2011 at 08:28:05PM +0100, Reimar D?ffinger wrote:
> > On Tue, Feb 22, 2011 at 02:05:25PM -0500, Justin Ruggles wrote:
> > > diff --git a/libavcodec/vmdav.c b/libavcodec/vmdav.c
> > > index 025d23a..ed3c5f8 100644
> > > --- a/libavcodec/vmdav.c
> > > +++ b/libavcodec/vmdav.c
> > > @@ -517,11 +517,7 @@ static int vmdaudio_decode_frame(AVCodecContext *avctx,
> > >          uint32_t flags = AV_RB32(p);
> > >          int raw_block_size = s->block_align *
> > >                               (av_get_bits_per_sample_fmt(avctx->sample_fmt) / 8);
> > > -        int silent_chunks;
> > > -        if(flags == 0xFFFFFFFF)
> > > -            silent_chunks = 32;
> > > -        else
> > > -            silent_chunks = av_log2(flags + 1);
> > > +        int silent_chunks = av_popcount(flags);
> > 
> > Hm, what exactly is the difference in meaning between
> > 0x1, 0x2, 0x4 for example?
> > 
> > >          if(*data_size < (s->block_align*silent_chunks + buf_size - 20) * 2)
> > 
> > Unrelated, but isn't it possible this might overflow?
> 
> block_align is 15 bit, max silent chunks = 32 (5 bit), so it won't overflow by
> itself

Well, block_align comes from AVCodecContext and the API does not mention
such a limitation.
Parsing errors in ffmpeg or non-ffmpeg demuxers, a new container
format that uses more bits to store it or wrong sign handling might
result in a value that can cause an overflow.
Maybe it doesn't make much sense to bother with it, or maybe it
would make sense to check it in the decode function, I don't
know but might be worth a thought.