Hi Traneptora On Wed, Sep 25, 2024 at 06:52:52PM -0400, Leo Izen wrote: > On 9/25/24 4:06 AM, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer <[email protected]> > > --- > > libavcodec/ffv1.h | 1 + > > libavcodec/ffv1dec.c | 10 ++++++---- > > libavcodec/ffv1enc.c | 12 +++++++++--- > > 3 files changed, 16 insertions(+), 7 deletions(-) > > > > diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h > > index 9aa04529228..06125d2be5f 100644 > > --- a/libavcodec/ffv1.h > > +++ b/libavcodec/ffv1.h > > @@ -118,6 +118,7 @@ typedef struct FFV1Context { > > int64_t picture_number; > > int key_frame; > > ProgressFrame picture, last_picture; > > + int crcref; > > Do we require sizeof(int) == 4? > May be more readable to declare this as an int32_t anyway (or even uint32_t) > to make it more clear that it's a 32-bit value and not a flag, especially > considering that av_crc takes uint32_t as an argument.
will become uint32_t
>
>
> > const AVFrame *cur_enc_frame;
> > int plane_count;
> > diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> > index 0afdeabd915..2463d2b934e 100644
> > --- a/libavcodec/ffv1dec.c
> > +++ b/libavcodec/ffv1dec.c
> > @@ -502,15 +502,17 @@ static int read_extra_header(FFV1Context *f)
> > if (f->version > 2) {
> > f->ec = get_symbol(&c, state, 0);
> > + if (f->ec >= 2)
> > + f->crcref = -1;
>
> How backward-compatible is this? Or is it not, which is why it needs a new
> version?
An old decoder encountering a file using ec=2 will likely detect every CRC
protected
block as damaged and discard them, thus not decoding the file. Technically a
decoder
could be smart and ignore the new unknown ec type but thats not what our
decoder does
>
> > if (f->micro_version > 2)
> > f->intra = get_symbol(&c, state, 0);
> > }
> > if (f->version > 2) {
> > unsigned v;
> > - v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0,
> > + v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), f->crcref,
> > f->avctx->extradata, f->avctx->extradata_size);
> > - if (v || f->avctx->extradata_size < 4) {
> > + if (v != f->crcref || f->avctx->extradata_size < 4) {
> > av_log(f->avctx, AV_LOG_ERROR, "CRC mismatch %X!\n", v);
> > return AVERROR_INVALIDDATA;
> > }
> > @@ -948,8 +950,8 @@ static int decode_frame(AVCodecContext *avctx, AVFrame
> > *rframe,
> > buf_p -= v;
> > if (f->ec) {
> > - unsigned crc = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0,
> > buf_p, v);
> > - if (crc) {
> > + unsigned crc = av_crc(av_crc_get_table(AV_CRC_32_IEEE),
> > f->crcref, buf_p, v);
>
> Do we require sizeof(unsigned int) == 4?
no
> Whether or not, it may be more readable to declare crc as a uint32_t
maybe, but that belongs in a seperate patch
>
>
> > + if (crc != f->crcref) {
> > int64_t ts = avpkt->pts != AV_NOPTS_VALUE ? avpkt->pts :
> > avpkt->dts;
> > av_log(f->avctx, AV_LOG_ERROR, "slice CRC mismatch %X!",
> > crc);
> > if (ts != AV_NOPTS_VALUE && avctx->pkt_timebase.num) {
> > diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
> > index a6f405289eb..f4bbdd9b943 100644
> > --- a/libavcodec/ffv1enc.c
> > +++ b/libavcodec/ffv1enc.c
> > @@ -458,7 +458,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->crcref,
> > f->avctx->extradata, f->avctx->extradata_size) ^ (f->crcref&0x4964AF46);
>
> Nitpick: should probably be (f->crcref & 0x4964af46), the space makes it a
> bit more readable.
code is different in the next revission
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
