[FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function

[FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

Resent due to my mail client incorrectly re-wrapping lines in the version I sent earlier.
See the previous patch for context.

Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
---
 libavutil/hdr_dynamic_metadata.c | 147 +++++++++++++++++++++++++++++++
 libavutil/hdr_dynamic_metadata.h |  10 +++
 libavutil/version.h              |   2 +-
 3 files changed, 158 insertions(+), 1 deletion(-)

diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..72d310e633 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,150 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
 
     return 0;
 }
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+    AVBufferRef *buf;
+    size_t size_bits, size_bytes;
+    PutBitContext pbc, *pb = &pbc;
+
+    if (!s)
+        return NULL;
+
+    // Buffer size per CTA-861-H p.253-254:
+    size_bits =
+    // 56 bits for the header
+    58 +
+    // 2 bits for num_windows
+    2 +
+    // 937 bits for window geometry for each window above 1
+    FFMAX((s->num_windows - 1), 0) * 937 +
+    // 27 bits for targeted_system_display_maximum_luminance
+    27 +
+    // 1-3855 bits for targeted system display peak luminance information
+    1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+        s->num_rows_targeted_system_display_actual_peak_luminance *
+        s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
+    // 0-442 bits for intra-window pixel distribution information
+    s->num_windows * 82;
+    for (int w = 0; w < s->num_windows; w++) {
+        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+    }
+    // 1-3855 bits for mastering display peak luminance information
+    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+        s->num_rows_mastering_display_actual_peak_luminance *
+        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+    // 0-537 bits for per-window tonemapping information
+    s->num_windows * 1;
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].tone_mapping_flag) {
+            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+        }
+    }
+    // 0-21 bits for per-window color saturation mapping information
+    size_bits += s->num_windows * 1;
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].color_saturation_mapping_flag) {
+            size_bits += 6;
+        }
+    }
+
+    size_bytes = (size_bits + 7) / 8;
+
+    buf = av_buffer_alloc(size_bytes);
+    if (!buf) {
+        return NULL;
+    }
+
+    init_put_bits(pb, buf->data, size_bytes);
+
+    // itu_t_t35_country_code shall be 0xB5 (USA)
+    put_bits(pb, 8, 0xB5);
+    // itu_t_t35_terminal_provider_code shall be 0x003C
+    put_bits(pb, 16, 0x003C);
+    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+    put_bits(pb, 16, 0x0001);
+    // application_identifier shall be set to 4
+    put_bits(pb, 8, 4);
+    // application_mode is set to Application Version 1
+    put_bits(pb, 8, 1);
+
+    // Payload as per CTA-861-H p.253-254
+    put_bits(pb, 2, s->num_windows);
+
+    for (int w = 1; w < s->num_windows; w++) {
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / s->params[w].window_upper_left_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / s->params[w].window_upper_left_corner_y.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / s->params[w].window_lower_right_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / s->params[w].window_lower_right_corner_y.den);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+        put_bits(pb, 8, s->params[w].rotation_angle);
+        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+        put_bits(pb, 1, s->params[w].overlap_process_option);
+    }
+
+    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * luminance_den /
+        s->targeted_system_display_maximum_luminance.den);
+    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, s->num_rows_targeted_system_display_actual_peak_luminance);
+        put_bits(pb, 5, s->num_cols_targeted_system_display_actual_peak_luminance);
+        for (int i = 0; i < s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < s->num_cols_targeted_system_display_actual_peak_luminance; j++) {
+                put_bits(pb, 4, s->targeted_system_display_actual_peak_luminance[i][j].num * peak_luminance_den /
+                    s->targeted_system_display_actual_peak_luminance[i][j].den);
+            }
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        for (int i = 0; i < 3; i++) {
+            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / s->params[w].maxscl[i].den);
+        }
+        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / s->params[w].average_maxrgb.den);
+        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
+        for (int i = 0; i < s->params[w].num_distribution_maxrgb_percentiles; i++) {
+            put_bits(pb, 7, s->params[w].distribution_maxrgb[i].percentage);
+            put_bits(pb, 17, s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
+                s->params[w].distribution_maxrgb[i].percentile.den);
+        }
+        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * fraction_pixel_den /
+            s->params[w].fraction_bright_pixels.den);
+    }
+
+    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, s->num_rows_mastering_display_actual_peak_luminance);
+        put_bits(pb, 5, s->num_cols_mastering_display_actual_peak_luminance);
+        for (int i = 0; i < s->num_rows_mastering_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < s->num_cols_mastering_display_actual_peak_luminance; j++) {
+                put_bits(pb, 4, s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
+                    s->mastering_display_actual_peak_luminance[i][j].den);
+            }
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        put_bits(pb, 1, s->params[w].tone_mapping_flag);
+        if (s->params[w].tone_mapping_flag) {
+            put_bits(pb, 12, s->params[w].knee_point_x.num * knee_point_den / s->params[w].knee_point_x.den);
+            put_bits(pb, 12, s->params[w].knee_point_y.num * knee_point_den / s->params[w].knee_point_y.den);
+            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
+            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; i++) {
+                put_bits(pb, 10, s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
+                    s->params[w].bezier_curve_anchors[i].den);
+            }
+            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
+            if (s->params[w].color_saturation_mapping_flag) {
+                put_bits(pb, 6, s->params[w].color_saturation_weight.num * saturation_weight_den /
+                    s->params[w].color_saturation_weight.den);
+            }
+        }
+    }
+
+    flush_put_bits(pb);
+    return buf;
+}
diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
index 1f953ef1f5..f75d3e0739 100644
--- a/libavutil/hdr_dynamic_metadata.h
+++ b/libavutil/hdr_dynamic_metadata.h
@@ -21,6 +21,7 @@
 #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
 #define AVUTIL_HDR_DYNAMIC_METADATA_H
 
+#include "buffer.h"
 #include "frame.h"
 #include "rational.h"
 
@@ -351,4 +352,13 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
 int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
                                  int size);
 
+/**
+ * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ *
+ * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
+ *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
+ */
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
+
 #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 900b798971..7635672985 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR   3
+#define LIBAVUTIL_VERSION_MINOR   4
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.39.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] avutil: add HDR10+ dynamic metadata serialization function

[quietvoid]

On 27/02/2023 12.34, Raphaël Zumer wrote:

> Resent due to my mail client incorrectly re-wrapping lines in the version I sent earlier.
> See the previous patch for context.
>
> Signed-off-by: Raphaël Zumer<rzumer@tebako.net>
> ---
>   libavutil/hdr_dynamic_metadata.c | 147 +++++++++++++++++++++++++++++++
>   libavutil/hdr_dynamic_metadata.h |  10 +++
>   libavutil/version.h              |   2 +-
>   3 files changed, 158 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
> index 98f399b032..72d310e633 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -225,3 +225,150 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>   
>       return 0;
>   }
> +
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
> +{
> +    AVBufferRef *buf;
> +    size_t size_bits, size_bytes;
> +    PutBitContext pbc, *pb = &pbc;
> +
> +    if (!s)
> +        return NULL;
> +
> +    // Buffer size per CTA-861-H p.253-254:
> +    size_bits =
> +    // 56 bits for the header
> +    58 +
> +    // 2 bits for num_windows
> +    2 +
> +    // 937 bits for window geometry for each window above 1
> +    FFMAX((s->num_windows - 1), 0) * 937 +
> +    // 27 bits for targeted_system_display_maximum_luminance
> +    27 +
> +    // 1-3855 bits for targeted system display peak luminance information
> +    1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
> +        s->num_rows_targeted_system_display_actual_peak_luminance *
> +        s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
> +    // 0-442 bits for intra-window pixel distribution information
> +    s->num_windows * 82;
> +    for (int w = 0; w < s->num_windows; w++) {
> +        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
> +    }
> +    // 1-3855 bits for mastering display peak luminance information
> +    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
> +        s->num_rows_mastering_display_actual_peak_luminance *
> +        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
> +    // 0-537 bits for per-window tonemapping information
> +    s->num_windows * 1;
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].tone_mapping_flag) {
> +            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
> +        }
> +    }
> +    // 0-21 bits for per-window color saturation mapping information
> +    size_bits += s->num_windows * 1;
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].color_saturation_mapping_flag) {
> +            size_bits += 6;
> +        }
> +    }
> +
> +    size_bytes = (size_bits + 7) / 8;
> +
> +    buf = av_buffer_alloc(size_bytes);
> +    if (!buf) {
> +        return NULL;
> +    }
> +
> +    init_put_bits(pb, buf->data, size_bytes);
> +
> +    // itu_t_t35_country_code shall be 0xB5 (USA)
> +    put_bits(pb, 8, 0xB5);

This patch series mentions that it would be used for AV1, however the AV1
specification is clear that the payload does not contain the country code.
https://aomediacodec.github.io/av1-hdr10plus/#hdr10plus-metadata, Figure 1.

So far all the AV1 HDR10+ conformance samples also respect this.
There would probably be a need to specify which behaviour is wanted.
The SEI for HEVC does keep the country code in the payload, but not AV1.

