[FFmpeg-devel] [PATCH] libavcodec/flacenc: add backward-compatible 32 bit-per-sample capability

[Lynne]

19 Dec 2021, 21:53 by mvanb1 at gmail.com:

> Enables creation of FLAC files with up to 32 bits-per-sample, up from the
> previous limit of 24 bit. This is a feature requested for RAWcooked, the
> archiving community has a need for storing files with 32-bit integer audio
> samples. See https://github.com/MediaArea/RAWcooked/issues/356
>
> Restrictions to the encoder are added so created files are compatible with
> existing decoders. Stereo decorrelation is disabled on 32 bit-per-sample,
> because a side channel would need 33 bit-per-sample, causing problems in
> existing 32 bit datapaths. Also only LPC encoding is enabled, because
> decoders capable of processing 24-bit files already use 64-bit processing
> for LPC, but not for fixed subframes.
>
> Furthermore, predictions and residuals are checked for causing integer
> overflow, reverting to a verbatim (store) subframe in case no LPC coeffs
> can be found that do not cause overflow.
>
> ffmpeg's FLAC decoder has been forward-compatible with this change since
> commit c720b9ce98 (May 2015). libFLAC is forward-compatible since release
> 1.2.1 (September 2007), the flac command line tool however blocks 32-bit
> files out of caution, it having been untested until now.
> ---
>  libavcodec/flacdsp.c | 47 ++++++++++++++++++++++
>  libavcodec/flacdsp.h |  3 ++
>  libavcodec/flacenc.c | 95 +++++++++++++++++++++++++++++++++++++-------
>  3 files changed, 130 insertions(+), 15 deletions(-)
>
> diff --git a/libavcodec/flacdsp.c b/libavcodec/flacdsp.c
> index bc9a5dbed9..422f6578ba 100644
> --- a/libavcodec/flacdsp.c
> +++ b/libavcodec/flacdsp.c
> @@ -43,6 +43,53 @@
>  #define PLANAR 1
>  #include "flacdsp_template.c"
>  
> +#define ZIGZAG_32BIT_MAX  0x3FFFFFFF
> +#define ZIGZAG_32BIT_MIN -0x3FFFFFFF
> +
> +int ff_flacdsp_lpc_encode_c_32_overflow_detect(int32_t *res, const int32_t *smp, int len,
> +                                               int order, int32_t *coefs, int shift)
> +{
> +    /* This function checks for every prediction and every residual
> +     * whether they cause integer overflow in existing decoders. In
> +     * case the prediction exceeds int32_t limits, prediction
> +     * coefficients are lowered accordingly. If the residual exceeds
> +     * ZIGZAG_32BIT_MAX and _MIN or coefficients have been lowered
> +     * twice but the prediction still overflows, give up */
>

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?

I'm really concerned about arbitrary data not being lossless in a codec
that's meant to be always lossless. If that can happen, I think 32-bit
encoding should be disabled unless -strict -1 is used.