On Fri, 17 Apr 2020 18:50:05 +0200
"Michael Niedermayer" <mich...@niedermayer.cc> wrote:
> 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" <mich...@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
>
Okay, I'll send a v10 shortly with the change, followed by a
standalone patch for the other codecs.
Zane
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
