[FFmpeg-devel] apedec: add ability to check CRC

The CRC flag is only signalled once every few minutes but CRC is still
always present so the patch uses the file version instead.
CRC on 24-bit files wants non-padded samples so skip such files.
Some corrupt samples may have been output before the final check
depending on the -max_samples setting.

2019-03-06 12:22 GMT+01:00, Lynne <dev@lynne.ee>:
> The CRC flag is only signalled once every few minutes but CRC is still
> always present so the patch uses the file version instead.
> CRC on 24-bit files wants non-padded samples so skip such files.
> Some corrupt samples may have been output before the final check
> depending on the -max_samples setting.

Imo, you should only return an error for some non-default
strict value (it should be possible for the user to show the
issue but continue to decode).

Carl Eugen

On 3/6/2019 8:22 AM, Lynne wrote:
> The CRC flag is only signalled once every few minutes but CRC is still
> always present so the patch uses the file version instead.
> CRC on 24-bit files wants non-padded samples so skip such files.
> Some corrupt samples may have been output before the final check
> depending on the -max_samples setting.
> 
> 
> 0001-apedec-add-ability-to-check-CRC.patch
> 
> From ad5773274448c209371dc8c70b4b21f39f968308 Mon Sep 17 00:00:00 2001
> From: Lynne <dev@lynne.ee>
> Date: Wed, 6 Mar 2019 11:01:01 +0000
> Subject: [PATCH] apedec: add ability to check CRC
> 
> The CRC flag is only signalled once every few minutes but CRC is still
> always present so the patch uses the file version instead.
> CRC on 24-bit files wants non-padded samples so skip such files.
> Some corrupt samples may have been output before the final check
> depending on the -max_samples setting.
> ---
>  libavcodec/apedec.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> index 15eb416ba4..3208275f6e 100644
> --- a/libavcodec/apedec.c
> +++ b/libavcodec/apedec.c
> @@ -24,6 +24,7 @@
>  
>  #include "libavutil/avassert.h"
>  #include "libavutil/channel_layout.h"
> +#include "libavutil/crc.h"
>  #include "libavutil/opt.h"
>  #include "lossless_audiodsp.h"
>  #include "avcodec.h"
> @@ -147,7 +148,8 @@ typedef struct APEContext {
>      int fset;                                ///< which filter set to use (calculated from compression level)
>      int flags;                               ///< global decoder flags
>  
> -    uint32_t CRC;                            ///< frame CRC
> +    uint32_t CRC;                            ///< signalled frame CRC
> +    uint32_t CRC_state;                      ///< accumulated CRC
>      int frameflags;                          ///< frame flags
>      APEPredictor predictor;                  ///< predictor used for final reconstruction
>  
> @@ -730,6 +732,7 @@ static int init_entropy_decoder(APEContext *ctx)
>  
>      /* Read the frame flags if they exist */
>      ctx->frameflags = 0;
> +    ctx->CRC_state = ~0U;

nit: UINT32_MAX.

>      if ((ctx->fileversion > 3820) && (ctx->CRC & 0x80000000)) {
>          ctx->CRC &= ~0x80000000;
>  
> @@ -1548,6 +1551,26 @@ static int ape_decode_frame(AVCodecContext *avctx, void *data,
>  
>      s->samples -= blockstodecode;
>  
> +    if ((avctx->err_recognition & (AV_EF_CRCCHECK | AV_EF_COMPLIANT)) &&
> +        (s->fileversion >= 3900) && (s->bps < 24)) {
> +        uint32_t crc = s->CRC_state;
> +        const AVCRC *crc_tab = av_crc_get_table(AV_CRC_32_IEEE_LE);
> +        for (i = 0; i < blockstodecode; i++) {
> +            for (ch = 0; ch < s->channels; ch++) {
> +                uint8_t *smp = frame->data[ch] + (i*(s->bps >> 3));
> +                crc = av_crc(crc_tab, crc, smp, s->bps >> 3);
> +            }
> +        }
> +
> +        if (!s->samples && ((~crc >> 1) ^ s->CRC)) {
> +            av_log(avctx, AV_LOG_ERROR, "CRC mismatch! Previously decoded "
> +                   "frames may have been affected as well.\n");
> +            return AVERROR_INVALIDDATA;

Abort with an error only if (avctx->err_recognition & AV_EF_EXPLODE) is
true.

> +        }
> +
> +        s->CRC_state = crc;
> +    }
> +
>      *got_frame_ptr = 1;
>  
>      return !s->samples ? avpkt->size : 0;
> -- 2.21.0
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

On Wed, Mar 6, 2019 at 2:18 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> 2019-03-06 12:22 GMT+01:00, Lynne <dev@lynne.ee>:
> > The CRC flag is only signalled once every few minutes but CRC is still
> > always present so the patch uses the file version instead.
> > CRC on 24-bit files wants non-padded samples so skip such files.
> > Some corrupt samples may have been output before the final check
> > depending on the -max_samples setting.
>
> Imo, you should only return an error for some non-default
> strict value (it should be possible for the user to show the
> issue but continue to decode).
>

strict is the wrong value to check here. It could use err_recognition
& AV_EF_EXPLODE and only print otherwise.

- Hendrik