[FFmpeg-devel] [PATCH] libavcodec/flacenc: add backward-compatible 32 bit-per-sample capability
Lynne
dev at lynne.ee
Mon Dec 20 01:05:23 EET 2021
19 Dec 2021, 22:41 by mvanb1 at gmail.com:
> Op zo 19 dec. 2021 om 22:11 schreef Lynne <dev at lynne.ee>:
>
>> What happens if there's an overflow and the prediction coefficients
>> are lowered? Is there a loss of bits? What about if it gives up?
>>
>
> The result remains lossless. If the prediction coefficients are
> lowered, the residual of the prediction increases. This means the file
> gets a little bigger, but the data is still all there.
>
> If it gives up on searching for suitable LPC coefficients, it uses a
> verbatim subframe instead of an LPC subframe. That means all data is
> stored as unencoded PCM, with no form of prediction whatsoever. This
> results in (usually a large) increase in storage space needed, but
> this is still lossless.
>
That's reasonable.
Some code style issues:
> + for (i = 0; i < s->frame.blocksize * s->channels; i++) {
> + AV_WL32(tmp + 4*i, samples0[i]);
> + }
No brackets here.
> + if(!ff_flacdsp_lpc_encode_c_32_overflow_detect(res, smp, n, opt_order,
> + lpc_try, shift[opt_order-1]))
> + continue;
Put a space after the if.
> + if(!ff_flacdsp_lpc_encode_c_32_overflow_detect(res, smp, n, i+1,
> + coefs[i], shift[i]))
> + continue;
And here, and in 2 other places.
> + if (pmax > 0) {
> + double scale;
> + if(lpc_reduction_tries >= 2)
Space after if.
> + return 0;
> + lpc_reduction_tries++;
> + scale = (double)INT32_MAX/pmax;
> + for (int i = 0; i < order; i++)
> + coefs[i] *= scale;
> + }
Could you replace this with a fixed-point implementation as well?
Even if it's outside of the audio path, we already have one issue
in the flac code where using a double in assembly is causing a
test to fail. I still have to rewrite that from inline assembly to
proper external asm.
More information about the ffmpeg-devel
mailing list