[FFmpeg-devel,2/4] avcodec/ffv1: Implementation of the CRC proposal for v4

Submitted by Michael Niedermayer on Oct. 17, 2019, 10:25 p.m.

Details

Message ID 20191017222515.21382-2-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Oct. 17, 2019, 10:25 p.m.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/ffv1.h    |  1 +
 libavcodec/ffv1dec.c | 10 +++++++---
 libavcodec/ffv1enc.c | 10 +++++++---
 3 files changed, 15 insertions(+), 6 deletions(-)

Comments

Lynne Oct. 18, 2019, 1:09 a.m.
Oct 17, 2019, 23:25 by michael@niedermayer.cc:
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/ffv1.h    |  1 +
 libavcodec/ffv1dec.c | 10 +++++++---
 libavcodec/ffv1enc.c | 10 +++++++---
 3 files changed, 15 insertions(+), 6 deletions(-)

Why 0x4964AF46 instead of 0xFFFFFFFF?
Michael Niedermayer Oct. 18, 2019, 6:24 p.m.
On Fri, Oct 18, 2019 at 03:09:48AM +0200, Lynne wrote:
> Oct 17, 2019, 23:25 by michael@niedermayer.cc:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/ffv1.h    |  1 +
>  libavcodec/ffv1dec.c | 10 +++++++---
>  libavcodec/ffv1enc.c | 10 +++++++---
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> Why 0x4964AF46 instead of 0xFFFFFFFF?

CRC32 of 0x4964AF46 is 0xFFFFFFFF

its effect is just to apply a 0xFFFFFFFF flip where its needed on the decoder
side

This is the result of building a block [data] + [32bit checksum] which as a
whole has a CRC of 0.
So the code can be made simpler on the decoder side, the checksum no longer
represents a special case and if you want to apply error correction also
the checksum is not a special case

If theres a more beautifull way to achive the same then iam certainly
interrested in that

Thanks

--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk
Michael Niedermayer Nov. 9, 2019, 3:06 p.m.
On Fri, Oct 18, 2019 at 08:24:21PM +0200, Michael Niedermayer wrote:
> On Fri, Oct 18, 2019 at 03:09:48AM +0200, Lynne wrote:
> > Oct 17, 2019, 23:25 by michael@niedermayer.cc:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/ffv1.h    |  1 +
> >  libavcodec/ffv1dec.c | 10 +++++++---
> >  libavcodec/ffv1enc.c | 10 +++++++---
> >  3 files changed, 15 insertions(+), 6 deletions(-)
> > 
> > Why 0x4964AF46 instead of 0xFFFFFFFF?
> 
> CRC32 of 0x4964AF46 is 0xFFFFFFFF
> 
> its effect is just to apply a 0xFFFFFFFF flip where its needed on the decoder
> side
> 
> This is the result of building a block [data] + [32bit checksum] which as a
> whole has a CRC of 0.
> So the code can be made simpler on the decoder side, the checksum no longer
> represents a special case and if you want to apply error correction also
> the checksum is not a special case
> 
> If theres a more beautifull way to achive the same then iam certainly
> interrested in that

will apply all patches of this set except this.

I intend to apply this one when the specification is updated

thx

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index f0bb19350a..3edf9dfca3 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -114,6 +114,7 @@  typedef struct FFV1Context {
     int use32bit;
 
     int ec;
+    unsigned crc_flip;
     int intra;
     int slice_damaged;
     int key_frame_ok;
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 261e0cf70c..bc2b776754 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -500,8 +500,12 @@  static int read_extra_header(FFV1Context *f)
 
     if (f->version > 2) {
         unsigned v;
-        v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0,
-                   f->avctx->extradata, f->avctx->extradata_size);
+
+        if (f->version > 4 || (f->version == 4 && f->micro_version >= 3))
+            f->crc_flip = 0xFFFFFFFF;
+
+        v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), f->crc_flip,
+                   f->avctx->extradata, f->avctx->extradata_size) ^ f->crc_flip;
         if (v || f->avctx->extradata_size < 4) {
             av_log(f->avctx, AV_LOG_ERROR, "CRC mismatch %X!\n", v);
             return AVERROR_INVALIDDATA;
@@ -903,7 +907,7 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
         buf_p -= v;
 
         if (f->ec) {
-            unsigned crc = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0, buf_p, v);
+            unsigned crc = av_crc(av_crc_get_table(AV_CRC_32_IEEE), f->crc_flip, buf_p, v) ^ f->crc_flip;
             if (crc) {
                 int64_t ts = avpkt->pts != AV_NOPTS_VALUE ? avpkt->pts : avpkt->dts;
                 av_log(f->avctx, AV_LOG_ERROR, "CRC mismatch %X!", crc);
diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index c521b7d445..4beb8fde35 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -417,7 +417,7 @@  static int write_extradata(FFV1Context *f)
         if (f->version == 3) {
             f->micro_version = 4;
         } else if (f->version == 4)
-            f->micro_version = 2;
+            f->micro_version = 3;
         put_symbol(c, state, f->micro_version, 0);
     }
 
@@ -459,7 +459,7 @@  static int write_extradata(FFV1Context *f)
     }
 
     f->avctx->extradata_size = ff_rac_terminate(c, 0);
-    v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0, f->avctx->extradata, f->avctx->extradata_size);
+    v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), f->crc_flip, f->avctx->extradata, f->avctx->extradata_size) ^ (f->crc_flip & 0x4964AF46);
     AV_WL32(f->avctx->extradata + f->avctx->extradata_size, v);
     f->avctx->extradata_size += 4;
 
@@ -898,6 +898,10 @@  FF_ENABLE_DEPRECATION_WARNINGS
                avctx->slices);
         return AVERROR(ENOSYS);
 slices_ok:
+
+        if (s->version > 3)
+            s->crc_flip = 0xFFFFFFFF;
+
         if ((ret = write_extradata(s)) < 0)
             return ret;
     }
@@ -1254,7 +1258,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
         if (f->ec) {
             unsigned v;
             buf_p[bytes++] = 0;
-            v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0, buf_p, bytes);
+            v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), f->crc_flip, buf_p, bytes) ^ (f->crc_flip & 0x4964AF46);
             AV_WL32(buf_p + bytes, v);
             bytes += 4;
         }