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

[Lynne]

8 Mar 2019, 17:48 by dev at lynne.ee:

>
>
>
> 7 Mar 2019, 21:18 by > michael at niedermayer.cc <mailto: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>>>>  <mailto:>>> 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.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>

Ping? Could someone take a look at this patch and my other 2 (apedec and pngdec CRC)?