| Message ID | 20191017222515.21382-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
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?
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
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 [...]
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; }
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(-)