> +    // itu_t_t35_terminal_provider_code shall be 0x003C
> +    put_bits(pb, 16, 0x003C);
> +    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
> +    put_bits(pb, 16, 0x0001);
> +    // application_identifier shall be set to 4
> +    put_bits(pb, 8, 4);
> +    // application_mode is set to Application Version 1
> +    put_bits(pb, 8, 1);
> +
> +    // Payload as per CTA-861-H p.253-254
> +    put_bits(pb, 2, s->num_windows);
> +
> +    for (int w = 1; w < s->num_windows; w++) {
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / s->params[w].window_upper_left_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / s->params[w].window_upper_left_corner_y.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / s->params[w].window_lower_right_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / s->params[w].window_lower_right_corner_y.den);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
> +        put_bits(pb, 8, s->params[w].rotation_angle);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
> +        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
> +        put_bits(pb, 1, s->params[w].overlap_process_option);
> +    }
> +
> +    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * luminance_den /
> +        s->targeted_system_display_maximum_luminance.den);
> +    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
> +    if (s->targeted_system_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_targeted_system_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_targeted_system_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_targeted_system_display_actual_peak_luminance; j++) {
> +                put_bits(pb, 4, s->targeted_system_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->targeted_system_display_actual_peak_luminance[i][j].den);
> +            }
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        for (int i = 0; i < 3; i++) {
> +            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / s->params[w].maxscl[i].den);
> +        }
> +        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / s->params[w].average_maxrgb.den);
> +        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
> +        for (int i = 0; i < s->params[w].num_distribution_maxrgb_percentiles; i++) {
> +            put_bits(pb, 7, s->params[w].distribution_maxrgb[i].percentage);
> +            put_bits(pb, 17, s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
> +                s->params[w].distribution_maxrgb[i].percentile.den);
> +        }
> +        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * fraction_pixel_den /
> +            s->params[w].fraction_bright_pixels.den);
> +    }
> +
> +    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
> +    if (s->mastering_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_mastering_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_mastering_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_mastering_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_mastering_display_actual_peak_luminance; j++) {
> +                put_bits(pb, 4, s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->mastering_display_actual_peak_luminance[i][j].den);
> +            }
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        put_bits(pb, 1, s->params[w].tone_mapping_flag);
> +        if (s->params[w].tone_mapping_flag) {
> +            put_bits(pb, 12, s->params[w].knee_point_x.num * knee_point_den / s->params[w].knee_point_x.den);
> +            put_bits(pb, 12, s->params[w].knee_point_y.num * knee_point_den / s->params[w].knee_point_y.den);
> +            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
> +            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; i++) {
> +                put_bits(pb, 10, s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
> +                    s->params[w].bezier_curve_anchors[i].den);
> +            }
> +            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
> +            if (s->params[w].color_saturation_mapping_flag) {
> +                put_bits(pb, 6, s->params[w].color_saturation_weight.num * saturation_weight_den /
> +                    s->params[w].color_saturation_weight.den);
> +            }
> +        }
> +    }
> +
> +    flush_put_bits(pb);
> +    return buf;
> +}
> diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
> index 1f953ef1f5..f75d3e0739 100644
> --- a/libavutil/hdr_dynamic_metadata.h
> +++ b/libavutil/hdr_dynamic_metadata.h
> @@ -21,6 +21,7 @@
>   #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
>   #define AVUTIL_HDR_DYNAMIC_METADATA_H
>   
> +#include "buffer.h"
>   #include "frame.h"
>   #include "rational.h"
>   
> @@ -351,4 +352,13 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
>   int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>                                    int size);
>   
> +/**
> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + *
> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
> + */
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
> +
>   #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 900b798971..7635672985 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>    */
>   
>   #define LIBAVUTIL_VERSION_MAJOR  58
> -#define LIBAVUTIL_VERSION_MINOR   3
> +#define LIBAVUTIL_VERSION_MINOR   4
>   #define LIBAVUTIL_VERSION_MICRO 100
>   
>   #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

On 3/2/23 13:33, quietvoid wrote:
> This patch series mentions that it would be used for AV1, however the AV1
> specification is clear that the payload does not contain the country code.
> https://aomediacodec.github.io/av1-hdr10plus/#hdr10plus-metadata, Figure 1.
>
> So far all the AV1 HDR10+ conformance samples also respect this.
> There would probably be a need to specify which behaviour is wanted.
> The SEI for HEVC does keep the country code in the payload, but not AV1.

As you pointed out, HEVC does include the country code in the payload, so either approach can be seen as reasonable. There are a few reasons why I don't think it makes sense to reproduce the AV1 specification:

- it does not make sense to exclude the country code from the payload while including other static headers like the terminal provider codes and the application identifier, so there is no particular advantage or logic that I can see in splitting the payload in that way.

- pragmatically, it is much less inconvenient to deal with a 1-byte offset in the payload for AV1 than it is to insert that byte into the payload for HEVC and other applications that expect the country code to be included in the header. For the same reason, I think that an option to include or exclude the country code byte would be more trouble than it's worth.

- perhaps most importantly, the payload syntax is standardized in ANSI/CTA-861-H, which does not make any distinction between the header (country code, terminal provider code, etc.) and the remainder of the payload, so the AV1 specification does not conform either to other implementations nor to the standard. Following the most authoritative document is the safest route in my opinion.

Raphaël Zumer

_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[James Almer]

On 3/2/2023 3:33 PM, quietvoid wrote:> On 27/02/2023 12.34, Raphaël 
Zumer wrote:
 >
 >> Resent due to my mail client incorrectly re-wrapping lines in the
 >> version I sent earlier.
 >> See the previous patch for context.
 >>
 >> Signed-off-by: Raphaël Zumer<rzumer@tebako.net>
 >> ---
 >>   libavutil/hdr_dynamic_metadata.c | 147 +++++++++++++++++++++++++++++++
 >>   libavutil/hdr_dynamic_metadata.h |  10 +++
 >>   libavutil/version.h              |   2 +-
 >>   3 files changed, 158 insertions(+), 1 deletion(-)
 >>
 >> diff --git a/libavutil/hdr_dynamic_metadata.c
 >> b/libavutil/hdr_dynamic_metadata.c
 >> index 98f399b032..72d310e633 100644
 >> --- a/libavutil/hdr_dynamic_metadata.c
 >> +++ b/libavutil/hdr_dynamic_metadata.c
 >> @@ -225,3 +225,150 @@ int
 >> av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
 >>       return 0;
 >>   }
 >> +
 >> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
 >> +{
 >> +    AVBufferRef *buf;
 >> +    size_t size_bits, size_bytes;
 >> +    PutBitContext pbc, *pb = &pbc;
 >> +
 >> +    if (!s)
 >> +        return NULL;
 >> +
 >> +    // Buffer size per CTA-861-H p.253-254:
 >> +    size_bits =
 >> +    // 56 bits for the header
 >> +    58 +
 >> +    // 2 bits for num_windows
 >> +    2 +
 >> +    // 937 bits for window geometry for each window above 1
 >> +    FFMAX((s->num_windows - 1), 0) * 937 +
 >> +    // 27 bits for targeted_system_display_maximum_luminance
 >> +    27 +
 >> +    // 1-3855 bits for targeted system display peak luminance
 >> information
 >> +    1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
 >> +        s->num_rows_targeted_system_display_actual_peak_luminance *
 >> +        s->num_cols_targeted_system_display_actual_peak_luminance * 4
 >> : 0) +
 >> +    // 0-442 bits for intra-window pixel distribution information
 >> +    s->num_windows * 82;
 >> +    for (int w = 0; w < s->num_windows; w++) {
 >> +        size_bits += s->params[w].num_distribution_maxrgb_percentiles
 >> * 24;
 >> +    }
 >> +    // 1-3855 bits for mastering display peak luminance information
 >> +    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag
 >> ? 10 +
 >> +        s->num_rows_mastering_display_actual_peak_luminance *
 >> +        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
 >> +    // 0-537 bits for per-window tonemapping information
 >> +    s->num_windows * 1;
 >> +    for (int w = 0; w < s->num_windows; w++) {
 >> +        if (s->params[w].tone_mapping_flag) {
 >> +            size_bits += 28 + s->params[w].num_bezier_curve_anchors *
 >> 10;
 >> +        }
 >> +    }
 >> +    // 0-21 bits for per-window color saturation mapping information
 >> +    size_bits += s->num_windows * 1;
 >> +    for (int w = 0; w < s->num_windows; w++) {
 >> +        if (s->params[w].color_saturation_mapping_flag) {
 >> +            size_bits += 6;
 >> +        }
 >> +    }
 >> +
 >> +    size_bytes = (size_bits + 7) / 8;
 >> +
 >> +    buf = av_buffer_alloc(size_bytes);
 >> +    if (!buf) {
 >> +        return NULL;
 >> +    }
 >> +
 >> +    init_put_bits(pb, buf->data, size_bytes);
 >> +
 >> +    // itu_t_t35_country_code shall be 0xB5 (USA)
 >> +    put_bits(pb, 8, 0xB5);
 >
 > This patch series mentions that it would be used for AV1, however the AV1
 > specification is clear that the payload does not contain the country 
code.
 > https://aomediacodec.github.io/av1-hdr10plus/#hdr10plus-metadata, 
Figure 1.
 >
 > So far all the AV1 HDR10+ conformance samples also respect this.
 > There would probably be a need to specify which behaviour is wanted.
 > The SEI for HEVC does keep the country code in the payload, but not AV1.
