[FFmpeg-devel] [PATCH 3/3] Add ADPCM IMA CRYO APC encoder

[Michael Niedermayer]

On Mon, Dec 10, 2018 at 12:32:51PM +0100, Tomas Härdin wrote:
> 

>  Changelog                      |    1 +
>  libavcodec/adpcmenc.c          |   33 +++++++++++++++++++++++++++++++++
>  libavcodec/allcodecs.c         |    1 +
>  libavcodec/version.h           |    4 ++--
>  tests/fate/acodec.mak          |    2 ++
>  tests/ref/acodec/adpcm-ima_apc |    4 ++++
>  6 files changed, 43 insertions(+), 2 deletions(-)
> e86974218c35b93a077f5a38bcccb56ee3b36ca5  0003-Add-ADPCM-IMA-CRYO-APC-encoder.patch
> From 32cc6a96f80b5406e8327d912c8fc38812e6a664 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <tjoppen at acc.umu.se>
> Date: Fri, 23 Nov 2018 15:15:02 +0100
> Subject: [PATCH 3/3] Add ADPCM IMA CRYO APC encoder
> 
> No trellis quantization yet
> ---
>  Changelog                      |  1 +
>  libavcodec/adpcmenc.c          | 33 +++++++++++++++++++++++++++++++++
>  libavcodec/allcodecs.c         |  1 +
>  libavcodec/version.h           |  4 ++--
>  tests/fate/acodec.mak          |  2 ++
>  tests/ref/acodec/adpcm-ima_apc |  4 ++++
>  6 files changed, 43 insertions(+), 2 deletions(-)
>  create mode 100644 tests/ref/acodec/adpcm-ima_apc
> 
> diff --git a/Changelog b/Changelog
> index f678feed65..e6ae0c1187 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -11,6 +11,7 @@ version <next>:
>  - dhav demuxer
>  - PCM-DVD encoder
>  - CRYO APC muxer
> +- ADPCM IMA CRYO APC encoder
>  
>  
>  version 4.1:
> diff --git a/libavcodec/adpcmenc.c b/libavcodec/adpcmenc.c
> index 668939c778..0d757d5b46 100644
> --- a/libavcodec/adpcmenc.c
> +++ b/libavcodec/adpcmenc.c
> @@ -54,6 +54,7 @@ typedef struct ADPCMEncodeContext {
>      TrellisNode *node_buf;
>      TrellisNode **nodep_buf;
>      uint8_t *trellis_hash;
> +    int extradata_updated;
>  } ADPCMEncodeContext;
>  
>  #define FREEZE_INTERVAL 128
> @@ -124,6 +125,15 @@ static av_cold int adpcm_encode_init(AVCodecContext *avctx)
>              bytestream_put_le16(&extradata, ff_adpcm_AdaptCoeff2[i] * 4);
>          }
>          break;
> +    case AV_CODEC_ID_ADPCM_IMA_APC:
> +        if (avctx->trellis) {
> +            av_log(avctx, AV_LOG_ERROR, "trellis encoding not implemented for CRYO APC\n");
> +            return AVERROR_PATCHWELCOME;
> +        }
> +        //extradata will be output in adpcm_encode_frame()
> +        avctx->frame_size  = BLKSIZE * 2 / avctx->channels;
> +        avctx->block_align = BLKSIZE;
> +        break;
>      case AV_CODEC_ID_ADPCM_YAMAHA:
>          avctx->frame_size  = BLKSIZE * 2 / avctx->channels;
>          avctx->block_align = BLKSIZE;
> @@ -491,6 +501,28 @@ static int adpcm_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>      dst = avpkt->data;
>  
>      switch(avctx->codec->id) {
> +    case AV_CODEC_ID_ADPCM_IMA_APC:
> +        //initialize predictors using initial samples
> +        if (!c->extradata_updated) {
> +            uint8_t *side_data = av_packet_new_side_data(
> +                avpkt, AV_PKT_DATA_NEW_EXTRADATA, 8);
> +
> +            if (!side_data) {
> +                return AVERROR(ENOMEM);
> +            }
> +
> +            for (ch = 0; ch < avctx->channels; ch++) {
> +                c->status[ch].prev_sample = samples[ch];
> +                bytestream_put_le32(&side_data, c->status[ch].prev_sample);
> +            }
> +            c->extradata_updated = 1;
> +        }

This looks like something went wrong with how IMA_APC was implemented

the first samples are not a global header. extradata is a global header
If its done as its implemented then extradata will not be available before
the first packet and it will not work with many muxers
in fact the muxer submitted here needs to special case the late occurance
of extradata.
I suspect the related code would be simpler if the data currently passed
through extradata would be passed as part of the first packet (not counting
code for compatibility with the old way of handling it)

another aspect of this is seeking. Seeking back to the begin has to reset
the initial sample stuff. This would occur naturally if its in the first packet
as is i think this doesnt work as extradata is not reused after init. That 
issue is about the demuxer/decoder though but its also connected via the way
extradata is used

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181212/8c84744c/attachment.sig>