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

[Paul B Mahol]

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.