Message ID | 20220328010851.1193-2-jamrial@gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/2] avcodec/cbs_av1: rename the private frame_header fields to frame_header_data | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_aarch64_jetson | success | Make finished |
andriy/make_fate_aarch64_jetson | success | Make fate finished |
andriy/make_armv7_RPi4 | success | Make finished |
andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On 28/03/2022 02:08, James Almer wrote: > This prevents CBS from propagating zeroed AV1RawFrameHeader units in reading > scenarios. > Writing scenarios remain unaffected as the content of these units is not used > to assemble the bitstream. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/cbs_av1.c | 11 +++++++++++ > libavcodec/cbs_av1.h | 2 ++ > libavcodec/cbs_av1_syntax_template.c | 6 ++++++ > 3 files changed, 19 insertions(+) I took me a while to work out what the issue was here, maybe make it a bit clearer that the problematic thing is that decomposed unit content for the redundant frame header OBU isn't filled if there was a frame header (in which case that content is never used, but someone could technically look at it if they wanted and observe that it is wrong). > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c > index ecd775ea2a..6cb832210c 100644 > --- a/libavcodec/cbs_av1.c > +++ b/libavcodec/cbs_av1.c > @@ -969,6 +969,15 @@ static int cbs_av1_read_unit(CodedBitstreamContext *ctx, > unit->data_ref); > if (err < 0) > return err; > + > + if (priv->frame_header) > + break; > + > + av_buffer_unref(&priv->frame_header_ref); > + priv->frame_header_ref = av_buffer_ref(unit->content_ref); > + if (!priv->frame_header_ref) > + return AVERROR(ENOMEM); > + priv->frame_header = &obu->obu.frame_header; > } > break; > case AV1_OBU_TILE_GROUP: > @@ -1251,6 +1260,7 @@ static void cbs_av1_flush(CodedBitstreamContext *ctx) > > av_buffer_unref(&priv->frame_header_data_ref); > priv->sequence_header = NULL; > + priv->frame_header = NULL; > priv->frame_header_data = NULL; > > memset(priv->ref, 0, sizeof(priv->ref)); > @@ -1264,6 +1274,7 @@ static void cbs_av1_close(CodedBitstreamContext *ctx) > CodedBitstreamAV1Context *priv = ctx->priv_data; > > av_buffer_unref(&priv->sequence_header_ref); > + av_buffer_unref(&priv->frame_header_ref); > av_buffer_unref(&priv->frame_header_data_ref); > } > > diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h > index d4776b7a30..138b273470 100644 > --- a/libavcodec/cbs_av1.h > +++ b/libavcodec/cbs_av1.h > @@ -431,6 +431,8 @@ typedef struct CodedBitstreamAV1Context { > AVBufferRef *sequence_header_ref; > > int seen_frame_header; > + AVBufferRef *frame_header_ref; > + AV1RawFrameHeader *frame_header; > AVBufferRef *frame_header_data_ref; > uint8_t *frame_header_data; > size_t frame_header_data_size; > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c > index bd50cfbe38..aadfa34b3c 100644 > --- a/libavcodec/cbs_av1_syntax_template.c > +++ b/libavcodec/cbs_av1_syntax_template.c > @@ -1708,6 +1708,11 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, > xf(b, frame_header_copy[i], > val, val, val, 1, i / 8); > } > + > +#ifdef READ > + av_assert0(priv->frame_header_ref && priv->frame_header); > + memcpy(current, priv->frame_header, sizeof(*current)); > +#endif > } > } else { > if (redundant) > @@ -1730,6 +1735,7 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, > } else { > priv->seen_frame_header = 1; > > + priv->frame_header = NULL; > av_buffer_unref(&priv->frame_header_data_ref); > > #ifdef READ LGTM in any case. (Previous patch fine too given this one.) - Mark
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c index ecd775ea2a..6cb832210c 100644 --- a/libavcodec/cbs_av1.c +++ b/libavcodec/cbs_av1.c @@ -969,6 +969,15 @@ static int cbs_av1_read_unit(CodedBitstreamContext *ctx, unit->data_ref); if (err < 0) return err; + + if (priv->frame_header) + break; + + av_buffer_unref(&priv->frame_header_ref); + priv->frame_header_ref = av_buffer_ref(unit->content_ref); + if (!priv->frame_header_ref) + return AVERROR(ENOMEM); + priv->frame_header = &obu->obu.frame_header; } break; case AV1_OBU_TILE_GROUP: @@ -1251,6 +1260,7 @@ static void cbs_av1_flush(CodedBitstreamContext *ctx) av_buffer_unref(&priv->frame_header_data_ref); priv->sequence_header = NULL; + priv->frame_header = NULL; priv->frame_header_data = NULL; memset(priv->ref, 0, sizeof(priv->ref)); @@ -1264,6 +1274,7 @@ static void cbs_av1_close(CodedBitstreamContext *ctx) CodedBitstreamAV1Context *priv = ctx->priv_data; av_buffer_unref(&priv->sequence_header_ref); + av_buffer_unref(&priv->frame_header_ref); av_buffer_unref(&priv->frame_header_data_ref); } diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h index d4776b7a30..138b273470 100644 --- a/libavcodec/cbs_av1.h +++ b/libavcodec/cbs_av1.h @@ -431,6 +431,8 @@ typedef struct CodedBitstreamAV1Context { AVBufferRef *sequence_header_ref; int seen_frame_header; + AVBufferRef *frame_header_ref; + AV1RawFrameHeader *frame_header; AVBufferRef *frame_header_data_ref; uint8_t *frame_header_data; size_t frame_header_data_size; diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index bd50cfbe38..aadfa34b3c 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -1708,6 +1708,11 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, xf(b, frame_header_copy[i], val, val, val, 1, i / 8); } + +#ifdef READ + av_assert0(priv->frame_header_ref && priv->frame_header); + memcpy(current, priv->frame_header, sizeof(*current)); +#endif } } else { if (redundant) @@ -1730,6 +1735,7 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, } else { priv->seen_frame_header = 1; + priv->frame_header = NULL; av_buffer_unref(&priv->frame_header_data_ref); #ifdef READ
This prevents CBS from propagating zeroed AV1RawFrameHeader units in reading scenarios. Writing scenarios remain unaffected as the content of these units is not used to assemble the bitstream. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/cbs_av1.c | 11 +++++++++++ libavcodec/cbs_av1.h | 2 ++ libavcodec/cbs_av1_syntax_template.c | 6 ++++++ 3 files changed, 19 insertions(+)