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