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

[Lynne]

7 Mar 2019, 21:18 by michael at niedermayer.cc:

> 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 <mailto: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
>

This is just a quick check to verify the files are uncorrupted, it shouldn't
make broken files sound better unless the decoder somehow outputs white noise
when it tries to decode them.
I corrupted some files with -bsf:a noise=0.5 and couldn't notice an improvement
with or without -err_detect crccheck+explode even though the outputs looked 
different. crccheck+explode makes much less errors about incorrect timestamps.