[FFmpeg-devel] [PATCH 1/2] avcodec/cbs_av1: rename the private frame_header fields to frame_header_data

[FFmpeg-devel] [PATCH 1/2] avcodec/cbs_av1: rename the private frame_header fields to frame_header_data

[James Almer]

It doesn't contain the decomposed struct, but the raw bitstream.
This is in preparation for the following patch.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/cbs_av1.c                 |  6 +++---
 libavcodec/cbs_av1.h                 |  6 +++---
 libavcodec/cbs_av1_syntax_template.c | 28 ++++++++++++++--------------
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 1229480567..ecd775ea2a 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -1249,9 +1249,9 @@ static void cbs_av1_flush(CodedBitstreamContext *ctx)
 {
     CodedBitstreamAV1Context *priv = ctx->priv_data;
 
-    av_buffer_unref(&priv->frame_header_ref);
+    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));
     priv->operating_point_idc = 0;
@@ -1264,7 +1264,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);
 }
 
 static void cbs_av1_free_metadata(void *unit, uint8_t *content)
diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 1fc80dcfa0..d4776b7a30 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -431,9 +431,9 @@ typedef struct CodedBitstreamAV1Context {
     AVBufferRef          *sequence_header_ref;
 
     int     seen_frame_header;
-    AVBufferRef *frame_header_ref;
-    uint8_t     *frame_header;
-    size_t       frame_header_size;
+    AVBufferRef *frame_header_data_ref;
+    uint8_t     *frame_header_data;
+    size_t       frame_header_data_size;
 
     int temporal_id;
     int spatial_id;
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
index d98d3d42de..bd50cfbe38 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1698,12 +1698,12 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw,
 
             HEADER("Redundant Frame Header");
 
-            av_assert0(priv->frame_header_ref && priv->frame_header);
+            av_assert0(priv->frame_header_data_ref && priv->frame_header_data);
 
-            init_get_bits(&fh, priv->frame_header,
-                          priv->frame_header_size);
-            for (i = 0; i < priv->frame_header_size; i += 8) {
-                b = FFMIN(priv->frame_header_size - i, 8);
+            init_get_bits(&fh, priv->frame_header_data,
+                          priv->frame_header_data_size);
+            for (i = 0; i < priv->frame_header_data_size; i += 8) {
+                b = FFMIN(priv->frame_header_data_size - i, 8);
                 val = get_bits(&fh, b);
                 xf(b, frame_header_copy[i],
                    val, val, val, 1, i / 8);
@@ -1730,7 +1730,7 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw,
         } else {
             priv->seen_frame_header = 1;
 
-            av_buffer_unref(&priv->frame_header_ref);
+            av_buffer_unref(&priv->frame_header_data_ref);
 
 #ifdef READ
             fh_bits  = get_bits_count(rw) - start_pos;
@@ -1748,20 +1748,20 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw,
 #endif
             fh_bytes = (fh_bits + 7) / 8;
 
-            priv->frame_header_size = fh_bits;
+            priv->frame_header_data_size = fh_bits;
 
             if (rw_buffer_ref) {
-                priv->frame_header_ref = av_buffer_ref(rw_buffer_ref);
-                if (!priv->frame_header_ref)
+                priv->frame_header_data_ref = av_buffer_ref(rw_buffer_ref);
+                if (!priv->frame_header_data_ref)
                     return AVERROR(ENOMEM);
-                priv->frame_header = fh_start;
+                priv->frame_header_data = fh_start;
             } else {
-                priv->frame_header_ref =
+                priv->frame_header_data_ref =
                     av_buffer_alloc(fh_bytes + AV_INPUT_BUFFER_PADDING_SIZE);
-                if (!priv->frame_header_ref)
+                if (!priv->frame_header_data_ref)
                     return AVERROR(ENOMEM);
-                priv->frame_header = priv->frame_header_ref->data;
-                memcpy(priv->frame_header, fh_start, fh_bytes);
+                priv->frame_header_data = priv->frame_header_data_ref->data;
+                memcpy(priv->frame_header_data, fh_start, fh_bytes);
             }
         }
     }
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[James Almer]

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(+)

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
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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 <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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".