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

2019-04-18 Thread Lynne 
Apr 9, 2019, 2:56 PM by d...@lynne.ee:

>
>
>
> Apr 7, 2019, 12:56 AM by > mich...@niedermayer.cc 
> > :
>
>> On Sat, Apr 06, 2019 at 10:44:00AM +0200, Lynne wrote:
>>
>>> Apr 4, 2019, 10:30 AM by >>> mich...@niedermayer.cc 
>>> >>
>>> > On Wed, Mar 06, 2019 at 02:47:37PM +0100, Lynne wrote:
>>> >
>>> >> 6 Mar 2019, 11:22 by >> >>> d...@lynne.ee >> >> >> d...@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 <>> >>> d...@lynne.ee > 
>>> >> d...@lynne.ee >> >
>>> >> Date: Wed, 6 Mar 2019 11:01:01 +
>>> >> 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 & 0x8000)) {
>>> >>  ctx->CRC &= ~0x8000;
>>> >> 
>>> >> @@ -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?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2019-04-09 Thread Lynne 



Apr 7, 2019, 12:56 AM by mich...@niedermayer.cc:

> On Sat, Apr 06, 2019 at 10:44:00AM +0200, Lynne wrote:
>
>> Apr 4, 2019, 10:30 AM by >> mich...@niedermayer.cc 
>> >> :
>>
>> > On Wed, Mar 06, 2019 at 02:47:37PM +0100, Lynne wrote:
>> >
>> >> 6 Mar 2019, 11:22 by >> >> d...@lynne.ee >>  
>> >> > d...@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 <>> >> d...@lynne.ee >>  > 
>> >> d...@lynne.ee >> >>> >
>> >> Date: Wed, 6 Mar 2019 11:01:01 +
>> >> 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 & 0x8000)) {
>> >>  ctx->CRC &= ~0x8000;
>> >> 
>> >> @@ -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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2019-04-06 Thread Michael Niedermayer 
On Sat, Apr 06, 2019 at 10:44:00AM +0200, Lynne wrote:
> Apr 4, 2019, 10:30 AM by mich...@niedermayer.cc:
> 
> > On Wed, Mar 06, 2019 at 02:47:37PM +0100, Lynne wrote:
> >
> >> 6 Mar 2019, 11:22 by >> d...@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 <>> d...@lynne.ee >> >
> >> Date: Wed, 6 Mar 2019 11:01:01 +
> >> 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 & 0x8000)) {
> >>  ctx->CRC &= ~0x8000;
> >>  
> >> @@ -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 ...

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2019-04-06 Thread Lynne 
Apr 4, 2019, 10:30 AM by mich...@niedermayer.cc:

> On Wed, Mar 06, 2019 at 02:47:37PM +0100, Lynne wrote:
>
>> 6 Mar 2019, 11:22 by >> d...@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 <>> d...@lynne.ee >> >
>> Date: Wed, 6 Mar 2019 11:01:01 +
>> 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 & 0x8000)) {
>>  ctx->CRC &= ~0x8000;
>>  
>> @@ -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.



>> +uint32_t crc = s->CRC_state;
>> +const AVCRC *crc_tab = av_crc_get_table(AV_CRC_32_IEEE_LE);
>> +for (i = 0; i < blockstodecode; i++) {
>> +for (ch = 0; ch < s->channels; ch++) {
>> +uint8_t *smp = frame->data[ch] + (i*(s->bps >> 3));
>> +crc = av_crc(crc_tab, crc, smp, s->bps >> 3);
>> +}
>> +}
>> +
>> +if (!s->samples && ((~crc >> 1) ^ s->CRC)) {
>>
>> +av_log(avctx, AV_LOG_ERROR, "CRC mismatch! Previously decoded "
>> +   "frames may have been affected as well.\n");
>>
>
> What is the usecase for this ?
> the implementation does only check CRC for a subset of files and it could
> report them at a time different from where they occur
>
The use case is to be able to verify the integrity of files. Other lossless 
codecs allow to do that, like wavpack and tta.
The subset of files is increasing and is large as the version of the encoder 
producing them isn't new.
The time of reporting depends on the -max_samples setting or the encoder's 
choice of samples in a packet. For an integrity check the report time shouldn't 
matter.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2019-04-04 Thread Michael Niedermayer 
On Wed, Mar 06, 2019 at 02:47:37PM +0100, Lynne wrote:
> 6 Mar 2019, 11:22 by d...@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 
> Date: Wed, 6 Mar 2019 11:01:01 +
> 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 & 0x8000)) {
>  ctx->CRC &= ~0x8000;
>  
> @@ -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 ?

what about othet bps ?



> +uint32_t crc = s->CRC_state;
> +const AVCRC *crc_tab = av_crc_get_table(AV_CRC_32_IEEE_LE);
> +for (i = 0; i < blockstodecode; i++) {
> +for (ch = 0; ch < s->channels; ch++) {
> +uint8_t *smp = frame->data[ch] + (i*(s->bps >> 3));
> +crc = av_crc(crc_tab, crc, smp, s->bps >> 3);
> +}
> +}
> +
> +if (!s->samples && ((~crc >> 1) ^ s->CRC)) {

> +av_log(avctx, AV_LOG_ERROR, "CRC mismatch! Previously decoded "
> +   "frames may have been affected as well.\n");

What is the usecase for this ?
the implementation does only check CRC for a subset of files and it could
report them at a time different from where they occur

In practice for a user, that adds some unpredictability to this information
A file may contains CRC errors but no errors would be detected
A file may contain CRC errors but they would be shown too late
A file may contain CRC errors and they are shown at the correct time


thanks

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2019-04-01 Thread Lynne 
Mar 6, 2019, 1:47 PM by d...@lynne.ee:

> 6 Mar 2019, 11:22 by > d...@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
>

Ping? It hasn't had any objections yet.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2019-03-06 Thread Lynne 
6 Mar 2019, 11:22 by d...@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
>From 68c25bb026761eacda3c276148e2beb34e8929fd Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Wed, 6 Mar 2019 11:01:01 +
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 & 0x8000)) {
 ctx->CRC &= ~0x8000;
 
@@ -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)) {
+uint32_t crc = s->CRC_state;
+const AVCRC *crc_tab = av_crc_get_table(AV_CRC_32_IEEE_LE);
+for (i = 0; i < blockstodecode; i++) {
+for (ch = 0; ch < s->channels; ch++) {
+uint8_t *smp = frame->data[ch] + (i*(s->bps >> 3));
+crc = av_crc(crc_tab, crc, smp, s->bps >> 3);
+}
+}
+
+if (!s->samples && ((~crc >> 1) ^ s->CRC)) {
+av_log(avctx, AV_LOG_ERROR, "CRC mismatch! Previously decoded "
+   "frames may have been affected as well.\n");
+if (avctx->err_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+}
+
+s->CRC_state = crc;
+}
+
 *got_frame_ptr = 1;
 
 return !s->samples ? avpkt->size : 0;
-- 
2.21.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2019-03-06 Thread Hendrik Leppkes 
On Wed, Mar 6, 2019 at 2:18 PM Carl Eugen Hoyos  wrote:
>
> 2019-03-06 12:22 GMT+01:00, Lynne :
> > 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.
>
> Imo, you should only return an error for some non-default
> strict value (it should be possible for the user to show the
> issue but continue to decode).
>

strict is the wrong value to check here. It could use err_recognition
& AV_EF_EXPLODE and only print otherwise.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2019-03-06 Thread James Almer 
On 3/6/2019 8:22 AM, Lynne wrote:
> 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.
> 
> 
> 0001-apedec-add-ability-to-check-CRC.patch
> 
> From ad5773274448c209371dc8c70b4b21f39f968308 Mon Sep 17 00:00:00 2001
> From: Lynne 
> Date: Wed, 6 Mar 2019 11:01:01 +
> Subject: [PATCH] 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 | 25 -
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> index 15eb416ba4..3208275f6e 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 = ~0U;

nit: UINT32_MAX.

>  if ((ctx->fileversion > 3820) && (ctx->CRC & 0x8000)) {
>  ctx->CRC &= ~0x8000;
>  
> @@ -1548,6 +1551,26 @@ static int ape_decode_frame(AVCodecContext *avctx, 
> void *data,
>  
>  s->samples -= blockstodecode;
>  
> +if ((avctx->err_recognition & (AV_EF_CRCCHECK | AV_EF_COMPLIANT)) &&
> +(s->fileversion >= 3900) && (s->bps < 24)) {
> +uint32_t crc = s->CRC_state;
> +const AVCRC *crc_tab = av_crc_get_table(AV_CRC_32_IEEE_LE);
> +for (i = 0; i < blockstodecode; i++) {
> +for (ch = 0; ch < s->channels; ch++) {
> +uint8_t *smp = frame->data[ch] + (i*(s->bps >> 3));
> +crc = av_crc(crc_tab, crc, smp, s->bps >> 3);
> +}
> +}
> +
> +if (!s->samples && ((~crc >> 1) ^ s->CRC)) {
> +av_log(avctx, AV_LOG_ERROR, "CRC mismatch! Previously decoded "
> +   "frames may have been affected as well.\n");
> +return AVERROR_INVALIDDATA;

Abort with an error only if (avctx->err_recognition & AV_EF_EXPLODE) is
true.

> +}
> +
> +s->CRC_state = crc;
> +}
> +
>  *got_frame_ptr = 1;
>  
>  return !s->samples ? avpkt->size : 0;
> -- 2.21.0
> 
> 
> ___
> 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

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

2019-03-06 Thread Carl Eugen Hoyos 
2019-03-06 12:22 GMT+01:00, Lynne :
> 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.

Imo, you should only return an error for some non-default
strict value (it should be possible for the user to show the
issue but continue to decode).

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2019-03-06 Thread Lynne 
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.
>From ad5773274448c209371dc8c70b4b21f39f968308 Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Wed, 6 Mar 2019 11:01:01 +
Subject: [PATCH] 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 | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index 15eb416ba4..3208275f6e 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 = ~0U;
 if ((ctx->fileversion > 3820) && (ctx->CRC & 0x8000)) {
 ctx->CRC &= ~0x8000;
 
@@ -1548,6 +1551,26 @@ static int ape_decode_frame(AVCodecContext *avctx, void *data,
 
 s->samples -= blockstodecode;
 
+if ((avctx->err_recognition & (AV_EF_CRCCHECK | AV_EF_COMPLIANT)) &&
+(s->fileversion >= 3900) && (s->bps < 24)) {
+uint32_t crc = s->CRC_state;
+const AVCRC *crc_tab = av_crc_get_table(AV_CRC_32_IEEE_LE);
+for (i = 0; i < blockstodecode; i++) {
+for (ch = 0; ch < s->channels; ch++) {
+uint8_t *smp = frame->data[ch] + (i*(s->bps >> 3));
+crc = av_crc(crc_tab, crc, smp, s->bps >> 3);
+}
+}
+
+if (!s->samples && ((~crc >> 1) ^ s->CRC)) {
+av_log(avctx, AV_LOG_ERROR, "CRC mismatch! Previously decoded "
+   "frames may have been affected as well.\n");
+return AVERROR_INVALIDDATA;
+}
+
+s->CRC_state = crc;
+}
+
 *got_frame_ptr = 1;
 
 return !s->samples ? avpkt->size : 0;
-- 
2.21.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel