Re: [FFmpeg-devel] [PATCH 2/2] avcodec/cbs_av1: also copy the last frame header's decomposed content when parsing redundant frame headers

[Mark Thompson]

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 <jamr...@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
_______________________________________________
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".