Are you sure about HEVC? I just checked a sample and trace_headers 
reported this:

[trace_headers] Prefix Supplemental Enhancement Information
[trace_headers] 0  forbidden_zero_bit               0 = 0
[trace_headers] 1  nal_unit_type               100111 = 39
[trace_headers] 7  nuh_layer_id                000000 = 0
[trace_headers] 13 nuh_temporal_id_plus1          001 = 1
[trace_headers] 16 last_payload_type_byte    00000100 = 4
[trace_headers] 24 last_payload_size_byte    01000000 = 64
[trace_headers] User Data Registered ITU-T T.35
[trace_headers] 32 itu_t_t35_country_code    10110101 = 181
[trace_headers] 40 itu_t_t35_payload_byte[1] 00000000 = 0
[trace_headers] 48 itu_t_t35_payload_byte[2] 00111100 = 60
[trace_headers] 56 itu_t_t35_payload_byte[3] 00000000 = 0
[trace_headers] 64 itu_t_t35_payload_byte[4] 00000001 = 1
[trace_headers] 72 itu_t_t35_payload_byte[5] 00000100 = 4
[trace_headers] 80 itu_t_t35_payload_byte[6] 00000001 = 1

Which is in line with section 8.3.1 of ITU-T Rec. H.274 as well as 
section D.2.6 of ITU-T Rec. H.265.

So i think this function should not set the country code at all as part 
of the payload, and start with itu_t_t35_terminal_provider_code.
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

On 3/2/23 13:57, James Almer wrote:
>  > The SEI for HEVC does keep the country code in the payload, but not AV1.
> Are you sure about HEVC? I just checked a sample and trace_headers 
> reported this:
>
> [trace_headers] Prefix Supplemental Enhancement Information
> [trace_headers] 0  forbidden_zero_bit               0 = 0
> [trace_headers] 1  nal_unit_type               100111 = 39
> [trace_headers] 7  nuh_layer_id                000000 = 0
> [trace_headers] 13 nuh_temporal_id_plus1          001 = 1
> [trace_headers] 16 last_payload_type_byte    00000100 = 4
> [trace_headers] 24 last_payload_size_byte    01000000 = 64
> [trace_headers] User Data Registered ITU-T T.35
> [trace_headers] 32 itu_t_t35_country_code    10110101 = 181
> [trace_headers] 40 itu_t_t35_payload_byte[1] 00000000 = 0
> [trace_headers] 48 itu_t_t35_payload_byte[2] 00111100 = 60
> [trace_headers] 56 itu_t_t35_payload_byte[3] 00000000 = 0
> [trace_headers] 64 itu_t_t35_payload_byte[4] 00000001 = 1
> [trace_headers] 72 itu_t_t35_payload_byte[5] 00000100 = 4
> [trace_headers] 80 itu_t_t35_payload_byte[6] 00000001 = 1
>
> Which is in line with section 8.3.1 of ITU-T Rec. H.274 as well as 
> section D.2.6 of ITU-T Rec. H.265.
>
> So i think this function should not set the country code at all as part 
> of the payload, and start with itu_t_t35_terminal_provider_code.

OK, I will make that change since both HEVC and AV1 implementations seem to match.

Thanks
Raphaël Zumer


_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[James Almer]

On 3/13/2023 7:25 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 3/9/2023 11:18 AM, Raphaël Zumer wrote:
>>> Hi,
>>>
>>> While I omitted adding v2/v3 here, I believe all comments on this set
>>> of patches have been addressed so far, unless anyone strongly
>>> disagrees with the rationale for moving dynamic HDR parsing and
>>> serialization to libavutil or with the function signature.
>>>
>>> Please let me know if I missed anything.
>>>
>>> Thanks,
>>> Raphaël Zumer
>>
>> I'll apply this (and patch 1/1) in a few days if nobody comments.
>> The code needs to be in lavu if we want muxers and demuxers to use this
>> functionality, so moving the existing lavc functions is fine unless we
>> add more avpriv_ functions that people tend to dislike.
> 
> Can we wait with this until we have the actual patches that make use of it?
> (And is lavfi actually supposed to make use of this? Or why is it moved
> to lavu at all? lavf can also use it in lavc.)

I have patches ready to make use of one of the two functions in the 
matroska demuxer. Will send them after this is pushed. And I'll attempt 
to write one to make use of the serialization function in the matroska 
muxer too.

And it makes sense for this to be in lavu even if only lavf uses these, 
given there's a public header for it already. Installing two headers for 
HDR10+ functions in two separate libraries seems overkill when they use 
the same struct.
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Andreas Rheinhardt]

James Almer:
> On 3/9/2023 11:18 AM, Raphaël Zumer wrote:
>> Hi,
>>
>> While I omitted adding v2/v3 here, I believe all comments on this set
>> of patches have been addressed so far, unless anyone strongly
>> disagrees with the rationale for moving dynamic HDR parsing and
>> serialization to libavutil or with the function signature.
>>
>> Please let me know if I missed anything.
>>
>> Thanks,
>> Raphaël Zumer
> 
> I'll apply this (and patch 1/1) in a few days if nobody comments.
> The code needs to be in lavu if we want muxers and demuxers to use this
> functionality, so moving the existing lavc functions is fine unless we
> add more avpriv_ functions that people tend to dislike.

Can we wait with this until we have the actual patches that make use of it?
(And is lavfi actually supposed to make use of this? Or why is it moved
to lavu at all? lavf can also use it in lavc.)

- Andreas

_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Anton Khirnov]

Quoting Raphaël Zumer (2023-03-12 22:50:27)
> I expanded on this in another email in the chain, but the buffer size needs to be communicated to the user, as it is not embedded in the payload. It seems needlessly convoluted to me to create a separate function solely to calculate the size of the buffer so that it can be allocated by the user and passed to the serialization function, and I cannot think of another solution that would not be even more convoluted and awkward for the user.
> 
> I don't understand how going from AVBufferRef to uint8_t* is more complicated than the reverse. The buffer in the AVBufferRef is allocated via av_malloc() and is directly accessible through the data field. Am I missing some detail?

What I mean is that once data is wrapped in an AVBufer, it is "locked
in", you always have to keep an AVBufferRef around.

-- 
Anton Khirnov
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

On 3/12/23 17:52, James Almer wrote:
> On 3/12/2023 6:50 PM, Raphaël Zumer wrote:
>> I expanded on this in another email in the chain, but the buffer size needs to be communicated to the user, as it is not embedded in the payload. It seems needlessly convoluted to me to create a separate function solely to calculate the size of the buffer so that it can be allocated by the user and passed to the serialization function, and I cannot think of another solution that would not be even more convoluted and awkward for the user.
>>
>> I don't understand how going from AVBufferRef to uint8_t* is more complicated than the reverse. The buffer in the AVBufferRef is allocated via av_malloc() and is directly accessible through the data field. Am I missing some detail?
>>
>> Raphaël Zumer
> You can do it like how the AVDynamicHDRPlus struct is allocated and its 
> size returned to the user in av_dynamic_hdr_plus_alloc(). That is
>
> uint8_t *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s, size_t *size);
>
> The function would then store the calculated size of the array on *size.

OK great, I will do that and address the remaining comments tomorrow.

Thanks

>> On 3/12/23 15:48, Anton Khirnov wrote:
>>> Quoting Raphaël Zumer (2023-03-02 22:43:29)
>>>> +/**
>>>> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
>>>> + * excluding the country code and beginning with the terminal provider code.
>>>> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
>>>> + *
>>>> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
>>>> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
>>>> + */
>>>> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
>>> Why is this an AVBufferRef rather than a plain av_malloced() uint8_t
>>> array? You can very easily turn the latter into the former, but the
>>> reverse is a lot more annoying.
>>>
>> _______________________________________________
>> 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 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 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] avutil: add HDR10+ dynamic metadata serialization function

[James Almer]

On 3/12/2023 6:50 PM, Raphaël Zumer wrote:
> I expanded on this in another email in the chain, but the buffer size needs to be communicated to the user, as it is not embedded in the payload. It seems needlessly convoluted to me to create a separate function solely to calculate the size of the buffer so that it can be allocated by the user and passed to the serialization function, and I cannot think of another solution that would not be even more convoluted and awkward for the user.
> 
> I don't understand how going from AVBufferRef to uint8_t* is more complicated than the reverse. The buffer in the AVBufferRef is allocated via av_malloc() and is directly accessible through the data field. Am I missing some detail?
> 
> Raphaël Zumer

You can do it like how the AVDynamicHDRPlus struct is allocated and its 
size returned to the user in av_dynamic_hdr_plus_alloc(). That is

uint8_t *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s, size_t *size);

The function would then store the calculated size of the array on *size.

> 
> On 3/12/23 15:48, Anton Khirnov wrote:
>> Quoting Raphaël Zumer (2023-03-02 22:43:29)
>>> +/**
>>> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
>>> + * excluding the country code and beginning with the terminal provider code.
>>> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
>>> + *
>>> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
>>> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
>>> + */
>>> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
>> Why is this an AVBufferRef rather than a plain av_malloced() uint8_t
>> array? You can very easily turn the latter into the former, but the
>> reverse is a lot more annoying.
>>
> _______________________________________________
> 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 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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

