diff mbox series

[FFmpeg-devel,1/3] avcodec/cbs_av1: add derived frame size to AV1RawFrameHeader

Message ID 20200823020218.12659-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec/cbs_av1: add derived frame size to AV1RawFrameHeader | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

James Almer Aug. 23, 2020, 2:02 a.m. UTC 
Same logic as tile_cols and tile_rows, this saves CBS users having to
recalculate these values, and makes them available for all frames within a
Temporal Unit.

Signed-off-by: James Almer <jamrial@gmail.com>
---
Not a fan of adding more derived values to "raw" structs, but much like the
tile ones, these will be useful.

 libavcodec/cbs_av1.h                 |  8 +++++++
 libavcodec/cbs_av1_syntax_template.c | 32 ++++++++++++++--------------
 2 files changed, 24 insertions(+), 16 deletions(-)

Comments

Mark Thompson Aug. 23, 2020, 11:36 a.m. UTC | #1 
On 23/08/2020 03:02, James Almer wrote:
> Same logic as tile_cols and tile_rows, this saves CBS users having to
> recalculate these values, and makes them available for all frames within a
> Temporal Unit.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Not a fan of adding more derived values to "raw" structs, but much like the
> tile ones, these will be useful.

Perhaps it would be cleaner to avoid adding anything by inferring values for the existing frame_width_minus_1 / etc. fields in the cases where they aren't directly in the bitstream?  (In the same way the tile ones do.)

>   libavcodec/cbs_av1.h                 |  8 +++++++
>   libavcodec/cbs_av1_syntax_template.c | 32 ++++++++++++++--------------
>   2 files changed, 24 insertions(+), 16 deletions(-)
> 
> ...
- Mark
James Almer Aug. 23, 2020, 1:32 p.m. UTC | #2 
On 8/23/2020 8:36 AM, Mark Thompson wrote:
> On 23/08/2020 03:02, James Almer wrote:
>> Same logic as tile_cols and tile_rows, this saves CBS users having to
>> recalculate these values, and makes them available for all frames
>> within a
>> Temporal Unit.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Not a fan of adding more derived values to "raw" structs, but much
>> like the
>> tile ones, these will be useful.
> 
> Perhaps it would be cleaner to avoid adding anything by inferring values
> for the existing frame_width_minus_1 / etc. fields in the cases where
> they aren't directly in the bitstream?  (In the same way the tile ones do.)

I suppose that if frame_size_override_flag is left as 0, inferring such
values into frame_width_minus_1 will not result in them being written to
the output.

But what about upscaled_width and frame_width? We can only keep one in
frame_width_minus_1.

> 
>>   libavcodec/cbs_av1.h                 |  8 +++++++
>>   libavcodec/cbs_av1_syntax_template.c | 32 ++++++++++++++--------------
>>   2 files changed, 24 insertions(+), 16 deletions(-)
>>
>> ...
> - 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".
diff mbox series

Patch

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index f5fed220a5..4482498eb3 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -161,6 +161,14 @@  typedef struct AV1RawFrameHeader {
     uint8_t  render_width_minus_1;
     uint8_t  render_height_minus_1;
 
+    // These are derived values, but it's very unhelpful to have to
+    // recalculate them all the time so we store them here.
+    uint32_t upscaled_width;
+    uint32_t frame_width;
+    uint32_t frame_height;
+    uint32_t render_width;
+    uint32_t render_height;
+
     uint8_t found_ref[AV1_REFS_PER_FRAME];
 
     uint8_t refresh_frame_flags;
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
index a315e8868a..27d04375fc 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -468,9 +468,9 @@  static int FUNC(superres_params)(CodedBitstreamContext *ctx, RWContext *rw,
         denom = AV1_SUPERRES_NUM;
     }
 
-    priv->upscaled_width = priv->frame_width;
-    priv->frame_width = (priv->upscaled_width * AV1_SUPERRES_NUM +
-                         denom / 2) / denom;
+    priv->upscaled_width = current->upscaled_width = priv->frame_width;
+    priv->frame_width = current->frame_width = (priv->upscaled_width * AV1_SUPERRES_NUM +
+                                                denom / 2) / denom;
 
     return 0;
 }
@@ -486,11 +486,11 @@  static int FUNC(frame_size)(CodedBitstreamContext *ctx, RWContext *rw,
         fb(seq->frame_width_bits_minus_1 + 1,  frame_width_minus_1);
         fb(seq->frame_height_bits_minus_1 + 1, frame_height_minus_1);
 
-        priv->frame_width  = current->frame_width_minus_1  + 1;
-        priv->frame_height = current->frame_height_minus_1 + 1;
+        priv->frame_width  = current->frame_width  = current->frame_width_minus_1  + 1;
+        priv->frame_height = current->frame_height = current->frame_height_minus_1 + 1;
     } else {
-        priv->frame_width  = seq->max_frame_width_minus_1  + 1;
-        priv->frame_height = seq->max_frame_height_minus_1 + 1;
+        priv->frame_width  = current->frame_width  = seq->max_frame_width_minus_1  + 1;
+        priv->frame_height = current->frame_height = seq->max_frame_height_minus_1 + 1;
     }
 
     CHECK(FUNC(superres_params)(ctx, rw, current));
@@ -510,11 +510,11 @@  static int FUNC(render_size)(CodedBitstreamContext *ctx, RWContext *rw,
         fb(16, render_width_minus_1);
         fb(16, render_height_minus_1);
 
-        priv->render_width  = current->render_width_minus_1  + 1;
-        priv->render_height = current->render_height_minus_1 + 1;
+        priv->render_width  = current->render_width  = current->render_width_minus_1  + 1;
+        priv->render_height = current->render_height = current->render_height_minus_1 + 1;
     } else {
-        priv->render_width  = priv->upscaled_width;
-        priv->render_height = priv->frame_height;
+        priv->render_width  = current->render_width  = current->upscaled_width;
+        priv->render_height = current->render_height = current->frame_height;
     }
 
     return 0;
@@ -540,11 +540,11 @@  static int FUNC(frame_size_with_refs)(CodedBitstreamContext *ctx, RWContext *rw,
                 return AVERROR_INVALIDDATA;
             }
 
-            priv->upscaled_width = ref->upscaled_width;
-            priv->frame_width    = ref->frame_width;
-            priv->frame_height   = ref->frame_height;
-            priv->render_width   = ref->render_width;
-            priv->render_height  = ref->render_height;
+            priv->upscaled_width = current->upscaled_width = ref->upscaled_width;
+            priv->frame_width    = current->frame_width    = ref->frame_width;
+            priv->frame_height   = current->frame_height   = ref->frame_height;
+            priv->render_width   = current->render_width   = ref->render_width;
+            priv->render_height  = current->render_height  = ref->render_height;
             break;
         }
     }