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

Michael Niedermayer michael at niedermayer.cc
Thu Dec 13 17:01:54 EET 2018


On Thu, Dec 13, 2018 at 03:34:18PM +0100, Tomas Härdin wrote:
> ons 2018-12-12 klockan 19:55 +0100 skrev Michael Niedermayer:
> > On Wed, Dec 12, 2018 at 01:25:15PM +0100, Tomas Härdin wrote:
> > > ons 2018-12-12 klockan 00:05 +0100 skrev Michael Niedermayer:
> > > > On Mon, Dec 10, 2018 at 12:32:51PM +0100, Tomas Härdin wrote:
> > > > > 
> > > > > @@ -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
> > > 
> > > For APC they are global. They are used to "warm up" the IMA ADPCM
> > > decoder. Compare this to some of the other IMA variants like IMA_WAV
> > > which have some bytes for initial samples in every packet.
> > 
> > If the APC data was global, then it should be used for each packet.
> > that is initialize from extradata on each packet
> > 
> > That would not work, instead the data is only correct for the first packet
> > Which is why this data is not global IMHO.
> 
> It is "used" for every packet in the sense that correct decode of each
> packet depends on correct decode of every preceding packet, back to
> packet zero, which depends on those eight bytes. It also happens to be
> the way the existing IMA_APC decoder works. See adpcm_decode_init() in
> adpcm.c. I suspect there are more codecs that work similarly.

yes, every codec that has I frames and P frames. Based on what you say
here the first I frame should be in extradata. Because future frames
depend on it and that makes it global.
APC: first samples, differentially coded next samples, differentially coded next samples, ...
MPEG:first I frame, differentially coded P frame, differentially coded P frame, ...

for APC the first samples are in extradata
for MPEG the first sample (I frame) is not in extradata

Its really the same as other codecs but its handled very differently in how its
mapped to extradata and packets

Again as i said, i dont mind if its left as it is, you are working on this not
me. Iam just trying to make really sure you understand what the issue is
iam seeing here. (seemed to me you dont see how this differs from other codecs)

Thanks

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/20181213/7dfe31a4/attachment.sig>


More information about the ffmpeg-devel mailing list