I expanded on this in another email in the chain, but the buffer size needs to be communicated to the user, as it is not embedded in the payload. It seems needlessly convoluted to me to create a separate function solely to calculate the size of the buffer so that it can be allocated by the user and passed to the serialization function, and I cannot think of another solution that would not be even more convoluted and awkward for the user.

I don't understand how going from AVBufferRef to uint8_t* is more complicated than the reverse. The buffer in the AVBufferRef is allocated via av_malloc() and is directly accessible through the data field. Am I missing some detail?

Raphaël Zumer

On 3/12/23 15:48, Anton Khirnov wrote:
> Quoting Raphaël Zumer (2023-03-02 22:43:29)
>> +/**
>> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
>> + * excluding the country code and beginning with the terminal provider code.
>> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
>> + *
>> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
>> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
>> + */
>> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
> Why is this an AVBufferRef rather than a plain av_malloced() uint8_t
> array? You can very easily turn the latter into the former, but the
> reverse is a lot more annoying.
>
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Anton Khirnov]

Quoting Raphaël Zumer (2023-03-02 22:43:29)
> +/**
> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
> + * excluding the country code and beginning with the terminal provider code.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + *
> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
> + */
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);

Why is this an AVBufferRef rather than a plain av_malloced() uint8_t
array? You can very easily turn the latter into the former, but the
reverse is a lot more annoying.

-- 
Anton Khirnov
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Zhao Zhili]

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Raphaël Zumer
> Sent: 2023年3月3日 5:43
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
> 
> Fixed brace style and moved inline buffer size calculation comments to a single block at the top.
> 
> 
> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
> ---
>  libavutil/hdr_dynamic_metadata.c | 142 +++++++++++++++++++++++++++++++
>  libavutil/hdr_dynamic_metadata.h |  11 +++
>  libavutil/version.h              |   2 +-
>  3 files changed, 154 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
> index 98f399b032..24dd3dab2d 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -225,3 +225,145 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
> 
>      return 0;
>  }
> +
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
> +{

How about add 'const' modifier to AVDynamicHDRPlus *s ?

> +    AVBufferRef *buf;
> +    size_t size_bits, size_bytes;
> +    PutBitContext pbc, *pb = &pbc;
> +
> +    if (!s)
> +        return NULL;
> +
> +    /**
> +     * Buffer size per CTA-861-H p.253-254:
> +     * 48 bits for the header (56 minus excluded 8-bit country code)
> +     * 2 bits for num_windows
> +     * 937 bits for window geometry, for each window above 1
> +     * 27 bits for targeted_system_display_maximum_luminance
> +     * 1-3855 bits for targeted system display peak luminance information
> +     * 0-442 bits for intra-window pixel distribution information
> +     * 1-3855 bits for mastering display peak luminance information
> +     * 0-537 bits for per-window tonemapping information
> +     * 0-21 bits for per-window color saturation mapping information
> +     */
> +    size_bits = 48 +
> +        2 +
> +        FFMAX((s->num_windows - 1), 0) * 937 +
> +        27 +
> +        1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
> +            s->num_rows_targeted_system_display_actual_peak_luminance *
> +            s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
> +        s->num_windows * 82;
> +
> +    for (int w = 0; w < s->num_windows; w++)
> +        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
> +
> +    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
> +        s->num_rows_mastering_display_actual_peak_luminance *
> +        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
> +        s->num_windows * 1;
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].tone_mapping_flag)
> +            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
> +    }
> +
> +    size_bits += s->num_windows * 1;
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].color_saturation_mapping_flag)
> +            size_bits += 6;
> +    }
> +
> +    size_bytes = (size_bits + 7) / 8;
> +
> +    buf = av_buffer_alloc(size_bytes);
> +    if (!buf)
> +        return NULL;
> +
> +    init_put_bits(pb, buf->data, size_bytes);
> +
> +    // itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
> +    // itu_t_t35_terminal_provider_code shall be 0x003C
> +    put_bits(pb, 16, 0x003C);
> +    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
> +    put_bits(pb, 16, 0x0001);
> +    // application_identifier shall be set to 4
> +    put_bits(pb, 8, 4);
> +    // application_mode is set to Application Version 1
> +    put_bits(pb, 8, 1);
> +
> +    // Payload as per CTA-861-H p.253-254
> +    put_bits(pb, 2, s->num_windows);
> +
> +    for (int w = 1; w < s->num_windows; w++) {
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / s->params[w].window_upper_left_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / s->params[w].window_upper_left_corner_y.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / s->params[w].window_lower_right_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / s->params[w].window_lower_right_corner_y.den);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
> +        put_bits(pb, 8, s->params[w].rotation_angle);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
> +        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
> +        put_bits(pb, 1, s->params[w].overlap_process_option);
> +    }
> +
> +    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * luminance_den /
> +        s->targeted_system_display_maximum_luminance.den);
> +    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
> +    if (s->targeted_system_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_targeted_system_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_targeted_system_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_targeted_system_display_actual_peak_luminance; j++)
> +                put_bits(pb, 4, s->targeted_system_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->targeted_system_display_actual_peak_luminance[i][j].den);
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        for (int i = 0; i < 3; i++)
> +            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / s->params[w].maxscl[i].den);
> +        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / s->params[w].average_maxrgb.den);
> +        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
> +        for (int i = 0; i < s->params[w].num_distribution_maxrgb_percentiles; i++) {
> +            put_bits(pb, 7, s->params[w].distribution_maxrgb[i].percentage);
> +            put_bits(pb, 17, s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
> +                s->params[w].distribution_maxrgb[i].percentile.den);
> +        }
> +        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * fraction_pixel_den /
> +            s->params[w].fraction_bright_pixels.den);
> +    }
> +
> +    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
> +    if (s->mastering_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_mastering_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_mastering_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_mastering_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_mastering_display_actual_peak_luminance; j++)
> +                put_bits(pb, 4, s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->mastering_display_actual_peak_luminance[i][j].den);
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        put_bits(pb, 1, s->params[w].tone_mapping_flag);
> +        if (s->params[w].tone_mapping_flag) {
> +            put_bits(pb, 12, s->params[w].knee_point_x.num * knee_point_den / s->params[w].knee_point_x.den);
> +            put_bits(pb, 12, s->params[w].knee_point_y.num * knee_point_den / s->params[w].knee_point_y.den);
> +            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
> +            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; i++)
> +                put_bits(pb, 10, s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
> +                    s->params[w].bezier_curve_anchors[i].den);
> +            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
> +            if (s->params[w].color_saturation_mapping_flag)
> +                put_bits(pb, 6, s->params[w].color_saturation_weight.num * saturation_weight_den /
> +                    s->params[w].color_saturation_weight.den);
> +        }
> +    }
> +
> +    flush_put_bits(pb);
> +    return buf;
> +}
> diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
> index 1f953ef1f5..797a5c64ae 100644
> --- a/libavutil/hdr_dynamic_metadata.h
> +++ b/libavutil/hdr_dynamic_metadata.h
> @@ -21,6 +21,7 @@
>  #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
>  #define AVUTIL_HDR_DYNAMIC_METADATA_H
> 
> +#include "buffer.h"
>  #include "frame.h"
>  #include "rational.h"
> 
> @@ -351,4 +352,14 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
>  int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>                                   int size);
> 
> +/**
> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
> + * excluding the country code and beginning with the terminal provider code.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + *
> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
> + */
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
> +
>  #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 900b798971..7635672985 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
> 
>  #define LIBAVUTIL_VERSION_MAJOR  58
> -#define LIBAVUTIL_VERSION_MINOR   3
> +#define LIBAVUTIL_VERSION_MINOR   4
>  #define LIBAVUTIL_VERSION_MICRO 100
> 
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> --
> 2.39.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 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] avutil: add HDR10+ dynamic metadata serialization function

[James Almer]

On 3/9/2023 11:18 AM, Raphaël Zumer wrote:
> Hi,
> 
> While I omitted adding v2/v3 here, I believe all comments on this set of patches have been addressed so far, unless anyone strongly disagrees with the rationale for moving dynamic HDR parsing and serialization to libavutil or with the function signature.
> 
> Please let me know if I missed anything.
> 
> Thanks,
> Raphaël Zumer

I'll apply this (and patch 1/1) in a few days if nobody comments.
The code needs to be in lavu if we want muxers and demuxers to use this 
functionality, so moving the existing lavc functions is fine unless we 
add more avpriv_ functions that people tend to dislike.
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

Hi,

While I omitted adding v2/v3 here, I believe all comments on this set of patches have been addressed so far, unless anyone strongly disagrees with the rationale for moving dynamic HDR parsing and serialization to libavutil or with the function signature.

Please let me know if I missed anything.

Thanks,
Raphaël Zumer

