[FFmpeg-devel] [PATCH v3 1/2] avcodec: add adpcm_ima_ssi encoder

[Zane van Iperen]

On Sun, 12 Apr 2020 11:41:40 +0200
"Paul B Mahol" <onemda at gmail.com> wrote:

> On 4/12/20, Zane van Iperen <zane at zanevaniperen.com> wrote:
> > On Sat, 11 Apr 2020 22:02:26 +0200
> > "Paul B Mahol" <onemda at gmail.com> wrote:
> >  
> >> >
> >> > Ping.
> >> >
> >> > Also, could someone please clarify something for me?
> >> >
> >> > When encoding, there's no inherit differences between trellis and
> >> > non-trellis other then a potentially more-accurate set of
> >> > nibbles?
> >> >
> >> > And the decoder is none the wiser and chews on them both the same
> >> > way?
> >> >
> >> > If I'm right, then this looks to be a decoder issue...
> >> >  
> >>
> >> You are wrong, just follow qt adpcm encoder code.
> >>  
> >
> > I did.
> >
> >
> > Is it possible, and I don't suggest this lightly, that the trellis
> > code is bugged, and errors accumulate over time?
> >
> > I ask this because all of the encoders (except adpcm_yamaha, which
> > is different anyway) have either the full sample and/or the step
> > index periodically written to the bitstream, which is then used to
> > reset the state machine, thus errors don't accumulate.
> >
> > adpcm_ima_ssi doesn't have that, so it relies on the state being
> > accurate through the entire stream. This would certainly explain the
> > behaviour I'm seeing in certain cases (see below).
> >
> > In both images, the top stream is encoded with '-trellis 0', and the
> > bottom with '-trellis 1':
> > - https://0x0.st/iSdS.png
> >   This one's obvious where the problem is.
> > - https://0x0.st/iSdQ.png
> >   This is more subtle, but the entire stream is offset slightly.
> > It's more noticeable at the start and end.
> >
> >
> > For reference, here's the code I'm using. You'll see it's basically
> > the same as the adpcm_ima_qt code:
> >
> > if (avctx->trellis > 0) {
> >     FF_ALLOC_OR_GOTO(avctx, buf, n * avctx->channels, error);
> >
> >     for (ch = 0; ch < avctx->channels; ch++) {
> >         adpcm_compress_trellis(avctx, samples + ch, buf + n * ch,
> >                                c->status + ch, n, avctx->channels);
> >         c->status[ch].prev_sample = c->status[ch].predictor;
> >     }
> >
> >     for (i = 0; i < n; i++) {
> >         for (ch = 0; ch < avctx->channels; ch++) {
> >             put_bits(&pb, 4, buf[n * ch + i]);
> >         }
> >     }
> >     av_free(buf);
> > }
> >
> >
> > Or maybe I just don't fully understand how trellis works.  
> 
> Yes. that is 100% correct.
> 
> Also you ignored fact that you do not update nodes for SSI variant in
> compress trellis.

I probably should have mentioned that yes, I had already added the
appropriate SSI code to adpcm_compress_trellis().