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

[Michael Niedermayer]

On Thu, Apr 16, 2020 at 03:54:01AM +0000, Zane van Iperen wrote:
> On Wed, 15 Apr 2020 19:31:44 +0200
> "Michael Niedermayer" <michael at niedermayer.cc> wrote:
> 
> > On Tue, Apr 07, 2020 at 10:48:58AM +0000, Zane van Iperen wrote:
> > > Signed-off-by: Zane van Iperen <zane at zanevaniperen.com>
> > > ---
> > >  Changelog               |  1 +
> > >  doc/general.texi        |  1 +
> > >  libavcodec/Makefile     |  1 +
> > >  libavcodec/adpcm.c      | 33 +++++++++++++++++++++++++++++++++
> > >  libavcodec/adpcm_data.c | 13 +++++++++++++
> > >  libavcodec/adpcm_data.h |  2 ++
> > >  libavcodec/allcodecs.c  |  1 +
> > >  libavcodec/avcodec.h    |  1 +
> > >  libavcodec/codec_desc.c |  7 +++++++
> > >  libavcodec/version.h    |  4 ++--
> > >  10 files changed, 62 insertions(+), 2 deletions(-)  
> > 
> > this doesnt apply anymore
> > 
> 
> Yeah, I've rebased upon the latest master, will submit as a v9 tonight.
> 
> 
> > > +    predictor = c->predictor + (step * nibble);  
> > 
> > unneeded ()
> > 
> 
> Removed.
> 
> > 
> > > +
> > > +    c->predictor = av_clip_int16(predictor);
> > > +    c->step_index = step_index;
> > > +
> > > +    return (int16_t)c->predictor;  
> > 
> > unneeded cast
> > 
> 
> Removed.
> 
> > > @@ -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

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- 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/20200416/926c254e/attachment.sig>