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

[Lynne]

Apr 9, 2019, 2:56 PM by dev at lynne.ee:

>
>
>
> Apr 7, 2019, 12:56 AM by > michael at niedermayer.cc <mailto:michael at niedermayer.cc>> :
>
>> On Sat, Apr 06, 2019 at 10:44:00AM +0200, Lynne wrote:
>>
>>> Apr 4, 2019, 10:30 AM by >>> michael at niedermayer.cc <mailto:michael at niedermayer.cc>>>> :
>>>
>>> > On Wed, Mar 06, 2019 at 02:47:37PM +0100, Lynne wrote:
>>> >
>>> >> 6 Mar 2019, 11:22 by >> >>> dev at lynne.ee <mailto:dev at lynne.ee>>>>  <mailto:>>> dev at lynne.ee <mailto:dev at lynne.ee>>>> >>> :
>>> >>
>>> >> > The CRC flag is only signalled once every few minutes but CRC is still
>>> >> > always present so the patch uses the file version instead.
>>> >> > CRC on 24-bit files wants non-padded samples so skip such files.
>>> >> > Some corrupt samples may have been output before the final check
>>> >> > depending on the -max_samples setting.
>>> >> >
>>> >> v2 attached
>>> >>
>>> >> apedec.c |   26 +++++++++++++++++++++++++-
>>> >>  1 file changed, 25 insertions(+), 1 deletion(-)
>>> >> c0cc550d77927ee0349094a4976f73d0ef671ff6  0001-apedec-add-ability-to-check-CRC-v2.patch
>>> >> From 68c25bb026761eacda3c276148e2beb34e8929fd 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 11:01:01 +0000
>>> >> Subject: [PATCH v2] apedec: add ability to check CRC
>>> >>
>>> >> The CRC flag is only signalled once every few minutes but CRC is still
>>> >> always present so the patch uses the file version instead.
>>> >> CRC on 24-bit files wants non-padded samples so skip such files.
>>> >> Some corrupt samples may have been output before the final check
>>> >> depending on the -max_samples setting.
>>> >> ---
>>> >>  libavcodec/apedec.c | 26 +++++++++++++++++++++++++-
>>> >>  1 file changed, 25 insertions(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>>> >> index 15eb416ba4..a20d884606 100644
>>> >> --- a/libavcodec/apedec.c
>>> >> +++ b/libavcodec/apedec.c
>>> >> @@ -24,6 +24,7 @@
>>> >> 
>>> >>  #include "libavutil/avassert.h"
>>> >>  #include "libavutil/channel_layout.h"
>>> >> +#include "libavutil/crc.h"
>>> >>  #include "libavutil/opt.h"
>>> >>  #include "lossless_audiodsp.h"
>>> >>  #include "avcodec.h"
>>> >> @@ -147,7 +148,8 @@ typedef struct APEContext {
>>> >>  int fset;                                ///< which filter set to use (calculated from compression level)
>>> >>  int flags;                               ///< global decoder flags
>>> >> 
>>> >> -    uint32_t CRC;                            ///< frame CRC
>>> >> +    uint32_t CRC;                            ///< signalled frame CRC
>>> >> +    uint32_t CRC_state;                      ///< accumulated CRC
>>> >>  int frameflags;                          ///< frame flags
>>> >>  APEPredictor predictor;                  ///< predictor used for final reconstruction
>>> >> 
>>> >> @@ -730,6 +732,7 @@ static int init_entropy_decoder(APEContext *ctx)
>>> >> 
>>> >>  /* Read the frame flags if they exist */
>>> >>  ctx->frameflags = 0;
>>> >> +    ctx->CRC_state = UINT32_MAX;
>>> >>  if ((ctx->fileversion > 3820) && (ctx->CRC & 0x80000000)) {
>>> >>  ctx->CRC &= ~0x80000000;
>>> >> 
>>> >> @@ -1548,6 +1551,27 @@ static int ape_decode_frame(AVCodecContext *avctx, void *data,
>>> >> 
>>> >>  s->samples -= blockstodecode;
>>> >> 
>>> >> +    if ((avctx->err_recognition & AV_EF_CRCCHECK) &&
>>> >>
>>> >> +        (s->fileversion >= 3900) && (s->bps < 24)) {
>>> >>
>>> >
>>> > what about file versions other than this ? do they have no crc ?
>>> >
>>> They have a different CRC version which is poorly documented.
>>>
>>>
>>> >
>>> > what about othet bps ?
>>> >
>>> 24 bit CRC needs to ignore the zeroes padding each sample in 32 bits. I couldn't find any 24-bit files so I left that for a future patch.
>>>
>>
>> can such a file not be generated ? or
>> maybe printing a note and asking for a sample in this case would be an option
>> if noone has one either ...
>>
>
> I think skipping the check for such files is okay for now. Its just for verification purposes after all, and not all decoders implement the option but silently skip it.
>

Ping?