[FFmpeg-devel] [PATCH] avcodec/dcaenc: Initial implementation of ADPCM encoding for DCA encoder

[Rostislav Pehlivanov]

On 2 May 2017 at 22:53, Даниил Чередник <dan.cherednik at gmail.com> wrote:

> Hi.
>
> This patch introduces initial implementation of subband ADPCM encoding for
> DCA codec.
>
> Some results:
>
> sample:
>
> https://yadi.sk/d/B_3sVskM3HZiWK - original
>
> https://yadi.sk/d/7CK47Nt63HZiWf - without adpcm
>
> https://yadi.sk/d/25q1JDV93HZiWq - with adpcm
>
> chirp tone:
>
> https://yadi.sk/i/tZKHoJ1d3HZk4c
>
> Right now this feature is disabled by default. But it is ready to try
> using -dca_adpcm 1 option.
>
> There are some issues, should be solved before enabling this feature by
> default:
>
> 1. Speed up: I am trying to find best filter in each subband. But with real
> signal, usually only few subbands has significant prediction gain. The idea
> is try to analyze FFT spectrum (which is already calculated), to check is
> particular subband looks like tonal or noise. If subband is noise like - do
> not try to find best LPC predictor.
>
> 2. Modify psychoacoustic to use prediction gain for bit allocation. Right
> now ADPCM encoded block can get some extra bits.
>
> 3. Tuning the prediction gain threshold.
>
>
> Thank you.
> --
> Daniil Cherednik
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
>+static int64_t calc_corr(const int32_t *x, int len, int j, int k)

Add inline attrib? Seems appropriate here.


>+for (n = 0; n < len; n++) {
>+        s += MUL64(x[n-j], x[n-k]);
>+    }

For loops with 1 line we leave the brackets out.


>+for (i = 0; i <= DCA_ADPCM_COEFFS; i++) {
>+        for (j = i; j <= DCA_ADPCM_COEFFS; j++) {
>+            corr[k++] = calc_corr(in+4, len, i, j);
>+        }
>+    }

Same


>+    for (i = 0; i < len + DCA_ADPCM_COEFFS; i++) {
>+        max |= FFABS(in[i]);
>+    }

Same


>for (ch = 0; ch < c->fullband_channels; ch++) {
>+        for (band = 0; band < 32; band++) {
>+            if (c->prediction_mode[ch][band] >= 0) {
>+                quantize_adpcm_subband(c, ch, band);
>+            }
>+        }
>+    }

Same


>+    for (ch = 0; ch < c->fullband_channels; ch++) {
>+        for (band = 0; band < 32; band++) {
>+            if (c->prediction_mode[ch][band] == -1) {
>+                for (sample = 0; sample < SUBBAND_SAMPLES; sample++) {
>+                    c->quantized[ch][band][sample] =
quantize_value(c->subband[ch][band][sample], c->quant[ch][band]);
>+                }
>+            }
>+        }
>+    }

Same, 4 whole whitespace lines added here.


>+    if (c->bitrate_index == 3) {
>+        step_size = ff_dca_lossless_quant[c->abits[ch][band]];
>+    } else {
>+        step_size = ff_dca_lossy_quant[c->abits[ch][band]];
>+    }

Same


>for (;;) {

while (1) {

>+        if (i++ == last_pos)
>+            break;

Better yet remove the infinite loop and just use a normal for () loop.


>+static inline void ff_dca_core_dequantize(int32_t *output, const int32_t
*input,
>+                              int32_t step_size, int32_t scale, int
residual, int len)

Fix second line's alignment.


>+struct premultiplied_coeffs {
>+    int32_t aa[10];
>+};

I think it would be simpler to just use int32_t premultiplied_coeffs[10]
instead of a struct.


Apart from these style issues patch looks fine. I'll be able to test it in
a day.