[FFmpeg-devel] [PATCH] mpegaudiodec_template: add ability to check CRC

[Michael Niedermayer]

On Wed, Mar 06, 2019 at 07:09:52PM +0100, Lynne wrote:
> A lot of files have CRC included.
> The CRC only covers 34 bytes at most from the frame but it should still be
> enough for some amount of error detection.

>  mpegaudiodec_template.c |   20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> e8276f62fa92aa3f78e53b182b4ca7a2a460754c  0001-mpegaudiodec_template-add-ability-to-check-CRC.patch
> From e1f4410f35d3d7f774a0de59ab72764033d14900 Mon Sep 17 00:00:00 2001
> From: Lynne <dev at lynne.ee>
> Date: Wed, 6 Mar 2019 17:04:04 +0000
> Subject: [PATCH] mpegaudiodec_template: add ability to check CRC
> 
> A lot of files have CRC included.
> The CRC only covers 34 bytes at most from the frame but it should still be
> enough for some amount of error detection.
> ---
>  libavcodec/mpegaudiodec_template.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/mpegaudiodec_template.c b/libavcodec/mpegaudiodec_template.c
> index 9cce88e263..0881b60bf5 100644
> --- a/libavcodec/mpegaudiodec_template.c
> +++ b/libavcodec/mpegaudiodec_template.c
> @@ -27,6 +27,7 @@
>  #include "libavutil/attributes.h"
>  #include "libavutil/avassert.h"
>  #include "libavutil/channel_layout.h"
> +#include "libavutil/crc.h"
>  #include "libavutil/float_dsp.h"
>  #include "libavutil/libm.h"
>  #include "avcodec.h"
> @@ -1565,9 +1566,22 @@ static int mp_decode_frame(MPADecodeContext *s, OUT_INT **samples,
>  
>      init_get_bits(&s->gb, buf + HEADER_SIZE, (buf_size - HEADER_SIZE) * 8);
>  
> -    /* skip error protection field */
> -    if (s->error_protection)
> -        skip_bits(&s->gb, 16);
> +    if (s->error_protection) {
> +        uint16_t crc = get_bits(&s->gb, 16);
> +        if (s->err_recognition & AV_EF_CRCCHECK) {
> +            const int sec_len = s->lsf ? ((s->nb_channels == 1) ? 9  : 17) :
> +                                         ((s->nb_channels == 1) ? 17 : 32);
> +            const AVCRC *crc_tab = av_crc_get_table(AV_CRC_16_ANSI);
> +            uint32_t crc_cal = av_crc(crc_tab, UINT16_MAX, &buf[2], 2);
> +            crc_cal = av_crc(crc_tab, crc_cal, &buf[6], sec_len);
> +
> +            if (av_bswap16(crc) ^ crc_cal) {
> +                av_log(s->avctx, AV_LOG_ERROR, "CRC mismatch!\n");
> +                if (s->err_recognition & AV_EF_EXPLODE)
> +                    return AVERROR_INVALIDDATA;
> +            }
> +        }
> +    }

For files with crcs and with damage, do they sound better with the
check and error out or without ?

The behavior which provides the best user experience should be the
default

It also may make sense to add one of AV_EF_CAREFUL / AV_EF_COMPLIANT / AV_EF_AGGRESSIVE
depending on how the check affects actual real world files

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190307/31435a11/attachment.sig>