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

James Almer jamrial at gmail.com
Thu Mar 7 23:32:00 EET 2019


On 3/7/2019 6:18 PM, Michael Niedermayer wrote:
> 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 ?

It depends on the amount of damage. Even a single bit would trigger a
crc mismatch, but be hardly noticeable.

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

If the user enables err_recognition and explode flags, both of which are
currently disabled by default, then aborting on crc failure is the
expected behavior.

> 
> 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
> 
> [...]
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 



More information about the ffmpeg-devel mailing list