diff mbox

[FFmpeg-devel,24/31] av1/h264_metadata: Don't reinitialize data

Message ID 20190619234521.15619-16-andreas.rheinhardt@gmail.com
State Accepted
Headers show

Commit Message

Andreas Rheinhardt June 19, 2019, 11:45 p.m. UTC 
If the relevant elements (the color description elements for AV1 and the
VUI elements in general for H.264 (since 1156b507)) are absent, then their
correct values (usually meaning unknown) have already been inferred by
the reading process, so that it is unnecessary to initialize them again
in the av1/h264_metadata filters even when they were initially absent.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/av1_metadata_bsf.c  |  7 +------
 libavcodec/h264_metadata_bsf.c | 37 ++++++++++------------------------
 2 files changed, 12 insertions(+), 32 deletions(-)

Comments

Mark Thompson July 28, 2019, 5:27 p.m. UTC | #1 
On 20/06/2019 00:45, Andreas Rheinhardt wrote:
> If the relevant elements (the color description elements for AV1 and the
> VUI elements in general for H.264 (since 1156b507)) are absent, then their
> correct values (usually meaning unknown) have already been inferred by
> the reading process, so that it is unnecessary to initialize them again
> in the av1/h264_metadata filters even when they were initially absent.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/av1_metadata_bsf.c  |  7 +------
>  libavcodec/h264_metadata_bsf.c | 37 ++++++++++------------------------
>  2 files changed, 12 insertions(+), 32 deletions(-)
> 
> diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
> index 9345095277..7d9d15b1a0 100644
> --- a/libavcodec/av1_metadata_bsf.c
> +++ b/libavcodec/av1_metadata_bsf.c
> @@ -61,12 +61,7 @@ static int av1_metadata_update_sequence_header(AVBSFContext *bsf,
>      if (ctx->color_primaries >= 0          ||
>          ctx->transfer_characteristics >= 0 ||
>          ctx->matrix_coefficients >= 0) {
> -        if (!clc->color_description_present_flag) {
> -            clc->color_description_present_flag = 1;
> -            clc->color_primaries          = AVCOL_PRI_UNSPECIFIED;
> -            clc->transfer_characteristics = AVCOL_TRC_UNSPECIFIED;
> -            clc->matrix_coefficients      = AVCOL_SPC_UNSPECIFIED;
> -        }
> +        clc->color_description_present_flag = 1;
>  
>          if (ctx->color_primaries >= 0)
>              clc->color_primaries = ctx->color_primaries;
> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
> index d760ee3182..40886bdde0 100644
> --- a/libavcodec/h264_metadata_bsf.c
> +++ b/libavcodec/h264_metadata_bsf.c
> @@ -122,13 +122,12 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
>          need_vui = 1;
>      }
>  
> -#define SET_OR_INFER(field, value, present_flag, infer) do { \
> -        if (value >= 0) { \
> -            field = value; \
> +#define SET_IF_SET(field) do { \

The name SET_IF_SET felt unnecessarily confusing, so I changed it to SET_VUI_FIELD.

Applied along with 20-22.

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
index 9345095277..7d9d15b1a0 100644
--- a/libavcodec/av1_metadata_bsf.c
+++ b/libavcodec/av1_metadata_bsf.c
@@ -61,12 +61,7 @@  static int av1_metadata_update_sequence_header(AVBSFContext *bsf,
     if (ctx->color_primaries >= 0          ||
         ctx->transfer_characteristics >= 0 ||
         ctx->matrix_coefficients >= 0) {
-        if (!clc->color_description_present_flag) {
-            clc->color_description_present_flag = 1;
-            clc->color_primaries          = AVCOL_PRI_UNSPECIFIED;
-            clc->transfer_characteristics = AVCOL_TRC_UNSPECIFIED;
-            clc->matrix_coefficients      = AVCOL_SPC_UNSPECIFIED;
-        }
+        clc->color_description_present_flag = 1;
 
         if (ctx->color_primaries >= 0)
             clc->color_primaries = ctx->color_primaries;
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index d760ee3182..40886bdde0 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -122,13 +122,12 @@  static int h264_metadata_update_sps(AVBSFContext *bsf,
         need_vui = 1;
     }
 
-#define SET_OR_INFER(field, value, present_flag, infer) do { \
-        if (value >= 0) { \
-            field = value; \
+#define SET_IF_SET(field) do { \
+        if (ctx->field >= 0) { \
+            sps->vui.field = ctx->field; \
             need_vui = 1; \
-        } else if (!present_flag) \
-            field = infer; \
-    } while (0)
+        } \
+        } while (0)
 
     if (ctx->video_format             >= 0 ||
         ctx->video_full_range_flag    >= 0 ||
@@ -136,33 +135,21 @@  static int h264_metadata_update_sps(AVBSFContext *bsf,
         ctx->transfer_characteristics >= 0 ||
         ctx->matrix_coefficients      >= 0) {
 
-        SET_OR_INFER(sps->vui.video_format, ctx->video_format,
-                     sps->vui.video_signal_type_present_flag, 5);
+        SET_IF_SET(video_format);
 
-        SET_OR_INFER(sps->vui.video_full_range_flag,
-                     ctx->video_full_range_flag,
-                     sps->vui.video_signal_type_present_flag, 0);
+        SET_IF_SET(video_full_range_flag);
 
         if (ctx->colour_primaries         >= 0 ||
             ctx->transfer_characteristics >= 0 ||
             ctx->matrix_coefficients      >= 0) {
 
-            SET_OR_INFER(sps->vui.colour_primaries,
-                         ctx->colour_primaries,
-                         sps->vui.colour_description_present_flag, 2);
-
-            SET_OR_INFER(sps->vui.transfer_characteristics,
-                         ctx->transfer_characteristics,
-                         sps->vui.colour_description_present_flag, 2);
-
-            SET_OR_INFER(sps->vui.matrix_coefficients,
-                         ctx->matrix_coefficients,
-                         sps->vui.colour_description_present_flag, 2);
+            SET_IF_SET(colour_primaries);
+            SET_IF_SET(transfer_characteristics);
+            SET_IF_SET(matrix_coefficients);
 
             sps->vui.colour_description_present_flag = 1;
         }
         sps->vui.video_signal_type_present_flag = 1;
-        need_vui = 1;
     }
 
     if (ctx->chroma_sample_loc_type >= 0) {
@@ -186,9 +173,7 @@  static int h264_metadata_update_sps(AVBSFContext *bsf,
         sps->vui.timing_info_present_flag = 1;
         need_vui = 1;
     }
-    SET_OR_INFER(sps->vui.fixed_frame_rate_flag,
-                 ctx->fixed_frame_rate_flag,
-                 sps->vui.timing_info_present_flag, 0);
+    SET_IF_SET(fixed_frame_rate_flag);
 
     if (sps->separate_colour_plane_flag || sps->chroma_format_idc == 0) {
         crop_unit_x = 1;