On 3/2/23 16:43, Raphaël Zumer wrote:
> Fixed brace style and moved inline buffer size calculation comments to a single block at the top.
>
>
> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
> ---
>  libavutil/hdr_dynamic_metadata.c | 142 +++++++++++++++++++++++++++++++
>  libavutil/hdr_dynamic_metadata.h |  11 +++
>  libavutil/version.h              |   2 +-
>  3 files changed, 154 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
> index 98f399b032..24dd3dab2d 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -225,3 +225,145 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>  
>      return 0;
>  }
> +
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
> +{
> +    AVBufferRef *buf;
> +    size_t size_bits, size_bytes;
> +    PutBitContext pbc, *pb = &pbc;
> +
> +    if (!s)
> +        return NULL;
> +
> +    /**
> +     * Buffer size per CTA-861-H p.253-254:
> +     * 48 bits for the header (56 minus excluded 8-bit country code)
> +     * 2 bits for num_windows
> +     * 937 bits for window geometry, for each window above 1
> +     * 27 bits for targeted_system_display_maximum_luminance
> +     * 1-3855 bits for targeted system display peak luminance information
> +     * 0-442 bits for intra-window pixel distribution information
> +     * 1-3855 bits for mastering display peak luminance information
> +     * 0-537 bits for per-window tonemapping information
> +     * 0-21 bits for per-window color saturation mapping information
> +     */
> +    size_bits = 48 +
> +        2 +
> +        FFMAX((s->num_windows - 1), 0) * 937 +
> +        27 +
> +        1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
> +            s->num_rows_targeted_system_display_actual_peak_luminance *
> +            s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
> +        s->num_windows * 82;
> +
> +    for (int w = 0; w < s->num_windows; w++)
> +        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
> +
> +    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
> +        s->num_rows_mastering_display_actual_peak_luminance *
> +        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
> +        s->num_windows * 1;
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].tone_mapping_flag)
> +            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
> +    }
> +
> +    size_bits += s->num_windows * 1;
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].color_saturation_mapping_flag)
> +            size_bits += 6;
> +    }
> +
> +    size_bytes = (size_bits + 7) / 8;
> +
> +    buf = av_buffer_alloc(size_bytes);
> +    if (!buf)
> +        return NULL;
> +
> +    init_put_bits(pb, buf->data, size_bytes);
> +
> +    // itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
> +    // itu_t_t35_terminal_provider_code shall be 0x003C
> +    put_bits(pb, 16, 0x003C);
> +    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
> +    put_bits(pb, 16, 0x0001);
> +    // application_identifier shall be set to 4
> +    put_bits(pb, 8, 4);
> +    // application_mode is set to Application Version 1
> +    put_bits(pb, 8, 1);
> +
> +    // Payload as per CTA-861-H p.253-254
> +    put_bits(pb, 2, s->num_windows);
> +
> +    for (int w = 1; w < s->num_windows; w++) {
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / s->params[w].window_upper_left_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / s->params[w].window_upper_left_corner_y.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / s->params[w].window_lower_right_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / s->params[w].window_lower_right_corner_y.den);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
> +        put_bits(pb, 8, s->params[w].rotation_angle);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
> +        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
> +        put_bits(pb, 1, s->params[w].overlap_process_option);
> +    }
> +
> +    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * luminance_den /
> +        s->targeted_system_display_maximum_luminance.den);
> +    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
> +    if (s->targeted_system_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_targeted_system_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_targeted_system_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_targeted_system_display_actual_peak_luminance; j++)
> +                put_bits(pb, 4, s->targeted_system_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->targeted_system_display_actual_peak_luminance[i][j].den);
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        for (int i = 0; i < 3; i++)
> +            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / s->params[w].maxscl[i].den);
> +        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / s->params[w].average_maxrgb.den);
> +        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
> +        for (int i = 0; i < s->params[w].num_distribution_maxrgb_percentiles; i++) {
> +            put_bits(pb, 7, s->params[w].distribution_maxrgb[i].percentage);
> +            put_bits(pb, 17, s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
> +                s->params[w].distribution_maxrgb[i].percentile.den);
> +        }
> +        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * fraction_pixel_den /
> +            s->params[w].fraction_bright_pixels.den);
> +    }
> +
> +    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
> +    if (s->mastering_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_mastering_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_mastering_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_mastering_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_mastering_display_actual_peak_luminance; j++)
> +                put_bits(pb, 4, s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->mastering_display_actual_peak_luminance[i][j].den);
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        put_bits(pb, 1, s->params[w].tone_mapping_flag);
> +        if (s->params[w].tone_mapping_flag) {
> +            put_bits(pb, 12, s->params[w].knee_point_x.num * knee_point_den / s->params[w].knee_point_x.den);
> +            put_bits(pb, 12, s->params[w].knee_point_y.num * knee_point_den / s->params[w].knee_point_y.den);
> +            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
> +            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; i++)
> +                put_bits(pb, 10, s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
> +                    s->params[w].bezier_curve_anchors[i].den);
> +            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
> +            if (s->params[w].color_saturation_mapping_flag)
> +                put_bits(pb, 6, s->params[w].color_saturation_weight.num * saturation_weight_den /
> +                    s->params[w].color_saturation_weight.den);
> +        }
> +    }
> +
> +    flush_put_bits(pb);
> +    return buf;
> +}
> diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
> index 1f953ef1f5..797a5c64ae 100644
> --- a/libavutil/hdr_dynamic_metadata.h
> +++ b/libavutil/hdr_dynamic_metadata.h
> @@ -21,6 +21,7 @@
>  #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
>  #define AVUTIL_HDR_DYNAMIC_METADATA_H
>  
> +#include "buffer.h"
>  #include "frame.h"
>  #include "rational.h"
>  
> @@ -351,4 +352,14 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
>  int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>                                   int size);
>  
> +/**
> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
> + * excluding the country code and beginning with the terminal provider code.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + *
> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
> + */
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
> +
>  #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 900b798971..7635672985 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  58
> -#define LIBAVUTIL_VERSION_MINOR   3
> +#define LIBAVUTIL_VERSION_MINOR   4
>  #define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

Fixed brace style and moved inline buffer size calculation comments to a single block at the top.


Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
---
 libavutil/hdr_dynamic_metadata.c | 142 +++++++++++++++++++++++++++++++
 libavutil/hdr_dynamic_metadata.h |  11 +++
 libavutil/version.h              |   2 +-
 3 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..24dd3dab2d 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,145 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
 
     return 0;
 }
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+    AVBufferRef *buf;
+    size_t size_bits, size_bytes;
+    PutBitContext pbc, *pb = &pbc;
+
+    if (!s)
+        return NULL;
+
+    /**
+     * Buffer size per CTA-861-H p.253-254:
+     * 48 bits for the header (56 minus excluded 8-bit country code)
+     * 2 bits for num_windows
+     * 937 bits for window geometry, for each window above 1
+     * 27 bits for targeted_system_display_maximum_luminance
+     * 1-3855 bits for targeted system display peak luminance information
+     * 0-442 bits for intra-window pixel distribution information
+     * 1-3855 bits for mastering display peak luminance information
+     * 0-537 bits for per-window tonemapping information
+     * 0-21 bits for per-window color saturation mapping information
+     */
+    size_bits = 48 +
+        2 +
+        FFMAX((s->num_windows - 1), 0) * 937 +
+        27 +
+        1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+            s->num_rows_targeted_system_display_actual_peak_luminance *
+            s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
+        s->num_windows * 82;
+
+    for (int w = 0; w < s->num_windows; w++)
+        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+
+    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+        s->num_rows_mastering_display_actual_peak_luminance *
+        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+        s->num_windows * 1;
+
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].tone_mapping_flag)
+            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+    }
+
+    size_bits += s->num_windows * 1;
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].color_saturation_mapping_flag)
+            size_bits += 6;
+    }
+
+    size_bytes = (size_bits + 7) / 8;
+
+    buf = av_buffer_alloc(size_bytes);
+    if (!buf)
+        return NULL;
+
+    init_put_bits(pb, buf->data, size_bytes);
+
+    // itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
+    // itu_t_t35_terminal_provider_code shall be 0x003C
+    put_bits(pb, 16, 0x003C);
+    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+    put_bits(pb, 16, 0x0001);
+    // application_identifier shall be set to 4
+    put_bits(pb, 8, 4);
+    // application_mode is set to Application Version 1
+    put_bits(pb, 8, 1);
+
+    // Payload as per CTA-861-H p.253-254
+    put_bits(pb, 2, s->num_windows);
+
+    for (int w = 1; w < s->num_windows; w++) {
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / s->params[w].window_upper_left_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / s->params[w].window_upper_left_corner_y.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / s->params[w].window_lower_right_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / s->params[w].window_lower_right_corner_y.den);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+        put_bits(pb, 8, s->params[w].rotation_angle);
+        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+        put_bits(pb, 1, s->params[w].overlap_process_option);
+    }
+
+    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * luminance_den /
+        s->targeted_system_display_maximum_luminance.den);
+    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, s->num_rows_targeted_system_display_actual_peak_luminance);
+        put_bits(pb, 5, s->num_cols_targeted_system_display_actual_peak_luminance);
+        for (int i = 0; i < s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < s->num_cols_targeted_system_display_actual_peak_luminance; j++)
+                put_bits(pb, 4, s->targeted_system_display_actual_peak_luminance[i][j].num * peak_luminance_den /
+                    s->targeted_system_display_actual_peak_luminance[i][j].den);
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        for (int i = 0; i < 3; i++)
+            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / s->params[w].maxscl[i].den);
+        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / s->params[w].average_maxrgb.den);
+        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
+        for (int i = 0; i < s->params[w].num_distribution_maxrgb_percentiles; i++) {
+            put_bits(pb, 7, s->params[w].distribution_maxrgb[i].percentage);
+            put_bits(pb, 17, s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
+                s->params[w].distribution_maxrgb[i].percentile.den);
+        }
+        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * fraction_pixel_den /
+            s->params[w].fraction_bright_pixels.den);
+    }
+
+    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, s->num_rows_mastering_display_actual_peak_luminance);
+        put_bits(pb, 5, s->num_cols_mastering_display_actual_peak_luminance);
+        for (int i = 0; i < s->num_rows_mastering_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < s->num_cols_mastering_display_actual_peak_luminance; j++)
+                put_bits(pb, 4, s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
+                    s->mastering_display_actual_peak_luminance[i][j].den);
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        put_bits(pb, 1, s->params[w].tone_mapping_flag);
+        if (s->params[w].tone_mapping_flag) {
+            put_bits(pb, 12, s->params[w].knee_point_x.num * knee_point_den / s->params[w].knee_point_x.den);
+            put_bits(pb, 12, s->params[w].knee_point_y.num * knee_point_den / s->params[w].knee_point_y.den);
+            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
+            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; i++)
+                put_bits(pb, 10, s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
+                    s->params[w].bezier_curve_anchors[i].den);
+            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
+            if (s->params[w].color_saturation_mapping_flag)
+                put_bits(pb, 6, s->params[w].color_saturation_weight.num * saturation_weight_den /
+                    s->params[w].color_saturation_weight.den);
+        }
+    }
+
+    flush_put_bits(pb);
+    return buf;
+}
diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
index 1f953ef1f5..797a5c64ae 100644
--- a/libavutil/hdr_dynamic_metadata.h
+++ b/libavutil/hdr_dynamic_metadata.h
@@ -21,6 +21,7 @@
 #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
 #define AVUTIL_HDR_DYNAMIC_METADATA_H
 
