[FFmpeg-devel] [PATCH v8 1/2] avcodec: add support for Cunning Developments' ADPCM

[Michael Niedermayer]

On Thu, Apr 16, 2020 at 11:39:00PM +0000, Zane van Iperen wrote:
> On Thu, 16 Apr 2020 21:37:54 +0200
> "Michael Niedermayer" <michael at niedermayer.cc> wrote:
> 
> > > > > @@ -1304,6 +1329,13 @@ static int
> > > > > adpcm_decode_frame(AVCodecContext *avctx, void *data, samples
> > > > > += avctx->channels; }
> > > > >          break;
> > > > > +    case AV_CODEC_ID_ADPCM_IMA_CUNNING:
> > > > > +        while (bytestream2_get_bytes_left(&gb) > 0) {
> > > > > +            int v = bytestream2_get_byteu(&gb);
> > > > > +            *samples++ =
> > > > > adpcm_ima_cunning_expand_nibble(&c->status[0], v & 0x0F);
> > > > > +            *samples++ =
> > > > > adpcm_ima_cunning_expand_nibble(&c->status[0], v >> 4);
> > > > > +        }
> > > > > +        break;  
> > > >
> > > > i would add an av_assert to ensure the samples array is large
> > > > enough and the code seting it stays in sync. And also so the
> > > > reader knows at a glance that this is ok with only a check on the
> > > > input size 
> > >
> > > So, something like this?
> > > av_assert0(frame->nb_samples == buf_size * 2);  
> > 
> > as the loop runs bytestream2_get_bytes_left(&gb) iterations
> > the check should be between that and nb_samples i think
> 
> In that case, would it just be better to change the while() to a for()?
> Same thing, but it shows the samples/nb_samples relationship.

yes


> 
>   for (n = 0; n < nb_samples / 2; n++) {}
> 
> There's a few other decoders that use the same while() I did
> originally, so it might be worth changing them too.

yes i think so too

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200417/a1ffe99d/attachment.sig>