+#include "buffer.h"
 #include "frame.h"
 #include "rational.h"
 
@@ -351,4 +352,14 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
 int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
                                  int size);
 
+/**
+ * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
+ * excluding the country code and beginning with the terminal provider code.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ *
+ * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
+ *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
+ */
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
+
 #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 900b798971..7635672985 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR   3
+#define LIBAVUTIL_VERSION_MINOR   4
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.39.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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

On 3/2/23 15:24, Leo Izen wrote:
> On 3/2/23 14:25, Raphaël Zumer wrote:
>> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
>> ---
>>   libavutil/hdr_dynamic_metadata.c | 146 +++++++++++++++++++++++++++++++
>>   libavutil/hdr_dynamic_metadata.h |  11 +++
>>   libavutil/version.h              |   2 +-
>>   3 files changed, 158 insertions(+), 1 deletion(-)
>>
> Why not put this in avcodec/dynamic_hdr10_plus.c? You reference 
> put_bits.h which is in avcodec, so that can possibly be an issue, even 
> though it is inlined (i.e. it sends the wrong message since avutil is 
> supposed to not depend on avcodec).

I agree it is somewhat awkward to introduce a circular dependency (albeit to header-only files). On the other hand, I think those functions make more sense in libavutil than libavcodec, and it improves readability by not splitting files that are logically connected between libraries. If there is a general consensus that it is better to keep them in libavcodec, I don't mind reverting that change, or moving get_bits and put_bits to libavutil if that is doable.

>> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
>> index 98f399b032..39a7886a2e 100644
>> --- a/libavutil/hdr_dynamic_metadata.c
>> +++ b/libavutil/hdr_dynamic_metadata.c
>> @@ -225,3 +225,149 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>>   
>>       return 0;
>>   }
>> +
>> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
>> +{
> av_dynamic_hdr_plus_from_t35 returns an int status code and takes a 
> pointer as an argument, is there any particular reason you didn't mirror 
> user interface here?

Mainly the added complexity of buffer size calculation. I think it would be doable by adding an additional function such as av_dynamic_hdr_plus_to_t35_size() that would return the serialized buffer size, which could be then used by the user to allocate a buffer to be written by av_dynamic_hdr_plus_to_t35(). But adding an additional function just to make the function signatures consistent feels contrived to me, and there aren't several errors that could happen in that function that would need to be disambiguated by the user.

>> +    AVBufferRef *buf;
>> +    size_t size_bits, size_bytes;
>> +    PutBitContext pbc, *pb = &pbc;
>> +
>> +    if (!s)
>> +        return NULL;
>> +
>> +    // Buffer size per CTA-861-H p.253-254:
>> +    size_bits =
>> +    // 56 bits for the header, minus 8-bit excluded country code
>> +    48 +
>> +    // 2 bits for num_windows
>> +    2 +
>> +    // 937 bits for window geometry for each window above 1
>> +    FFMAX((s->num_windows - 1), 0) * 937 +
>> +    // 27 bits for targeted_system_display_maximum_luminance
>> +    27 +
>> +    // 1-3855 bits for targeted system display peak luminance information
>> +    1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
>> +        s->num_rows_targeted_system_display_actual_peak_luminance *
>> +        s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
>> +    // 0-442 bits for intra-window pixel distribution information
>> +    s->num_windows * 82;
> This sequence above is difficult to read due to the inline // comments. 
> It should be more readable to just have the entire expression be 
> contiguous with a /* */ multiline block comment above it explaining each 
> item.
>> +    for (int w = 0; w < s->num_windows; w++) {
>> +        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
>> +    }
> Likewise, another code style issue, don't use {} to enclose a single 
> line unless it's unavoidable. This occurs in several places in this patch.

OK, will correct.


Thanks
Raphaël Zumer

_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Derek Buitenhuis]

On 3/2/2023 8:24 PM, Leo Izen wrote:
> Why not put this in avcodec/dynamic_hdr10_plus.c? You reference 
> put_bits.h which is in avcodec, so that can possibly be an issue, even 
> though it is inlined (i.e. it sends the wrong message since avutil is 
> supposed to not depend on avcodec).

put_bits is a header-only implementation, isn't it? No dependency is
introduced.

Putting it in avutil was my suggested, as it is a function which takes
side data as input, FWIW.

- Derek
_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Leo Izen]

On 3/2/23 14:25, Raphaël Zumer wrote:
> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
> ---
>   libavutil/hdr_dynamic_metadata.c | 146 +++++++++++++++++++++++++++++++
>   libavutil/hdr_dynamic_metadata.h |  11 +++
>   libavutil/version.h              |   2 +-
>   3 files changed, 158 insertions(+), 1 deletion(-)
> 

Why not put this in avcodec/dynamic_hdr10_plus.c? You reference 
put_bits.h which is in avcodec, so that can possibly be an issue, even 
though it is inlined (i.e. it sends the wrong message since avutil is 
supposed to not depend on avcodec).

> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
> index 98f399b032..39a7886a2e 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -225,3 +225,149 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>   
>       return 0;
>   }
> +
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
> +{

av_dynamic_hdr_plus_from_t35 returns an int status code and takes a 
pointer as an argument, is there any particular reason you didn't mirror 
user interface here?


> +    AVBufferRef *buf;
> +    size_t size_bits, size_bytes;
> +    PutBitContext pbc, *pb = &pbc;
> +
> +    if (!s)
> +        return NULL;
> +
> +    // Buffer size per CTA-861-H p.253-254:
> +    size_bits =
> +    // 56 bits for the header, minus 8-bit excluded country code
> +    48 +
> +    // 2 bits for num_windows
> +    2 +
> +    // 937 bits for window geometry for each window above 1
> +    FFMAX((s->num_windows - 1), 0) * 937 +
> +    // 27 bits for targeted_system_display_maximum_luminance
> +    27 +
> +    // 1-3855 bits for targeted system display peak luminance information
> +    1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
> +        s->num_rows_targeted_system_display_actual_peak_luminance *
> +        s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
> +    // 0-442 bits for intra-window pixel distribution information
> +    s->num_windows * 82;

This sequence above is difficult to read due to the inline // comments. 
It should be more readable to just have the entire expression be 
contiguous with a /* */ multiline block comment above it explaining each 
item.

> +    for (int w = 0; w < s->num_windows; w++) {
> +        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
> +    }

Likewise, another code style issue, don't use {} to enclose a single 
line unless it's unavoidable. This occurs in several places in this patch.

> +    // 1-3855 bits for mastering display peak luminance information
> +    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
> +        s->num_rows_mastering_display_actual_peak_luminance *
> +        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
> +    // 0-537 bits for per-window tonemapping information
> +    s->num_windows * 1;
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].tone_mapping_flag) {
> +            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
> +        }
> +    }
> +    // 0-21 bits for per-window color saturation mapping information
> +    size_bits += s->num_windows * 1;
> +    for (int w = 0; w < s->num_windows; w++) {
> +        if (s->params[w].color_saturation_mapping_flag) {
> +            size_bits += 6;
> +        }
> +    }
> +
> +    size_bytes = (size_bits + 7) / 8;
> +
> +    buf = av_buffer_alloc(size_bytes);
> +    if (!buf) {
> +        return NULL;
> +    

If you update this to match the status code, this becomes AVERROR(ENOMEM);


> +
> +    init_put_bits(pb, buf->data, size_bytes);
> +
> +    // itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
> +    // itu_t_t35_terminal_provider_code shall be 0x003C
> +    put_bits(pb, 16, 0x003C);
> +    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
> +    put_bits(pb, 16, 0x0001);
> +    // application_identifier shall be set to 4
> +    put_bits(pb, 8, 4);
> +    // application_mode is set to Application Version 1
> +    put_bits(pb, 8, 1);
> +
> +    // Payload as per CTA-861-H p.253-254
> +    put_bits(pb, 2, s->num_windows);
> +
> +    for (int w = 1; w < s->num_windows; w++) {
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / s->params[w].window_upper_left_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / s->params[w].window_upper_left_corner_y.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / s->params[w].window_lower_right_corner_x.den);
> +        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / s->params[w].window_lower_right_corner_y.den);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
> +        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
> +        put_bits(pb, 8, s->params[w].rotation_angle);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
> +        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
> +        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
> +        put_bits(pb, 1, s->params[w].overlap_process_option);
> +    }
> +
> +    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * luminance_den /
> +        s->targeted_system_display_maximum_luminance.den);
> +    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
> +    if (s->targeted_system_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_targeted_system_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_targeted_system_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_targeted_system_display_actual_peak_luminance; j++) {
> +                put_bits(pb, 4, s->targeted_system_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->targeted_system_display_actual_peak_luminance[i][j].den);
> +            }
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        for (int i = 0; i < 3; i++) {
> +            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / s->params[w].maxscl[i].den);
> +        }
> +        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / s->params[w].average_maxrgb.den);
> +        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
> +        for (int i = 0; i < s->params[w].num_distribution_maxrgb_percentiles; i++) {
> +            put_bits(pb, 7, s->params[w].distribution_maxrgb[i].percentage);
> +            put_bits(pb, 17, s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
> +                s->params[w].distribution_maxrgb[i].percentile.den);
> +        }
> +        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * fraction_pixel_den /
> +            s->params[w].fraction_bright_pixels.den);
> +    }
> +
> +    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
> +    if (s->mastering_display_actual_peak_luminance_flag) {
> +        put_bits(pb, 5, s->num_rows_mastering_display_actual_peak_luminance);
> +        put_bits(pb, 5, s->num_cols_mastering_display_actual_peak_luminance);
> +        for (int i = 0; i < s->num_rows_mastering_display_actual_peak_luminance; i++) {
> +            for (int j = 0; j < s->num_cols_mastering_display_actual_peak_luminance; j++) {
> +                put_bits(pb, 4, s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
> +                    s->mastering_display_actual_peak_luminance[i][j].den);
> +            }
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        put_bits(pb, 1, s->params[w].tone_mapping_flag);
> +        if (s->params[w].tone_mapping_flag) {
> +            put_bits(pb, 12, s->params[w].knee_point_x.num * knee_point_den / s->params[w].knee_point_x.den);
> +            put_bits(pb, 12, s->params[w].knee_point_y.num * knee_point_den / s->params[w].knee_point_y.den);
> +            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
> +            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; i++) {
> +                put_bits(pb, 10, s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
> +                    s->params[w].bezier_curve_anchors[i].den);
> +            }
> +            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
> +            if (s->params[w].color_saturation_mapping_flag) {
> +                put_bits(pb, 6, s->params[w].color_saturation_weight.num * saturation_weight_den /
> +                    s->params[w].color_saturation_weight.den);
> +            }
> +        }
> +    }
> +
> +    flush_put_bits(pb);
> +    return buf;
> +}
> diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
> index 1f953ef1f5..797a5c64ae 100644
> --- a/libavutil/hdr_dynamic_metadata.h
> +++ b/libavutil/hdr_dynamic_metadata.h
> @@ -21,6 +21,7 @@
>   #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
>   #define AVUTIL_HDR_DYNAMIC_METADATA_H
>   
> +#include "buffer.h"
>   #include "frame.h"
>   #include "rational.h"
>   
> @@ -351,4 +352,14 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
>   int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>                                    int size);
>   
> +/**
> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
> + * excluding the country code and beginning with the terminal provider code.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + *
> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
> + *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
> + */
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
> +
>   #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 900b798971..7635672985 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>    */
>   
>   #define LIBAVUTIL_VERSION_MAJOR  58
> -#define LIBAVUTIL_VERSION_MINOR   3
> +#define LIBAVUTIL_VERSION_MINOR   4
>   #define LIBAVUTIL_VERSION_MICRO 100
>   
>   #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \

- Leo Izen (thebombzen)


_______________________________________________
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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
---
 libavutil/hdr_dynamic_metadata.c | 146 +++++++++++++++++++++++++++++++
 libavutil/hdr_dynamic_metadata.h |  11 +++
 libavutil/version.h              |   2 +-
 3 files changed, 158 insertions(+), 1 deletion(-)

diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..39a7886a2e 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,149 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
 
     return 0;
 }
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+    AVBufferRef *buf;
+    size_t size_bits, size_bytes;
+    PutBitContext pbc, *pb = &pbc;
+
+    if (!s)
+        return NULL;
+
+    // Buffer size per CTA-861-H p.253-254:
+    size_bits =
+    // 56 bits for the header, minus 8-bit excluded country code
+    48 +
+    // 2 bits for num_windows
+    2 +
+    // 937 bits for window geometry for each window above 1
+    FFMAX((s->num_windows - 1), 0) * 937 +
+    // 27 bits for targeted_system_display_maximum_luminance
+    27 +
+    // 1-3855 bits for targeted system display peak luminance information
+    1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+        s->num_rows_targeted_system_display_actual_peak_luminance *
+        s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
+    // 0-442 bits for intra-window pixel distribution information
+    s->num_windows * 82;
+    for (int w = 0; w < s->num_windows; w++) {
+        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+    }
+    // 1-3855 bits for mastering display peak luminance information
+    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+        s->num_rows_mastering_display_actual_peak_luminance *
+        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+    // 0-537 bits for per-window tonemapping information
+    s->num_windows * 1;
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].tone_mapping_flag) {
+            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+        }
+    }
+    // 0-21 bits for per-window color saturation mapping information
+    size_bits += s->num_windows * 1;
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].color_saturation_mapping_flag) {
+            size_bits += 6;
+        }
+    }
+
+    size_bytes = (size_bits + 7) / 8;
+
+    buf = av_buffer_alloc(size_bytes);
+    if (!buf) {
+        return NULL;
+    }
+
+    init_put_bits(pb, buf->data, size_bytes);
+
+    // itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
+    // itu_t_t35_terminal_provider_code shall be 0x003C
+    put_bits(pb, 16, 0x003C);
+    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+    put_bits(pb, 16, 0x0001);
+    // application_identifier shall be set to 4
+    put_bits(pb, 8, 4);
+    // application_mode is set to Application Version 1
+    put_bits(pb, 8, 1);
+
+    // Payload as per CTA-861-H p.253-254
+    put_bits(pb, 2, s->num_windows);
+
+    for (int w = 1; w < s->num_windows; w++) {
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / s->params[w].window_upper_left_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / s->params[w].window_upper_left_corner_y.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / s->params[w].window_lower_right_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / s->params[w].window_lower_right_corner_y.den);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+        put_bits(pb, 8, s->params[w].rotation_angle);
+        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+        put_bits(pb, 1, s->params[w].overlap_process_option);
+    }
+
+    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * luminance_den /
+        s->targeted_system_display_maximum_luminance.den);
+    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, s->num_rows_targeted_system_display_actual_peak_luminance);
+        put_bits(pb, 5, s->num_cols_targeted_system_display_actual_peak_luminance);
+        for (int i = 0; i < s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < s->num_cols_targeted_system_display_actual_peak_luminance; j++) {
+                put_bits(pb, 4, s->targeted_system_display_actual_peak_luminance[i][j].num * peak_luminance_den /
+                    s->targeted_system_display_actual_peak_luminance[i][j].den);
+            }
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        for (int i = 0; i < 3; i++) {
+            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / s->params[w].maxscl[i].den);
+        }
+        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / s->params[w].average_maxrgb.den);
+        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
+        for (int i = 0; i < s->params[w].num_distribution_maxrgb_percentiles; i++) {
+            put_bits(pb, 7, s->params[w].distribution_maxrgb[i].percentage);
+            put_bits(pb, 17, s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
+                s->params[w].distribution_maxrgb[i].percentile.den);
+        }
+        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * fraction_pixel_den /
+            s->params[w].fraction_bright_pixels.den);
+    }
+
+    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, s->num_rows_mastering_display_actual_peak_luminance);
+        put_bits(pb, 5, s->num_cols_mastering_display_actual_peak_luminance);
+        for (int i = 0; i < s->num_rows_mastering_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < s->num_cols_mastering_display_actual_peak_luminance; j++) {
+                put_bits(pb, 4, s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
+                    s->mastering_display_actual_peak_luminance[i][j].den);
+            }
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        put_bits(pb, 1, s->params[w].tone_mapping_flag);
+        if (s->params[w].tone_mapping_flag) {
+            put_bits(pb, 12, s->params[w].knee_point_x.num * knee_point_den / s->params[w].knee_point_x.den);
+            put_bits(pb, 12, s->params[w].knee_point_y.num * knee_point_den / s->params[w].knee_point_y.den);
+            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
+            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; i++) {
+                put_bits(pb, 10, s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
+                    s->params[w].bezier_curve_anchors[i].den);
+            }
+            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
+            if (s->params[w].color_saturation_mapping_flag) {
+                put_bits(pb, 6, s->params[w].color_saturation_weight.num * saturation_weight_den /
+                    s->params[w].color_saturation_weight.den);
+            }
+        }
+    }
+
+    flush_put_bits(pb);
+    return buf;
+}
diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
index 1f953ef1f5..797a5c64ae 100644
--- a/libavutil/hdr_dynamic_metadata.h
+++ b/libavutil/hdr_dynamic_metadata.h
@@ -21,6 +21,7 @@
 #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
 #define AVUTIL_HDR_DYNAMIC_METADATA_H
 
+#include "buffer.h"
 #include "frame.h"
 #include "rational.h"
 
@@ -351,4 +352,14 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
 int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
                                  int size);
 
+/**
+ * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
+ * excluding the country code and beginning with the terminal provider code.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ *
+ * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation
+ *         of the HDR10+ metadata if succeed, or NULL if buffer allocation fails.
+ */
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
+
 #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 900b798971..7635672985 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR   3
+#define LIBAVUTIL_VERSION_MINOR   4
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.39.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] avutil: add HDR10+ dynamic metadata serialization function

[Raphaël Zumer]

See the previous patch for context.

Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
---
  libavutil/hdr_dynamic_metadata.c | 147 +++++++++++++++++++++++++++++++
  libavutil/hdr_dynamic_metadata.h |  10 +++
  libavutil/version.h              |   2 +-
  3 files changed, 158 insertions(+), 1 deletion(-)

diff --git a/libavutil/hdr_dynamic_metadata.c 
b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..72d310e633 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,150 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus 
*s, const uint8_t *data,
       return 0;
  }
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+    AVBufferRef *buf;
+    size_t size_bits, size_bytes;
+    PutBitContext pbc, *pb = &pbc;
+
+    if (!s)
+        return NULL;
+
+    // Buffer size per CTA-861-H p.253-254:
+    size_bits =
+    // 56 bits for the header
+    58 +
+    // 2 bits for num_windows
+    2 +
+    // 937 bits for window geometry for each window above 1
+    FFMAX((s->num_windows - 1), 0) * 937 +
+    // 27 bits for targeted_system_display_maximum_luminance
+    27 +
+    // 1-3855 bits for targeted system display peak luminance information
+    1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+        s->num_rows_targeted_system_display_actual_peak_luminance *
+        s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 
0) +
+    // 0-442 bits for intra-window pixel distribution information
+    s->num_windows * 82;
+    for (int w = 0; w < s->num_windows; w++) {
+        size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+    }
+    // 1-3855 bits for mastering display peak luminance information
+    size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 
10 +
+        s->num_rows_mastering_display_actual_peak_luminance *
+        s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+    // 0-537 bits for per-window tonemapping information
+    s->num_windows * 1;
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].tone_mapping_flag) {
+            size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+        }
+    }
+    // 0-21 bits for per-window color saturation mapping information
+    size_bits += s->num_windows * 1;
+    for (int w = 0; w < s->num_windows; w++) {
+        if (s->params[w].color_saturation_mapping_flag) {
+            size_bits += 6;
+        }
+    }
+
+    size_bytes = (size_bits + 7) / 8;
+
+    buf = av_buffer_alloc(size_bytes);
+    if (!buf) {
+        return NULL;
+    }
+
+    init_put_bits(pb, buf->data, size_bytes);
+
+    // itu_t_t35_country_code shall be 0xB5 (USA)
+    put_bits(pb, 8, 0xB5);
+    // itu_t_t35_terminal_provider_code shall be 0x003C
+    put_bits(pb, 16, 0x003C);
+    // itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+    put_bits(pb, 16, 0x0001);
+    // application_identifier shall be set to 4
+    put_bits(pb, 8, 4);
+    // application_mode is set to Application Version 1
+    put_bits(pb, 8, 1);
+
+    // Payload as per CTA-861-H p.253-254
+    put_bits(pb, 2, s->num_windows);
+
+    for (int w = 1; w < s->num_windows; w++) {
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num / 
s->params[w].window_upper_left_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num / 
s->params[w].window_upper_left_corner_y.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num / 
s->params[w].window_lower_right_corner_x.den);
+        put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num / 
s->params[w].window_lower_right_corner_y.den);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+        put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+        put_bits(pb, 8, s->params[w].rotation_angle);
+        put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+        put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+        put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+        put_bits(pb, 1, s->params[w].overlap_process_option);
+    }
+
+    put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num * 
luminance_den /
+        s->targeted_system_display_maximum_luminance.den);
+    put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, 
s->num_rows_targeted_system_display_actual_peak_luminance);
+        put_bits(pb, 5, 
s->num_cols_targeted_system_display_actual_peak_luminance);
+        for (int i = 0; i < 
s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < 
s->num_cols_targeted_system_display_actual_peak_luminance; j++) {
+                put_bits(pb, 4, 
s->targeted_system_display_actual_peak_luminance[i][j].num * 
peak_luminance_den /
+ s->targeted_system_display_actual_peak_luminance[i][j].den);
+            }
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        for (int i = 0; i < 3; i++) {
+            put_bits(pb, 17, s->params[w].maxscl[i].num * rgb_den / 
s->params[w].maxscl[i].den);
+        }
+        put_bits(pb, 17, s->params[w].average_maxrgb.num * rgb_den / 
s->params[w].average_maxrgb.den);
+        put_bits(pb, 4, s->params[w].num_distribution_maxrgb_percentiles);
+        for (int i = 0; i < 
s->params[w].num_distribution_maxrgb_percentiles; i++) {
+            put_bits(pb, 7, 
s->params[w].distribution_maxrgb[i].percentage);
+            put_bits(pb, 17, 
s->params[w].distribution_maxrgb[i].percentile.num * rgb_den /
+                s->params[w].distribution_maxrgb[i].percentile.den);
+        }
+        put_bits(pb, 10, s->params[w].fraction_bright_pixels.num * 
fraction_pixel_den /
+            s->params[w].fraction_bright_pixels.den);
+    }
+
+    put_bits(pb, 1, s->mastering_display_actual_peak_luminance_flag);
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        put_bits(pb, 5, 
s->num_rows_mastering_display_actual_peak_luminance);
+        put_bits(pb, 5, 
s->num_cols_mastering_display_actual_peak_luminance);
+        for (int i = 0; i < 
s->num_rows_mastering_display_actual_peak_luminance; i++) {
+            for (int j = 0; j < 
s->num_cols_mastering_display_actual_peak_luminance; j++) {
+                put_bits(pb, 4, 
s->mastering_display_actual_peak_luminance[i][j].num * peak_luminance_den /
+                    s->mastering_display_actual_peak_luminance[i][j].den);
+            }
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        put_bits(pb, 1, s->params[w].tone_mapping_flag);
+        if (s->params[w].tone_mapping_flag) {
+            put_bits(pb, 12, s->params[w].knee_point_x.num * 
knee_point_den / s->params[w].knee_point_x.den);
+            put_bits(pb, 12, s->params[w].knee_point_y.num * 
knee_point_den / s->params[w].knee_point_y.den);
+            put_bits(pb, 4, s->params[w].num_bezier_curve_anchors);
+            for (int i = 0; i < s->params[w].num_bezier_curve_anchors; 
i++) {
+                put_bits(pb, 10, 
s->params[w].bezier_curve_anchors[i].num * bezier_anchor_den /
+                    s->params[w].bezier_curve_anchors[i].den);
+            }
+            put_bits(pb, 1, s->params[w].color_saturation_mapping_flag);
+            if (s->params[w].color_saturation_mapping_flag) {
+                put_bits(pb, 6, 
s->params[w].color_saturation_weight.num * saturation_weight_den /
+                    s->params[w].color_saturation_weight.den);
+            }
+        }
+    }
+
+    flush_put_bits(pb);
+    return buf;
+}
diff --git a/libavutil/hdr_dynamic_metadata.h 
b/libavutil/hdr_dynamic_metadata.h
index 1f953ef1f5..f75d3e0739 100644
--- a/libavutil/hdr_dynamic_metadata.h
+++ b/libavutil/hdr_dynamic_metadata.h
@@ -21,6 +21,7 @@
  #ifndef AVUTIL_HDR_DYNAMIC_METADATA_H
  #define AVUTIL_HDR_DYNAMIC_METADATA_H
  +#include "buffer.h"
  #include "frame.h"
  #include "rational.h"
  @@ -351,4 +352,13 @@ AVDynamicHDRPlus 
*av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
  int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
                                   int size);
  +/**
+ * Serialize dynamic HDR10+ metadata to a user data registered ITU-T 
T.35 buffer.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ *
+ * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 
representation
+ *         of the HDR10+ metadata if succeed, or NULL if buffer 
allocation fails.
+ */
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s);
+
  #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 900b798971..7635672985 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
   */
   #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR   3
+#define LIBAVUTIL_VERSION_MINOR   4
  #define LIBAVUTIL_VERSION_MICRO 100
   #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.39.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".