Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
       [not found] <20220417172658.GN3529341@pb2>
@ 2022-09-06 21:47 ` Mohammad Izadi
  2022-09-06 22:29   ` Michael Niedermayer
  2022-09-07 13:12   ` Derek Buitenhuis
  0 siblings, 2 replies; 12+ messages in thread
From: Mohammad Izadi @ 2022-09-06 21:47 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Mohammad Izadi

The fate test file can be found here: https://drive.google.com/file/d/1jGW3f94rglLfr5WGmMQe3SEnp1YkbMRy/view?usp=drivesdk
The video file needs to be copied to fate-suite/mkv/
---
 libavcodec/dynamic_hdr10_plus.c             | 269 +++++++++++++++++---
 libavcodec/dynamic_hdr10_plus.h             |  35 ++-
 libavformat/matroska.h                      |   5 +
 libavformat/matroskadec.c                   |  30 ++-
 libavformat/matroskaenc.c                   |  45 +++-
 tests/fate/matroska.mak                     |   6 +
 tests/ref/fate/matroska-hdr10-plus-metadata |  74 ++++++
 7 files changed, 407 insertions(+), 57 deletions(-)
 create mode 100644 tests/ref/fate/matroska-hdr10-plus-metadata

diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
index 34a44aac65..ce15569196 100644
--- a/libavcodec/dynamic_hdr10_plus.c
+++ b/libavcodec/dynamic_hdr10_plus.c
@@ -18,6 +18,12 @@
 
 #include "dynamic_hdr10_plus.h"
 #include "get_bits.h"
+#include "put_bits.h"
+
+static const uint8_t usa_country_code = 0xB5;
+static const uint16_t smpte_provider_code = 0x003C;
+static const uint16_t smpte2094_40_provider_oriented_code = 0x0001;
+static const uint16_t smpte2094_40_application_identifier = 0x04;
 
 static const int64_t luminance_den = 1;
 static const int32_t peak_luminance_den = 15;
@@ -27,8 +33,8 @@ static const int32_t knee_point_den = 4095;
 static const int32_t bezier_anchor_den = 1023;
 static const int32_t saturation_weight_den = 8;
 
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size)
+int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size)
 {
     GetBitContext gbc, *gb = &gbc;
     int ret;
@@ -40,10 +46,12 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     if (ret < 0)
         return ret;
 
-    if (get_bits_left(gb) < 10)
+    if (get_bits_left(gb) < 8)
         return AVERROR_INVALIDDATA;
+     s->application_version = get_bits(gb, 8);
 
-    s->application_version = get_bits(gb, 8);
+    if (get_bits_left(gb) < 2)
+        return AVERROR_INVALIDDATA;
     s->num_windows = get_bits(gb, 2);
 
     if (s->num_windows < 1 || s->num_windows > 3) {
@@ -56,15 +64,11 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     for (int w = 1; w < s->num_windows; w++) {
         // The corners are set to absolute coordinates here. They should be
         // converted to the relative coordinates (in [0, 1]) in the decoder.
-        AVHDRPlusColorTransformParams *params = &s->params[w];
-        params->window_upper_left_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_upper_left_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
+        AVHDRPlusColorTransformParams* params = &s->params[w];
+        params->window_upper_left_corner_x = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_upper_left_corner_y = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_lower_right_corner_x = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_lower_right_corner_y = (AVRational) { get_bits(gb, 16), 1 };
 
         params->center_of_ellipse_x = get_bits(gb, 16);
         params->center_of_ellipse_y = get_bits(gb, 16);
@@ -78,8 +82,7 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     if (get_bits_left(gb) < 28)
         return AVERROR_INVALIDDATA;
 
-    s->targeted_system_display_maximum_luminance =
-        (AVRational){get_bits_long(gb, 27), luminance_den};
+    s->targeted_system_display_maximum_luminance = (AVRational) { get_bits_long(gb, 27), luminance_den };
     s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
 
     if (s->targeted_system_display_actual_peak_luminance_flag) {
@@ -99,22 +102,19 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < rows; i++) {
             for (int j = 0; j < cols; j++) {
-                s->targeted_system_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+                s->targeted_system_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
             }
         }
     }
     for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
+        AVHDRPlusColorTransformParams* params = &s->params[w];
         if (get_bits_left(gb) < (3 * 17 + 17 + 4))
             return AVERROR_INVALIDDATA;
 
         for (int i = 0; i < 3; i++) {
-            params->maxscl[i] =
-                (AVRational){get_bits(gb, 17), rgb_den};
+            params->maxscl[i] = (AVRational) { get_bits(gb, 17), rgb_den };
         }
-        params->average_maxrgb =
-            (AVRational){get_bits(gb, 17), rgb_den};
+        params->average_maxrgb = (AVRational) { get_bits(gb, 17), rgb_den };
         params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
 
         if (get_bits_left(gb) <
@@ -123,14 +123,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
             params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
-            params->distribution_maxrgb[i].percentile =
-                (AVRational){get_bits(gb, 17), rgb_den};
+            params->distribution_maxrgb[i].percentile = (AVRational) { get_bits(gb, 17), rgb_den };
         }
 
         if (get_bits_left(gb) < 10)
             return AVERROR_INVALIDDATA;
 
-        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
+        params->fraction_bright_pixels = (AVRational) { get_bits(gb, 10), fraction_pixel_den };
     }
     if (get_bits_left(gb) < 1)
         return AVERROR_INVALIDDATA;
@@ -152,14 +151,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < rows; i++) {
             for (int j = 0; j < cols; j++) {
-                s->mastering_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+                s->mastering_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
             }
         }
     }
 
     for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
+        AVHDRPlusColorTransformParams* params = &s->params[w];
         if (get_bits_left(gb) < 1)
             return AVERROR_INVALIDDATA;
 
@@ -168,18 +166,15 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
             if (get_bits_left(gb) < 28)
                 return AVERROR_INVALIDDATA;
 
-            params->knee_point_x =
-                (AVRational){get_bits(gb, 12), knee_point_den};
-            params->knee_point_y =
-                (AVRational){get_bits(gb, 12), knee_point_den};
+            params->knee_point_x = (AVRational) { get_bits(gb, 12), knee_point_den };
+            params->knee_point_y = (AVRational) { get_bits(gb, 12), knee_point_den };
             params->num_bezier_curve_anchors = get_bits(gb, 4);
 
             if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
                 return AVERROR_INVALIDDATA;
 
             for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
-                params->bezier_curve_anchors[i] =
-                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
+                params->bezier_curve_anchors[i] = (AVRational) { get_bits(gb, 10), bezier_anchor_den };
             }
         }
 
@@ -196,3 +191,209 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
     return 0;
 }
+
+int ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size)
+{
+    uint8_t country_code;
+    uint16_t provider_code;
+    uint16_t provider_oriented_code;
+    uint8_t application_identifier;
+    GetBitContext gbc, *gb = &gbc;
+    int ret, offset;
+
+    if (!s)
+        return AVERROR(ENOMEM);
+
+    if (size < 7)
+        return AVERROR_INVALIDDATA;
+
+    ret = init_get_bits8(gb, data, size);
+    if (ret < 0)
+        return ret;
+
+    country_code = get_bits(gb, 8);
+    provider_code = get_bits(gb, 16);
+
+    if (country_code != usa_country_code ||
+        provider_code != smpte_provider_code)
+        return AVERROR_INVALIDDATA;
+
+    // A/341 Amendment – 2094-40
+    provider_oriented_code = get_bits(gb, 16);
+    application_identifier = get_bits(gb, 8);
+    if (provider_oriented_code != smpte2094_40_provider_oriented_code ||
+        application_identifier != smpte2094_40_application_identifier)
+        return AVERROR_INVALIDDATA;
+
+    offset = get_bits_count(gb) / 8;
+
+    return ff_parse_itu_t_t35_to_dynamic_hdr10_plus(s, gb->buffer + offset, size - offset);
+}
+
+int ff_itu_t_t35_buffer_size(const AVDynamicHDRPlus* s)
+{
+    int bit_count = 0;
+    int w, size;
+
+    if (!s)
+        return 0;
+
+    // 7 bytes for country code, provider code, and user identifier.
+    bit_count += 56;
+
+    if (s->num_windows < 1 || s->num_windows > 3)
+        return 0;
+    // Count bits for window params.
+    bit_count += 2 + ((19 * 8 + 1) * (s->num_windows - 1));
+
+    bit_count += 28;
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
+        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
+            return 0;
+
+        bit_count += (10 + rows * cols * 4);
+    }
+    for (w = 0; w < s->num_windows; w++) {
+        bit_count += (3 * 17 + 17 + 4 + 10) + (s->params[w].num_distribution_maxrgb_percentiles * 24);
+    }
+    bit_count++;
+
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        rows = s->num_rows_mastering_display_actual_peak_luminance;
+        cols = s->num_cols_mastering_display_actual_peak_luminance;
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
+            return 0;
+
+        bit_count += (10 + rows * cols * 4);
+    }
+
+    for (w = 0; w < s->num_windows; w++) {
+        bit_count++;
+        if (s->params[w].tone_mapping_flag)
+            bit_count += (28 + s->params[w].num_bezier_curve_anchors * 10);
+
+        bit_count++;
+        if (s->params[w].color_saturation_mapping_flag)
+            bit_count += 6;
+    }
+    size = bit_count / 8;
+    if (bit_count % 8 != 0)
+        size++;
+    return size;
+}
+
+int ff_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size)
+{
+    int w, i, j;
+    PutBitContext pbc, *pb = &pbc;
+
+    if (!s || !size)
+        return AVERROR(EINVAL);
+
+    *size = ff_itu_t_t35_buffer_size(s);
+    if (*size <= 0)
+        return AVERROR(EINVAL);
+    *data = av_mallocz(*size);
+    init_put_bits(pb, *data, *size);
+    if (put_bits_left(pb) < *size) {
+        av_freep(data);
+        return AVERROR(EINVAL);
+    }
+    put_bits(pb, 8, usa_country_code);
+
+    put_bits(pb, 16, smpte_provider_code);
+    put_bits(pb, 16, smpte2094_40_provider_oriented_code);
+    put_bits(pb, 8, smpte2094_40_application_identifier);
+    put_bits(pb, 8, s->application_version);
+
+    put_bits(pb, 2, s->num_windows);
+
+    for (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) {
+        int rows, cols;
+        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
+        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
+        put_bits(pb, 5, rows);
+        put_bits(pb, 5, cols);
+        for (i = 0; i < rows; i++) {
+            for (j = 0; j < cols; 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 (w = 0; w < s->num_windows; w++) {
+        for (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 (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) {
+        int rows, cols;
+        rows = s->num_rows_mastering_display_actual_peak_luminance;
+        cols = s->num_cols_mastering_display_actual_peak_luminance;
+        put_bits(pb, 5, rows);
+        put_bits(pb, 5, cols);
+        for (i = 0; i < rows; i++) {
+            for (j = 0; j < cols; 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 (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 (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 0;
+}
diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
index cd7acf0432..dafe548d2d 100644
--- a/libavcodec/dynamic_hdr10_plus.h
+++ b/libavcodec/dynamic_hdr10_plus.h
@@ -29,7 +29,38 @@
  *
  * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
  */
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size);
+int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size);
+
+/**
+ * Parse the user data registered ITU-T T.35 with header to AVDynamicHDRPlus. At first check
+ * the header if the provider code is SMPTE-2094-40. Then will parse the data to AVDynamicHDRPlus.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ * @param data The byte array containing the raw ITU-T T.35 data with header.
+ * @param size Size of the data array in bytes.
+ *
+ * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
+ */
+int ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size);
+
+/**
+ * Get the size of buffer required to save the encoded bit stream of
+ * AVDynamicHDRPlus in the form of the user data registered ITU-T T.35.
+ * @param s The AVDynamicHDRPlus structure.
+ *
+ * @return The size of bit stream required for encoding. 0 if the data is invalid.
+ */
+int ff_itu_t_t35_buffer_size(const AVDynamicHDRPlus* s);
+
+/**
+ * Encode and write AVDynamicHDRPlus to the user data registered ITU-T T.3 with header (containing the provider code).
+ * @param s A pointer containing the AVDynamicHDRPlus structure.
+ * @param data The byte array containing the raw ITU-T T.35 data with header.
+ * @param size The size of the raw ITU-T T.35 data.
+ *
+ * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
+ */
+int ff_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size);
 
 #endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 45077ed33f..c3cf9eef53 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -358,6 +358,11 @@ typedef enum {
   MATROSKA_VIDEO_PROJECTION_TYPE_MESH               = 3,
 } MatroskaVideoProjectionType;
 
+typedef enum {
+  MATROSKA_BLOCK_ADD_ID_DEFAULT                     = 0,
+  MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS          = 4,
+} MatroskaBlockAddID;
+
 /*
  * Matroska Codec IDs, strings
  */
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 16a3e93611..bd5bf34b83 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -50,6 +50,7 @@
 #include "libavutil/spherical.h"
 
 #include "libavcodec/bytestream.h"
+#include "libavcodec/dynamic_hdr10_plus.h"
 #include "libavcodec/flac.h"
 #include "libavcodec/mpeg4audio.h"
 #include "libavcodec/packet_internal.h"
@@ -3636,15 +3637,28 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska,
     pkt->stream_index = st->index;
 
     if (additional_size > 0) {
-        uint8_t *side_data = av_packet_new_side_data(pkt,
-                                                     AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
-                                                     additional_size + 8);
-        if (!side_data) {
-            av_packet_unref(pkt);
-            return AVERROR(ENOMEM);
+        if (additional_id == MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS) {
+            AVDynamicHDRPlus hdr10_plus;
+            if (!ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus(&hdr10_plus, additional, additional_size)) {
+                uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, sizeof(hdr10_plus));
+                if (!side_data) {
+                    av_packet_unref(pkt);
+                    av_free(pkt);
+                    return AVERROR(ENOMEM);
+                }
+                memcpy(side_data, (uint8_t*)(&hdr10_plus), sizeof(hdr10_plus));
+                }
+	} else {
+	    uint8_t *side_data = av_packet_new_side_data(pkt,
+                                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
+                                                         additional_size + 8);
+            if (!side_data) {
+                av_packet_unref(pkt);
+                return AVERROR(ENOMEM);
+            }
+            AV_WB64(side_data, additional_id);
+            memcpy(side_data + 8, additional, additional_size);
         }
-        AV_WB64(side_data, additional_id);
-        memcpy(side_data + 8, additional, additional_size);
     }
 
     if (discard_padding) {
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index ed1ad5039d..ee16b76d76 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -57,6 +57,7 @@
 #include "libavutil/stereo3d.h"
 
 #include "libavcodec/av1.h"
+#include "libavcodec/dynamic_hdr10_plus.h"
 #include "libavcodec/xiph.h"
 #include "libavcodec/mpeg4audio.h"
 
@@ -2490,7 +2491,7 @@ static int mkv_write_header(AVFormatContext *s)
 
 #if CONFIG_MATROSKA_MUXER
 static int mkv_reformat_h2645(MatroskaMuxContext *mkv, AVIOContext *pb,
-                              const AVPacket *pkt, int *size)
+                               const AVPacket *pkt, int *size)
 {
     int ret;
     if (pb) {
@@ -2591,8 +2592,12 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
 {
     uint8_t *side_data;
     size_t side_data_size;
-    uint64_t additional_id;
+    uint64_t additional_id = 0;
+    int64_t discard_padding = 0;
     unsigned track_number = track->track_num;
+    int has_codec_specific_blockmore = 0;
+    uint8_t *hdr10_plus_itu_t_t35;
+    size_t hdr10_plus_itu_t_t35_size = 0;
     EBML_WRITER(9);
 
     mkv->cur_block.track  = track;
@@ -2627,24 +2632,38 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
             ebml_writer_add_sint(&writer, MATROSKA_ID_DISCARDPADDING, discard_padding);
         }
     }
+    side_data = av_packet_get_side_data(pkt,
+                                        AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
+                                        &side_data_size);
+    if (side_data && side_data_size > 0)
+        ff_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data, &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
 
     side_data = av_packet_get_side_data(pkt,
                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
                                         &side_data_size);
-    if (side_data && side_data_size >= 8 &&
-        // Only the Codec-specific BlockMore (id == 1) is currently supported.
-        (additional_id = AV_RB64(side_data)) == 1) {
+    has_codec_specific_blockmore = side_data && side_data_size >= 8 &&
+                                   (additional_id = AV_RB64(side_data)) == 1;
+    // The Codec-specific BlockMore (id == 1) and HDR10+ (id == 4) are supported.
+    if (has_codec_specific_blockmore || hdr10_plus_itu_t_t35_size > 0) {
         ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKADDITIONS);
-        ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
-        /* Until dbc50f8a our demuxer used a wrong default value
-         * of BlockAddID, so we write it unconditionally. */
-        ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
-        ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
-                             side_data + 8, side_data_size - 8);
-        ebml_writer_close_master(&writer);
+	if (has_codec_specific_blockmore) {
+            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
+            /* Until dbc50f8a our demuxer used a wrong default value
+             * of BlockAddID, so we write it unconditionally. */
+            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
+            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
+                                 side_data + 8, side_data_size - 8);
+            ebml_writer_close_master(&writer);
+	}
+	if (hdr10_plus_itu_t_t35_size > 0) {
+            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
+            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS);
+            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
+                                 hdr10_plus_itu_t_t35, hdr10_plus_itu_t_t35_size);
+            ebml_writer_close_master(&writer);
+	}
         ebml_writer_close_master(&writer);
     }
-
     if (!force_blockgroup && writer.nb_elements == 2) {
         /* Nothing except the BlockGroup + Block. Can use a SimpleBlock. */
         writer.elements++;    // Skip the BlockGroup.
diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index 63e81f121b..068f178d9b 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -148,6 +148,12 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call TRANSCODE, PCM_S32LE MP2, MATROSKA,      \
                                                ARESAMPLE_FILTER              \
                                                PCM_S32BE_ENCODER)            \
                                += fate-matroska-h264-remux
+
+# The input file of the following test contains HDR10+ metadata and so this
+# test tests correct encoding and decoding HDR10+ for VP9/MKV.
+FATE_MATROSKA_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER) += fate-matroska-hdr10-plus-metadata
+fate-matroska-hdr10-plus-metadata: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_frames -select_streams v -v 0 $(TARGET_SAMPLES)/mkv/hdr10_plus_vp9_sample.mkv
+
 fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "-show_entries stream=index,codec_name:stream_tags=title,language"
 
 # Tests writing BlockAdditional and BlockGroups with ReferenceBlock elements;
diff --git a/tests/ref/fate/matroska-hdr10-plus-metadata b/tests/ref/fate/matroska-hdr10-plus-metadata
new file mode 100644
index 0000000000..0f9ad331ad
--- /dev/null
+++ b/tests/ref/fate/matroska-hdr10-plus-metadata
@@ -0,0 +1,74 @@
+[FRAME]
+media_type=video
+stream_index=0
+key_frame=1
+pts=0
+pts_time=0.000000
+pkt_dts=0
+pkt_dts_time=0.000000
+best_effort_timestamp=0
+best_effort_timestamp_time=0.000000
+pkt_duration=40
+pkt_duration_time=0.040000
+duration=40
+duration_time=0.040000
+pkt_pos=561
+pkt_size=13350
+width=1280
+height=720
+pix_fmt=yuv420p10le
+sample_aspect_ratio=1:1
+pict_type=I
+coded_picture_number=0
+display_picture_number=0
+interlaced_frame=0
+top_field_first=0
+repeat_pict=0
+color_range=tv
+color_space=unknown
+color_primaries=unknown
+color_transfer=unknown
+chroma_location=unspecified
+[SIDE_DATA]
+side_data_type=HDR Dynamic Metadata SMPTE2094-40 (HDR10+)
+application version=1
+num_windows=1
+targeted_system_display_maximum_luminance=400/1
+maxscl=3340/100000
+maxscl=2870/100000
+maxscl=2720/100000
+average_maxrgb=510/100000
+num_distribution_maxrgb_percentiles=9
+distribution_maxrgb_percentage=1
+distribution_maxrgb_percentile=30/100000
+distribution_maxrgb_percentage=5
+distribution_maxrgb_percentile=2940/100000
+distribution_maxrgb_percentage=10
+distribution_maxrgb_percentile=255/100000
+distribution_maxrgb_percentage=25
+distribution_maxrgb_percentile=70/100000
+distribution_maxrgb_percentage=50
+distribution_maxrgb_percentile=1340/100000
+distribution_maxrgb_percentage=75
+distribution_maxrgb_percentile=1600/100000
+distribution_maxrgb_percentage=90
+distribution_maxrgb_percentile=1850/100000
+distribution_maxrgb_percentage=95
+distribution_maxrgb_percentile=1950/100000
+distribution_maxrgb_percentage=99
+distribution_maxrgb_percentile=2940/100000
+fraction_bright_pixels=1/1000
+knee_point_x=0/4095
+knee_point_y=0/4095
+num_bezier_curve_anchors=9
+bezier_curve_anchors=102/1023
+bezier_curve_anchors=205/1023
+bezier_curve_anchors=307/1023
+bezier_curve_anchors=410/1023
+bezier_curve_anchors=512/1023
+bezier_curve_anchors=614/1023
+bezier_curve_anchors=717/1023
+bezier_curve_anchors=819/1023
+bezier_curve_anchors=922/1023
+[/SIDE_DATA]
+[/FRAME]
-- 
2.37.2.789.g6183377224-goog

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-06 21:47 ` [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska Mohammad Izadi
@ 2022-09-06 22:29   ` Michael Niedermayer
  2022-09-07 13:12   ` Derek Buitenhuis
  1 sibling, 0 replies; 12+ messages in thread
From: Michael Niedermayer @ 2022-09-06 22:29 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1415 bytes --]

On Tue, Sep 06, 2022 at 09:47:04PM +0000, Mohammad Izadi wrote:
> The fate test file can be found here: https://drive.google.com/file/d/1jGW3f94rglLfr5WGmMQe3SEnp1YkbMRy/view?usp=drivesdk
> The video file needs to be copied to fate-suite/mkv/
> ---
>  libavcodec/dynamic_hdr10_plus.c             | 269 +++++++++++++++++---
>  libavcodec/dynamic_hdr10_plus.h             |  35 ++-
>  libavformat/matroska.h                      |   5 +
>  libavformat/matroskadec.c                   |  30 ++-
>  libavformat/matroskaenc.c                   |  45 +++-
>  tests/fate/matroska.mak                     |   6 +
>  tests/ref/fate/matroska-hdr10-plus-metadata |  74 ++++++
>  7 files changed, 407 insertions(+), 57 deletions(-)
>  create mode 100644 tests/ref/fate/matroska-hdr10-plus-metadata

LD	ffmpeg_g
libavformat/libavformat.so: undefined reference to `ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus'
libavformat/libavformat.so: undefined reference to `ff_write_dynamic_hdr10_plus_to_full_itu_t_t35'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Makefile:131: recipe for target 'ffmpeg_g' failed
make: *** [ffmpeg_g] Error 1

ff_* symbols arent shared between libs

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-06 21:47 ` [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska Mohammad Izadi
  2022-09-06 22:29   ` Michael Niedermayer
@ 2022-09-07 13:12   ` Derek Buitenhuis
  2022-09-08  0:07     ` Mohammad Izadi
                       ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Derek Buitenhuis @ 2022-09-07 13:12 UTC (permalink / raw)
  To: ffmpeg-devel

On 9/6/2022 10:47 PM, Mohammad Izadi wrote:
> +    if (side_data && side_data_size > 0)
> +        ff_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data, &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);

You can't use ff_-prefixed functions across library boundaries.

It nees to be either public (av*) or avpriv. I suspect people won't want it to
be avpriv.

Personally, I think having serialization as a public API is useful, but YMMV. Mostly
because I was just writing my own serialization to make use of the exported side data :P.

- 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".

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-07 13:12   ` Derek Buitenhuis
@ 2022-09-08  0:07     ` Mohammad Izadi
  2022-09-08  1:22     ` Mohammad Izadi
  2022-09-08 16:31     ` Michael Niedermayer
  2 siblings, 0 replies; 12+ messages in thread
From: Mohammad Izadi @ 2022-09-08  0:07 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Thank you Derek for your comment ! It’s fixed.


On Wed, Sep 7, 2022 at 6:13 AM Derek Buitenhuis <derek.buitenhuis@gmail.com>
wrote:

> On 9/6/2022 10:47 PM, Mohammad Izadi wrote:
> > +    if (side_data && side_data_size > 0)
> > +
> ff_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data,
> &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
>
> You can't use ff_-prefixed functions across library boundaries.
>
> It nees to be either public (av*) or avpriv. I suspect people won't want
> it to
> be avpriv.
>
> Personally, I think having serialization as a public API is useful, but
> YMMV. Mostly
> because I was just writing my own serialization to make use of the
> exported side data :P.
>
> - 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".
>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-07 13:12   ` Derek Buitenhuis
  2022-09-08  0:07     ` Mohammad Izadi
@ 2022-09-08  1:22     ` Mohammad Izadi
  2022-10-26  0:16       ` Andreas Rheinhardt
  2022-09-08 16:31     ` Michael Niedermayer
  2 siblings, 1 reply; 12+ messages in thread
From: Mohammad Izadi @ 2022-09-08  1:22 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Mohammad Izadi

The fate test file can be found here: https://drive.google.com/file/d/1jGW3f94rglLfr5WGmMQe3SEnp1YkbMRy/view?usp=drivesdk
The video file needs to be copied to fate-suite/mkv/
---
 libavcodec/dynamic_hdr10_plus.c             | 269 +++++++++++++++++---
 libavcodec/dynamic_hdr10_plus.h             |  26 +-
 libavformat/matroska.h                      |   5 +
 libavformat/matroskadec.c                   |  30 ++-
 libavformat/matroskaenc.c                   |  44 +++-
 tests/fate/matroska.mak                     |   6 +
 tests/ref/fate/matroska-hdr10-plus-metadata |  74 ++++++
 7 files changed, 397 insertions(+), 57 deletions(-)
 create mode 100644 tests/ref/fate/matroska-hdr10-plus-metadata

diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
index 34a44aac65..d05f211c94 100644
--- a/libavcodec/dynamic_hdr10_plus.c
+++ b/libavcodec/dynamic_hdr10_plus.c
@@ -18,6 +18,12 @@
 
 #include "dynamic_hdr10_plus.h"
 #include "get_bits.h"
+#include "put_bits.h"
+
+static const uint8_t usa_country_code = 0xB5;
+static const uint16_t smpte_provider_code = 0x003C;
+static const uint16_t smpte2094_40_provider_oriented_code = 0x0001;
+static const uint16_t smpte2094_40_application_identifier = 0x04;
 
 static const int64_t luminance_den = 1;
 static const int32_t peak_luminance_den = 15;
@@ -27,8 +33,8 @@ static const int32_t knee_point_den = 4095;
 static const int32_t bezier_anchor_den = 1023;
 static const int32_t saturation_weight_den = 8;
 
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size)
+int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size)
 {
     GetBitContext gbc, *gb = &gbc;
     int ret;
@@ -40,10 +46,12 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     if (ret < 0)
         return ret;
 
-    if (get_bits_left(gb) < 10)
+    if (get_bits_left(gb) < 8)
         return AVERROR_INVALIDDATA;
+     s->application_version = get_bits(gb, 8);
 
-    s->application_version = get_bits(gb, 8);
+    if (get_bits_left(gb) < 2)
+        return AVERROR_INVALIDDATA;
     s->num_windows = get_bits(gb, 2);
 
     if (s->num_windows < 1 || s->num_windows > 3) {
@@ -56,15 +64,11 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     for (int w = 1; w < s->num_windows; w++) {
         // The corners are set to absolute coordinates here. They should be
         // converted to the relative coordinates (in [0, 1]) in the decoder.
-        AVHDRPlusColorTransformParams *params = &s->params[w];
-        params->window_upper_left_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_upper_left_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
+        AVHDRPlusColorTransformParams* params = &s->params[w];
+        params->window_upper_left_corner_x = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_upper_left_corner_y = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_lower_right_corner_x = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_lower_right_corner_y = (AVRational) { get_bits(gb, 16), 1 };
 
         params->center_of_ellipse_x = get_bits(gb, 16);
         params->center_of_ellipse_y = get_bits(gb, 16);
@@ -78,8 +82,7 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     if (get_bits_left(gb) < 28)
         return AVERROR_INVALIDDATA;
 
-    s->targeted_system_display_maximum_luminance =
-        (AVRational){get_bits_long(gb, 27), luminance_den};
+    s->targeted_system_display_maximum_luminance = (AVRational) { get_bits_long(gb, 27), luminance_den };
     s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
 
     if (s->targeted_system_display_actual_peak_luminance_flag) {
@@ -99,22 +102,19 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < rows; i++) {
             for (int j = 0; j < cols; j++) {
-                s->targeted_system_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+                s->targeted_system_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
             }
         }
     }
     for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
+        AVHDRPlusColorTransformParams* params = &s->params[w];
         if (get_bits_left(gb) < (3 * 17 + 17 + 4))
             return AVERROR_INVALIDDATA;
 
         for (int i = 0; i < 3; i++) {
-            params->maxscl[i] =
-                (AVRational){get_bits(gb, 17), rgb_den};
+            params->maxscl[i] = (AVRational) { get_bits(gb, 17), rgb_den };
         }
-        params->average_maxrgb =
-            (AVRational){get_bits(gb, 17), rgb_den};
+        params->average_maxrgb = (AVRational) { get_bits(gb, 17), rgb_den };
         params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
 
         if (get_bits_left(gb) <
@@ -123,14 +123,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
             params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
-            params->distribution_maxrgb[i].percentile =
-                (AVRational){get_bits(gb, 17), rgb_den};
+            params->distribution_maxrgb[i].percentile = (AVRational) { get_bits(gb, 17), rgb_den };
         }
 
         if (get_bits_left(gb) < 10)
             return AVERROR_INVALIDDATA;
 
-        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
+        params->fraction_bright_pixels = (AVRational) { get_bits(gb, 10), fraction_pixel_den };
     }
     if (get_bits_left(gb) < 1)
         return AVERROR_INVALIDDATA;
@@ -152,14 +151,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < rows; i++) {
             for (int j = 0; j < cols; j++) {
-                s->mastering_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+                s->mastering_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
             }
         }
     }
 
     for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
+        AVHDRPlusColorTransformParams* params = &s->params[w];
         if (get_bits_left(gb) < 1)
             return AVERROR_INVALIDDATA;
 
@@ -168,18 +166,15 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
             if (get_bits_left(gb) < 28)
                 return AVERROR_INVALIDDATA;
 
-            params->knee_point_x =
-                (AVRational){get_bits(gb, 12), knee_point_den};
-            params->knee_point_y =
-                (AVRational){get_bits(gb, 12), knee_point_den};
+            params->knee_point_x = (AVRational) { get_bits(gb, 12), knee_point_den };
+            params->knee_point_y = (AVRational) { get_bits(gb, 12), knee_point_den };
             params->num_bezier_curve_anchors = get_bits(gb, 4);
 
             if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
                 return AVERROR_INVALIDDATA;
 
             for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
-                params->bezier_curve_anchors[i] =
-                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
+                params->bezier_curve_anchors[i] = (AVRational) { get_bits(gb, 10), bezier_anchor_den };
             }
         }
 
@@ -196,3 +191,209 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
     return 0;
 }
+
+int av_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size)
+{
+    uint8_t country_code;
+    uint16_t provider_code;
+    uint16_t provider_oriented_code;
+    uint8_t application_identifier;
+    GetBitContext gbc, *gb = &gbc;
+    int ret, offset;
+
+    if (!s)
+        return AVERROR(ENOMEM);
+
+    if (size < 7)
+        return AVERROR_INVALIDDATA;
+
+    ret = init_get_bits8(gb, data, size);
+    if (ret < 0)
+        return ret;
+
+    country_code = get_bits(gb, 8);
+    provider_code = get_bits(gb, 16);
+
+    if (country_code != usa_country_code ||
+        provider_code != smpte_provider_code)
+        return AVERROR_INVALIDDATA;
+
+    // A/341 Amendment – 2094-40
+    provider_oriented_code = get_bits(gb, 16);
+    application_identifier = get_bits(gb, 8);
+    if (provider_oriented_code != smpte2094_40_provider_oriented_code ||
+        application_identifier != smpte2094_40_application_identifier)
+        return AVERROR_INVALIDDATA;
+
+    offset = get_bits_count(gb) / 8;
+
+    return ff_parse_itu_t_t35_to_dynamic_hdr10_plus(s, gb->buffer + offset, size - offset);
+}
+
+static int ff_itu_t_t35_buffer_size(const AVDynamicHDRPlus* s)
+{
+    int bit_count = 0;
+    int w, size;
+
+    if (!s)
+        return 0;
+
+    // 7 bytes for country code, provider code, and user identifier.
+    bit_count += 56;
+
+    if (s->num_windows < 1 || s->num_windows > 3)
+        return 0;
+    // Count bits for window params.
+    bit_count += 2 + ((19 * 8 + 1) * (s->num_windows - 1));
+
+    bit_count += 28;
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
+        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
+            return 0;
+
+        bit_count += (10 + rows * cols * 4);
+    }
+    for (w = 0; w < s->num_windows; w++) {
+        bit_count += (3 * 17 + 17 + 4 + 10) + (s->params[w].num_distribution_maxrgb_percentiles * 24);
+    }
+    bit_count++;
+
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        rows = s->num_rows_mastering_display_actual_peak_luminance;
+        cols = s->num_cols_mastering_display_actual_peak_luminance;
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
+            return 0;
+
+        bit_count += (10 + rows * cols * 4);
+    }
+
+    for (w = 0; w < s->num_windows; w++) {
+        bit_count++;
+        if (s->params[w].tone_mapping_flag)
+            bit_count += (28 + s->params[w].num_bezier_curve_anchors * 10);
+
+        bit_count++;
+        if (s->params[w].color_saturation_mapping_flag)
+            bit_count += 6;
+    }
+    size = bit_count / 8;
+    if (bit_count % 8 != 0)
+        size++;
+    return size;
+}
+
+int av_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size)
+{
+    int w, i, j;
+    PutBitContext pbc, *pb = &pbc;
+
+    if (!s || !size)
+        return AVERROR(EINVAL);
+
+    *size = ff_itu_t_t35_buffer_size(s);
+    if (*size <= 0)
+        return AVERROR(EINVAL);
+    *data = av_mallocz(*size);
+    init_put_bits(pb, *data, *size);
+    if (put_bits_left(pb) < *size) {
+        av_freep(data);
+        return AVERROR(EINVAL);
+    }
+    put_bits(pb, 8, usa_country_code);
+
+    put_bits(pb, 16, smpte_provider_code);
+    put_bits(pb, 16, smpte2094_40_provider_oriented_code);
+    put_bits(pb, 8, smpte2094_40_application_identifier);
+    put_bits(pb, 8, s->application_version);
+
+    put_bits(pb, 2, s->num_windows);
+
+    for (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) {
+        int rows, cols;
+        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
+        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
+        put_bits(pb, 5, rows);
+        put_bits(pb, 5, cols);
+        for (i = 0; i < rows; i++) {
+            for (j = 0; j < cols; 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 (w = 0; w < s->num_windows; w++) {
+        for (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 (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) {
+        int rows, cols;
+        rows = s->num_rows_mastering_display_actual_peak_luminance;
+        cols = s->num_cols_mastering_display_actual_peak_luminance;
+        put_bits(pb, 5, rows);
+        put_bits(pb, 5, cols);
+        for (i = 0; i < rows; i++) {
+            for (j = 0; j < cols; 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 (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 (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 0;
+}
diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
index cd7acf0432..d52c3e552d 100644
--- a/libavcodec/dynamic_hdr10_plus.h
+++ b/libavcodec/dynamic_hdr10_plus.h
@@ -29,7 +29,29 @@
  *
  * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
  */
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size);
+int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size);
+
+/**
+ * Parse the user data registered ITU-T T.35 with header to AVDynamicHDRPlus. At first check
+ * the header if the provider code is SMPTE-2094-40. Then will parse the data to AVDynamicHDRPlus.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ * @param data The byte array containing the raw ITU-T T.35 data with header.
+ * @param size Size of the data array in bytes.
+ *
+ * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
+ */
+int av_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size);
+
+/**
+ * Encode and write AVDynamicHDRPlus to the user data registered ITU-T T.3 with header (containing the provider code).
+ * @param s A pointer containing the AVDynamicHDRPlus structure.
+ * @param data The byte array containing the raw ITU-T T.35 data with header.
+ * @param size The size of the raw ITU-T T.35 data.
+ *
+ * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
+ */
+int av_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size);
 
 #endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 45077ed33f..c3cf9eef53 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -358,6 +358,11 @@ typedef enum {
   MATROSKA_VIDEO_PROJECTION_TYPE_MESH               = 3,
 } MatroskaVideoProjectionType;
 
+typedef enum {
+  MATROSKA_BLOCK_ADD_ID_DEFAULT                     = 0,
+  MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS          = 4,
+} MatroskaBlockAddID;
+
 /*
  * Matroska Codec IDs, strings
  */
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 16a3e93611..7fa4483ba4 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -50,6 +50,7 @@
 #include "libavutil/spherical.h"
 
 #include "libavcodec/bytestream.h"
+#include "libavcodec/dynamic_hdr10_plus.h"
 #include "libavcodec/flac.h"
 #include "libavcodec/mpeg4audio.h"
 #include "libavcodec/packet_internal.h"
@@ -3636,15 +3637,28 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska,
     pkt->stream_index = st->index;
 
     if (additional_size > 0) {
-        uint8_t *side_data = av_packet_new_side_data(pkt,
-                                                     AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
-                                                     additional_size + 8);
-        if (!side_data) {
-            av_packet_unref(pkt);
-            return AVERROR(ENOMEM);
+        if (additional_id == MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS) {
+            AVDynamicHDRPlus hdr10_plus;
+            if (!av_parse_full_itu_t_t35_to_dynamic_hdr10_plus(&hdr10_plus, additional, additional_size)) {
+                uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, sizeof(hdr10_plus));
+                if (!side_data) {
+                    av_packet_unref(pkt);
+                    av_free(pkt);
+                    return AVERROR(ENOMEM);
+                }
+                memcpy(side_data, (uint8_t*)(&hdr10_plus), sizeof(hdr10_plus));
+                }
+	} else {
+	    uint8_t *side_data = av_packet_new_side_data(pkt,
+                                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
+                                                         additional_size + 8);
+            if (!side_data) {
+                av_packet_unref(pkt);
+                return AVERROR(ENOMEM);
+            }
+            AV_WB64(side_data, additional_id);
+            memcpy(side_data + 8, additional, additional_size);
         }
-        AV_WB64(side_data, additional_id);
-        memcpy(side_data + 8, additional, additional_size);
     }
 
     if (discard_padding) {
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index ed1ad5039d..665e3b7ec6 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -57,6 +57,7 @@
 #include "libavutil/stereo3d.h"
 
 #include "libavcodec/av1.h"
+#include "libavcodec/dynamic_hdr10_plus.h"
 #include "libavcodec/xiph.h"
 #include "libavcodec/mpeg4audio.h"
 
@@ -2490,7 +2491,7 @@ static int mkv_write_header(AVFormatContext *s)
 
 #if CONFIG_MATROSKA_MUXER
 static int mkv_reformat_h2645(MatroskaMuxContext *mkv, AVIOContext *pb,
-                              const AVPacket *pkt, int *size)
+                               const AVPacket *pkt, int *size)
 {
     int ret;
     if (pb) {
@@ -2591,8 +2592,11 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
 {
     uint8_t *side_data;
     size_t side_data_size;
-    uint64_t additional_id;
+    uint64_t additional_id = 0;
     unsigned track_number = track->track_num;
+    int has_codec_specific_blockmore = 0;
+    uint8_t *hdr10_plus_itu_t_t35;
+    size_t hdr10_plus_itu_t_t35_size = 0;
     EBML_WRITER(9);
 
     mkv->cur_block.track  = track;
@@ -2627,24 +2631,38 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
             ebml_writer_add_sint(&writer, MATROSKA_ID_DISCARDPADDING, discard_padding);
         }
     }
+    side_data = av_packet_get_side_data(pkt,
+                                        AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
+                                        &side_data_size);
+    if (side_data && side_data_size > 0)
+        av_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data, &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
 
     side_data = av_packet_get_side_data(pkt,
                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
                                         &side_data_size);
-    if (side_data && side_data_size >= 8 &&
-        // Only the Codec-specific BlockMore (id == 1) is currently supported.
-        (additional_id = AV_RB64(side_data)) == 1) {
+    has_codec_specific_blockmore = side_data && side_data_size >= 8 &&
+                                   (additional_id = AV_RB64(side_data)) == 1;
+    // The Codec-specific BlockMore (id == 1) and HDR10+ (id == 4) are supported.
+    if (has_codec_specific_blockmore || hdr10_plus_itu_t_t35_size > 0) {
         ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKADDITIONS);
-        ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
-        /* Until dbc50f8a our demuxer used a wrong default value
-         * of BlockAddID, so we write it unconditionally. */
-        ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
-        ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
-                             side_data + 8, side_data_size - 8);
-        ebml_writer_close_master(&writer);
+	if (has_codec_specific_blockmore) {
+            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
+            /* Until dbc50f8a our demuxer used a wrong default value
+             * of BlockAddID, so we write it unconditionally. */
+            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
+            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
+                                 side_data + 8, side_data_size - 8);
+            ebml_writer_close_master(&writer);
+	}
+	if (hdr10_plus_itu_t_t35_size > 0) {
+            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
+            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS);
+            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
+                                 hdr10_plus_itu_t_t35, hdr10_plus_itu_t_t35_size);
+            ebml_writer_close_master(&writer);
+	}
         ebml_writer_close_master(&writer);
     }
-
     if (!force_blockgroup && writer.nb_elements == 2) {
         /* Nothing except the BlockGroup + Block. Can use a SimpleBlock. */
         writer.elements++;    // Skip the BlockGroup.
diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index 63e81f121b..068f178d9b 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -148,6 +148,12 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call TRANSCODE, PCM_S32LE MP2, MATROSKA,      \
                                                ARESAMPLE_FILTER              \
                                                PCM_S32BE_ENCODER)            \
                                += fate-matroska-h264-remux
+
+# The input file of the following test contains HDR10+ metadata and so this
+# test tests correct encoding and decoding HDR10+ for VP9/MKV.
+FATE_MATROSKA_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER) += fate-matroska-hdr10-plus-metadata
+fate-matroska-hdr10-plus-metadata: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_frames -select_streams v -v 0 $(TARGET_SAMPLES)/mkv/hdr10_plus_vp9_sample.mkv
+
 fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "-show_entries stream=index,codec_name:stream_tags=title,language"
 
 # Tests writing BlockAdditional and BlockGroups with ReferenceBlock elements;
diff --git a/tests/ref/fate/matroska-hdr10-plus-metadata b/tests/ref/fate/matroska-hdr10-plus-metadata
new file mode 100644
index 0000000000..0f9ad331ad
--- /dev/null
+++ b/tests/ref/fate/matroska-hdr10-plus-metadata
@@ -0,0 +1,74 @@
+[FRAME]
+media_type=video
+stream_index=0
+key_frame=1
+pts=0
+pts_time=0.000000
+pkt_dts=0
+pkt_dts_time=0.000000
+best_effort_timestamp=0
+best_effort_timestamp_time=0.000000
+pkt_duration=40
+pkt_duration_time=0.040000
+duration=40
+duration_time=0.040000
+pkt_pos=561
+pkt_size=13350
+width=1280
+height=720
+pix_fmt=yuv420p10le
+sample_aspect_ratio=1:1
+pict_type=I
+coded_picture_number=0
+display_picture_number=0
+interlaced_frame=0
+top_field_first=0
+repeat_pict=0
+color_range=tv
+color_space=unknown
+color_primaries=unknown
+color_transfer=unknown
+chroma_location=unspecified
+[SIDE_DATA]
+side_data_type=HDR Dynamic Metadata SMPTE2094-40 (HDR10+)
+application version=1
+num_windows=1
+targeted_system_display_maximum_luminance=400/1
+maxscl=3340/100000
+maxscl=2870/100000
+maxscl=2720/100000
+average_maxrgb=510/100000
+num_distribution_maxrgb_percentiles=9
+distribution_maxrgb_percentage=1
+distribution_maxrgb_percentile=30/100000
+distribution_maxrgb_percentage=5
+distribution_maxrgb_percentile=2940/100000
+distribution_maxrgb_percentage=10
+distribution_maxrgb_percentile=255/100000
+distribution_maxrgb_percentage=25
+distribution_maxrgb_percentile=70/100000
+distribution_maxrgb_percentage=50
+distribution_maxrgb_percentile=1340/100000
+distribution_maxrgb_percentage=75
+distribution_maxrgb_percentile=1600/100000
+distribution_maxrgb_percentage=90
+distribution_maxrgb_percentile=1850/100000
+distribution_maxrgb_percentage=95
+distribution_maxrgb_percentile=1950/100000
+distribution_maxrgb_percentage=99
+distribution_maxrgb_percentile=2940/100000
+fraction_bright_pixels=1/1000
+knee_point_x=0/4095
+knee_point_y=0/4095
+num_bezier_curve_anchors=9
+bezier_curve_anchors=102/1023
+bezier_curve_anchors=205/1023
+bezier_curve_anchors=307/1023
+bezier_curve_anchors=410/1023
+bezier_curve_anchors=512/1023
+bezier_curve_anchors=614/1023
+bezier_curve_anchors=717/1023
+bezier_curve_anchors=819/1023
+bezier_curve_anchors=922/1023
+[/SIDE_DATA]
+[/FRAME]
-- 
2.37.2.789.g6183377224-goog

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-07 13:12   ` Derek Buitenhuis
  2022-09-08  0:07     ` Mohammad Izadi
  2022-09-08  1:22     ` Mohammad Izadi
@ 2022-09-08 16:31     ` Michael Niedermayer
  2022-09-08 17:03       ` Mohammad Izadi
  2 siblings, 1 reply; 12+ messages in thread
From: Michael Niedermayer @ 2022-09-08 16:31 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1390 bytes --]

On Wed, Sep 07, 2022 at 02:12:46PM +0100, Derek Buitenhuis wrote:
> On 9/6/2022 10:47 PM, Mohammad Izadi wrote:
> > +    if (side_data && side_data_size > 0)
> > +        ff_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data, &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
> 
> You can't use ff_-prefixed functions across library boundaries.
> 
> It nees to be either public (av*) or avpriv. I suspect people won't want it to
> be avpriv.
> 
> Personally, I think having serialization as a public API is useful, but YMMV. Mostly
> because I was just writing my own serialization to make use of the exported side data :P.

I agree

on a related subject, side data serialization should be moved to a common API
We have common APIs for parsers, decoder, bitstream filters but for 
parsing/decoding side data this is heading toward something less structured

Above is not a comment on this patch, the patch is fine. I just want to point
to this before we have several dozen such functions which need to be
deprecated and supported when a more structured system is introduced

thx


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-08 16:31     ` Michael Niedermayer
@ 2022-09-08 17:03       ` Mohammad Izadi
  2022-10-25 23:38         ` Mohammad Izadi
  0 siblings, 1 reply; 12+ messages in thread
From: Mohammad Izadi @ 2022-09-08 17:03 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Michael, I appreciate it if you can take a look and give me your feedback.


On Thu, Sep 8, 2022 at 9:31 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Sep 07, 2022 at 02:12:46PM +0100, Derek Buitenhuis wrote:
> > On 9/6/2022 10:47 PM, Mohammad Izadi wrote:
> > > +    if (side_data && side_data_size > 0)
> > > +
> ff_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data,
> &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
> >
> > You can't use ff_-prefixed functions across library boundaries.
> >
> > It nees to be either public (av*) or avpriv. I suspect people won't want
> it to
> > be avpriv.
> >
> > Personally, I think having serialization as a public API is useful, but
> YMMV. Mostly
> > because I was just writing my own serialization to make use of the
> exported side data :P.
>
> I agree
>
> on a related subject, side data serialization should be moved to a common
> API
> We have common APIs for parsers, decoder, bitstream filters but for
> parsing/decoding side data this is heading toward something less structured
>
> Above is not a comment on this patch, the patch is fine. I just want to
> point
> to this before we have several dozen such functions which need to be
> deprecated and supported when a more structured system is introduced
>
> thx
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Into a blind darkness they enter who follow after the Ignorance,
> they as if into a greater darkness enter who devote themselves
> to the Knowledge alone. -- Isha Upanishad
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-08 17:03       ` Mohammad Izadi
@ 2022-10-25 23:38         ` Mohammad Izadi
  2022-10-26 20:17           ` Michael Niedermayer
  0 siblings, 1 reply; 12+ messages in thread
From: Mohammad Izadi @ 2022-10-25 23:38 UTC (permalink / raw)
  To: FFmpeg development discussions and patches, Thierry Foucu

Michael, I appreciate it if you can take a look and give me your feedback.

Thanks,
Mohammad


On Thu, Sep 8, 2022 at 10:03 AM Mohammad Izadi <izadi@google.com> wrote:

> Michael, I appreciate it if you can take a look and give me your feedback.
>
>
> On Thu, Sep 8, 2022 at 9:31 AM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
>
>> On Wed, Sep 07, 2022 at 02:12:46PM +0100, Derek Buitenhuis wrote:
>> > On 9/6/2022 10:47 PM, Mohammad Izadi wrote:
>> > > +    if (side_data && side_data_size > 0)
>> > > +
>> ff_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data,
>> &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
>> >
>> > You can't use ff_-prefixed functions across library boundaries.
>> >
>> > It nees to be either public (av*) or avpriv. I suspect people won't
>> want it to
>> > be avpriv.
>> >
>> > Personally, I think having serialization as a public API is useful, but
>> YMMV. Mostly
>> > because I was just writing my own serialization to make use of the
>> exported side data :P.
>>
>> I agree
>>
>> on a related subject, side data serialization should be moved to a common
>> API
>> We have common APIs for parsers, decoder, bitstream filters but for
>> parsing/decoding side data this is heading toward something less
>> structured
>>
>> Above is not a comment on this patch, the patch is fine. I just want to
>> point
>> to this before we have several dozen such functions which need to be
>> deprecated and supported when a more structured system is introduced
>>
>> thx
>>
>>
>> [...]
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Into a blind darkness they enter who follow after the Ignorance,
>> they as if into a greater darkness enter who devote themselves
>> to the Knowledge alone. -- Isha Upanishad
>> _______________________________________________
>> 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".

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-09-08  1:22     ` Mohammad Izadi
@ 2022-10-26  0:16       ` Andreas Rheinhardt
  0 siblings, 0 replies; 12+ messages in thread
From: Andreas Rheinhardt @ 2022-10-26  0:16 UTC (permalink / raw)
  To: ffmpeg-devel

Mohammad Izadi:
> The fate test file can be found here: https://drive.google.com/file/d/1jGW3f94rglLfr5WGmMQe3SEnp1YkbMRy/view?usp=drivesdk
> The video file needs to be copied to fate-suite/mkv/

This does not describe the patch at all and should therefore not be part
of the commit message.

> ---

Instead put it here.

>  libavcodec/dynamic_hdr10_plus.c             | 269 +++++++++++++++++---

The changes to libavcodec and Matroska need to be in separate patches.

>  libavcodec/dynamic_hdr10_plus.h             |  26 +-

This is a private header and therefore must not be used for public symbols.

>  libavformat/matroska.h                      |   5 +
>  libavformat/matroskadec.c                   |  30 ++-
>  libavformat/matroskaenc.c                   |  44 +++-
>  tests/fate/matroska.mak                     |   6 +
>  tests/ref/fate/matroska-hdr10-plus-metadata |  74 ++++++
>  7 files changed, 397 insertions(+), 57 deletions(-)
>  create mode 100644 tests/ref/fate/matroska-hdr10-plus-metadata
> 
> diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
> index 34a44aac65..d05f211c94 100644
> --- a/libavcodec/dynamic_hdr10_plus.c
> +++ b/libavcodec/dynamic_hdr10_plus.c
> @@ -18,6 +18,12 @@
>  
>  #include "dynamic_hdr10_plus.h"
>  #include "get_bits.h"
> +#include "put_bits.h"
> +
> +static const uint8_t usa_country_code = 0xB5;
> +static const uint16_t smpte_provider_code = 0x003C;
> +static const uint16_t smpte2094_40_provider_oriented_code = 0x0001;
> +static const uint16_t smpte2094_40_application_identifier = 0x04;
>  

We typically use defines or enums for such things; C does not have
constexpr.

>  static const int64_t luminance_den = 1;
>  static const int32_t peak_luminance_den = 15;
> @@ -27,8 +33,8 @@ static const int32_t knee_point_den = 4095;
>  static const int32_t bezier_anchor_den = 1023;
>  static const int32_t saturation_weight_den = 8;
>  
> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
> -                                             int size)
> +int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
> +					     int size)
>  {
>      GetBitContext gbc, *gb = &gbc;
>      int ret;
> @@ -40,10 +46,12 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>      if (ret < 0)
>          return ret;
>  
> -    if (get_bits_left(gb) < 10)
> +    if (get_bits_left(gb) < 8)
>          return AVERROR_INVALIDDATA;
> +     s->application_version = get_bits(gb, 8);
>  
> -    s->application_version = get_bits(gb, 8);
> +    if (get_bits_left(gb) < 2)
> +        return AVERROR_INVALIDDATA;

These changes add an unnecessary branch.

>      s->num_windows = get_bits(gb, 2);
>  
>      if (s->num_windows < 1 || s->num_windows > 3) {
> @@ -56,15 +64,11 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>      for (int w = 1; w < s->num_windows; w++) {
>          // The corners are set to absolute coordinates here. They should be
>          // converted to the relative coordinates (in [0, 1]) in the decoder.

All the following changes to this functions are cosmetic and should be
removed. Mixing cosmetic and non-cosmetic changes make patches harder to
review.

> -        AVHDRPlusColorTransformParams *params = &s->params[w];
> -        params->window_upper_left_corner_x =
> -            (AVRational){get_bits(gb, 16), 1};
> -        params->window_upper_left_corner_y =
> -            (AVRational){get_bits(gb, 16), 1};
> -        params->window_lower_right_corner_x =
> -            (AVRational){get_bits(gb, 16), 1};
> -        params->window_lower_right_corner_y =
> -            (AVRational){get_bits(gb, 16), 1};
> +        AVHDRPlusColorTransformParams* params = &s->params[w];
> +        params->window_upper_left_corner_x = (AVRational) { get_bits(gb, 16), 1 };
> +        params->window_upper_left_corner_y = (AVRational) { get_bits(gb, 16), 1 };
> +        params->window_lower_right_corner_x = (AVRational) { get_bits(gb, 16), 1 };
> +        params->window_lower_right_corner_y = (AVRational) { get_bits(gb, 16), 1 };
>  
>          params->center_of_ellipse_x = get_bits(gb, 16);
>          params->center_of_ellipse_y = get_bits(gb, 16);
> @@ -78,8 +82,7 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>      if (get_bits_left(gb) < 28)
>          return AVERROR_INVALIDDATA;
>  
> -    s->targeted_system_display_maximum_luminance =
> -        (AVRational){get_bits_long(gb, 27), luminance_den};
> +    s->targeted_system_display_maximum_luminance = (AVRational) { get_bits_long(gb, 27), luminance_den };
>      s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
>  
>      if (s->targeted_system_display_actual_peak_luminance_flag) {
> @@ -99,22 +102,19 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>  
>          for (int i = 0; i < rows; i++) {
>              for (int j = 0; j < cols; j++) {
> -                s->targeted_system_display_actual_peak_luminance[i][j] =
> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
> +                s->targeted_system_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
>              }
>          }
>      }
>      for (int w = 0; w < s->num_windows; w++) {
> -        AVHDRPlusColorTransformParams *params = &s->params[w];
> +        AVHDRPlusColorTransformParams* params = &s->params[w];
>          if (get_bits_left(gb) < (3 * 17 + 17 + 4))
>              return AVERROR_INVALIDDATA;
>  
>          for (int i = 0; i < 3; i++) {
> -            params->maxscl[i] =
> -                (AVRational){get_bits(gb, 17), rgb_den};
> +            params->maxscl[i] = (AVRational) { get_bits(gb, 17), rgb_den };
>          }
> -        params->average_maxrgb =
> -            (AVRational){get_bits(gb, 17), rgb_den};
> +        params->average_maxrgb = (AVRational) { get_bits(gb, 17), rgb_den };
>          params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
>  
>          if (get_bits_left(gb) <
> @@ -123,14 +123,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>  
>          for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
>              params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
> -            params->distribution_maxrgb[i].percentile =
> -                (AVRational){get_bits(gb, 17), rgb_den};
> +            params->distribution_maxrgb[i].percentile = (AVRational) { get_bits(gb, 17), rgb_den };
>          }
>  
>          if (get_bits_left(gb) < 10)
>              return AVERROR_INVALIDDATA;
>  
> -        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
> +        params->fraction_bright_pixels = (AVRational) { get_bits(gb, 10), fraction_pixel_den };
>      }
>      if (get_bits_left(gb) < 1)
>          return AVERROR_INVALIDDATA;
> @@ -152,14 +151,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>  
>          for (int i = 0; i < rows; i++) {
>              for (int j = 0; j < cols; j++) {
> -                s->mastering_display_actual_peak_luminance[i][j] =
> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
> +                s->mastering_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
>              }
>          }
>      }
>  
>      for (int w = 0; w < s->num_windows; w++) {
> -        AVHDRPlusColorTransformParams *params = &s->params[w];
> +        AVHDRPlusColorTransformParams* params = &s->params[w];

This is not the codestyle preferred by FFmpeg (rationale: The '*'
belongs to the variable, not the type. After all "T* foo, bar;" defines
foo as pointer to type T and bar as variable of type T, not as pointer
to T). And anyway, it is a cosmetic change.

>          if (get_bits_left(gb) < 1)
>              return AVERROR_INVALIDDATA;
>  
> @@ -168,18 +166,15 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>              if (get_bits_left(gb) < 28)
>                  return AVERROR_INVALIDDATA;
>  
> -            params->knee_point_x =
> -                (AVRational){get_bits(gb, 12), knee_point_den};
> -            params->knee_point_y =
> -                (AVRational){get_bits(gb, 12), knee_point_den};
> +            params->knee_point_x = (AVRational) { get_bits(gb, 12), knee_point_den };
> +            params->knee_point_y = (AVRational) { get_bits(gb, 12), knee_point_den };
>              params->num_bezier_curve_anchors = get_bits(gb, 4);
>  
>              if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
>                  return AVERROR_INVALIDDATA;
>  
>              for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
> -                params->bezier_curve_anchors[i] =
> -                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
> +                params->bezier_curve_anchors[i] = (AVRational) { get_bits(gb, 10), bezier_anchor_den };
>              }
>          }
>  
> @@ -196,3 +191,209 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
>  
>      return 0;
>  }
> +
> +int av_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
> +					     int size)
> +{
> +    uint8_t country_code;
> +    uint16_t provider_code;
> +    uint16_t provider_oriented_code;
> +    uint8_t application_identifier;
> +    GetBitContext gbc, *gb = &gbc;
> +    int ret, offset;
> +
> +    if (!s)
> +        return AVERROR(ENOMEM);

?

> +
> +    if (size < 7)
> +        return AVERROR_INVALIDDATA;
> +
> +    ret = init_get_bits8(gb, data, size);
> +    if (ret < 0)
> +        return ret;
> +
> +    country_code = get_bits(gb, 8);
> +    provider_code = get_bits(gb, 16);
> +
> +    if (country_code != usa_country_code ||
> +        provider_code != smpte_provider_code)
> +        return AVERROR_INVALIDDATA;
> +
> +    // A/341 Amendment – 2094-40
> +    provider_oriented_code = get_bits(gb, 16);
> +    application_identifier = get_bits(gb, 8);
> +    if (provider_oriented_code != smpte2094_40_provider_oriented_code ||
> +        application_identifier != smpte2094_40_application_identifier)
> +        return AVERROR_INVALIDDATA;
> +
> +    offset = get_bits_count(gb) / 8;

This function only performs byte-aligned reads and does not need to use
the GetBit API at all.

> +
> +    return ff_parse_itu_t_t35_to_dynamic_hdr10_plus(s, gb->buffer + offset, size - offset);
> +}
> +
> +static int ff_itu_t_t35_buffer_size(const AVDynamicHDRPlus* s)

ff_ is our prefix for functions with external linkage (as well as
sometimes for static inline functions declared in headers).

> +{
> +    int bit_count = 0;
> +    int w, size;
> +
> +    if (!s)
> +        return 0;
> +
> +    // 7 bytes for country code, provider code, and user identifier.
> +    bit_count += 56;
> +
> +    if (s->num_windows < 1 || s->num_windows > 3)
> +        return 0;
> +    // Count bits for window params.
> +    bit_count += 2 + ((19 * 8 + 1) * (s->num_windows - 1));
> +
> +    bit_count += 28;
> +    if (s->targeted_system_display_actual_peak_luminance_flag) {
> +        int rows, cols;
> +        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
> +        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
> +        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
> +            return 0;
> +
> +        bit_count += (10 + rows * cols * 4);
> +    }
> +    for (w = 0; w < s->num_windows; w++) {
> +        bit_count += (3 * 17 + 17 + 4 + 10) + (s->params[w].num_distribution_maxrgb_percentiles * 24);
> +    }
> +    bit_count++;
> +
> +    if (s->mastering_display_actual_peak_luminance_flag) {
> +        int rows, cols;
> +        rows = s->num_rows_mastering_display_actual_peak_luminance;
> +        cols = s->num_cols_mastering_display_actual_peak_luminance;
> +        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
> +            return 0;
> +
> +        bit_count += (10 + rows * cols * 4);
> +    }
> +
> +    for (w = 0; w < s->num_windows; w++) {
> +        bit_count++;
> +        if (s->params[w].tone_mapping_flag)
> +            bit_count += (28 + s->params[w].num_bezier_curve_anchors * 10);
> +
> +        bit_count++;
> +        if (s->params[w].color_saturation_mapping_flag)
> +            bit_count += 6;
> +    }
> +    size = bit_count / 8;
> +    if (bit_count % 8 != 0)
> +        size++;
> +    return size;
> +}
> +
> +int av_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size)
> +{
> +    int w, i, j;
> +    PutBitContext pbc, *pb = &pbc;
> +
> +    if (!s || !size)
> +        return AVERROR(EINVAL);
> +
> +    *size = ff_itu_t_t35_buffer_size(s);
> +    if (*size <= 0)
> +        return AVERROR(EINVAL);
> +    *data = av_mallocz(*size);
> +    init_put_bits(pb, *data, *size);
> +    if (put_bits_left(pb) < *size) {
> +        av_freep(data);
> +        return AVERROR(EINVAL);
> +    }
> +    put_bits(pb, 8, usa_country_code);
> +
> +    put_bits(pb, 16, smpte_provider_code);
> +    put_bits(pb, 16, smpte2094_40_provider_oriented_code);
> +    put_bits(pb, 8, smpte2094_40_application_identifier);
> +    put_bits(pb, 8, s->application_version);
> +
> +    put_bits(pb, 2, s->num_windows);
> +
> +    for (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) {
> +        int rows, cols;
> +        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
> +        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
> +        put_bits(pb, 5, rows);
> +        put_bits(pb, 5, cols);
> +        for (i = 0; i < rows; i++) {
> +            for (j = 0; j < cols; 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 (w = 0; w < s->num_windows; w++) {
> +        for (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 (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) {
> +        int rows, cols;
> +        rows = s->num_rows_mastering_display_actual_peak_luminance;
> +        cols = s->num_cols_mastering_display_actual_peak_luminance;
> +        put_bits(pb, 5, rows);
> +        put_bits(pb, 5, cols);
> +        for (i = 0; i < rows; i++) {
> +            for (j = 0; j < cols; 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 (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 (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 0;
> +}
> diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
> index cd7acf0432..d52c3e552d 100644
> --- a/libavcodec/dynamic_hdr10_plus.h
> +++ b/libavcodec/dynamic_hdr10_plus.h
> @@ -29,7 +29,29 @@
>   *
>   * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
>   */
> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
> -                                             int size);
> +int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
> +					     int size);
> +
> +/**
> + * Parse the user data registered ITU-T T.35 with header to AVDynamicHDRPlus. At first check
> + * the header if the provider code is SMPTE-2094-40. Then will parse the data to AVDynamicHDRPlus.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + * @param data The byte array containing the raw ITU-T T.35 data with header.
> + * @param size Size of the data array in bytes.
> + *
> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
> + */
> +int av_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
> +					     int size);
> +
> +/**
> + * Encode and write AVDynamicHDRPlus to the user data registered ITU-T T.3 with header (containing the provider code).
> + * @param s A pointer containing the AVDynamicHDRPlus structure.
> + * @param data The byte array containing the raw ITU-T T.35 data with header.
> + * @param size The size of the raw ITU-T T.35 data.
> + *
> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
> + */
> +int av_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size);
>  
>  #endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index 45077ed33f..c3cf9eef53 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -358,6 +358,11 @@ typedef enum {
>    MATROSKA_VIDEO_PROJECTION_TYPE_MESH               = 3,
>  } MatroskaVideoProjectionType;
>  
> +typedef enum {
> +  MATROSKA_BLOCK_ADD_ID_DEFAULT                     = 0,

The default value of BlockAddID is actually 1. 0 is an invalid value for
BlockAddID.

> +  MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS          = 4,
> +} MatroskaBlockAddID;
> +
>  /*
>   * Matroska Codec IDs, strings
>   */
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 16a3e93611..7fa4483ba4 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -50,6 +50,7 @@
>  #include "libavutil/spherical.h"
>  
>  #include "libavcodec/bytestream.h"
> +#include "libavcodec/dynamic_hdr10_plus.h"
>  #include "libavcodec/flac.h"
>  #include "libavcodec/mpeg4audio.h"
>  #include "libavcodec/packet_internal.h"
> @@ -3636,15 +3637,28 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska,
>      pkt->stream_index = st->index;
>  
>      if (additional_size > 0) {
> -        uint8_t *side_data = av_packet_new_side_data(pkt,
> -                                                     AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
> -                                                     additional_size + 8);
> -        if (!side_data) {
> -            av_packet_unref(pkt);
> -            return AVERROR(ENOMEM);
> +        if (additional_id == MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS) {
> +            AVDynamicHDRPlus hdr10_plus;
> +            if (!av_parse_full_itu_t_t35_to_dynamic_hdr10_plus(&hdr10_plus, additional, additional_size)) {
> +                uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, sizeof(hdr10_plus));
> +                if (!side_data) {
> +                    av_packet_unref(pkt);
> +                    av_free(pkt);

Where did you get that from?

> +                    return AVERROR(ENOMEM);
> +                }
> +                memcpy(side_data, (uint8_t*)(&hdr10_plus), sizeof(hdr10_plus));
> +                }
> +	} else {
> +	    uint8_t *side_data = av_packet_new_side_data(pkt,
> +                                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
> +                                                         additional_size + 8);
> +            if (!side_data) {
> +                av_packet_unref(pkt);
> +                return AVERROR(ENOMEM);
> +            }
> +            AV_WB64(side_data, additional_id);
> +            memcpy(side_data + 8, additional, additional_size);
>          }
> -        AV_WB64(side_data, additional_id);
> -        memcpy(side_data + 8, additional, additional_size);
>      }
>  
>      if (discard_padding) {
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index ed1ad5039d..665e3b7ec6 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -57,6 +57,7 @@
>  #include "libavutil/stereo3d.h"
>  
>  #include "libavcodec/av1.h"
> +#include "libavcodec/dynamic_hdr10_plus.h"
>  #include "libavcodec/xiph.h"
>  #include "libavcodec/mpeg4audio.h"
>  
> @@ -2490,7 +2491,7 @@ static int mkv_write_header(AVFormatContext *s)
>  
>  #if CONFIG_MATROSKA_MUXER
>  static int mkv_reformat_h2645(MatroskaMuxContext *mkv, AVIOContext *pb,
> -                              const AVPacket *pkt, int *size)
> +                               const AVPacket *pkt, int *size)

Spurious change.

>  {
>      int ret;
>      if (pb) {
> @@ -2591,8 +2592,11 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
>  {
>      uint8_t *side_data;
>      size_t side_data_size;
> -    uint64_t additional_id;
> +    uint64_t additional_id = 0;
>      unsigned track_number = track->track_num;
> +    int has_codec_specific_blockmore = 0;
> +    uint8_t *hdr10_plus_itu_t_t35;
> +    size_t hdr10_plus_itu_t_t35_size = 0;
>      EBML_WRITER(9);
>  
>      mkv->cur_block.track  = track;
> @@ -2627,24 +2631,38 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
>              ebml_writer_add_sint(&writer, MATROSKA_ID_DISCARDPADDING, discard_padding);
>          }
>      }
> +    side_data = av_packet_get_side_data(pkt,
> +                                        AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
> +                                        &side_data_size);
> +    if (side_data && side_data_size > 0)
> +        av_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data, &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
>  
>      side_data = av_packet_get_side_data(pkt,
>                                          AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
>                                          &side_data_size);
> -    if (side_data && side_data_size >= 8 &&
> -        // Only the Codec-specific BlockMore (id == 1) is currently supported.
> -        (additional_id = AV_RB64(side_data)) == 1) {
> +    has_codec_specific_blockmore = side_data && side_data_size >= 8 &&
> +                                   (additional_id = AV_RB64(side_data)) == 1;
> +    // The Codec-specific BlockMore (id == 1) and HDR10+ (id == 4) are supported.
> +    if (has_codec_specific_blockmore || hdr10_plus_itu_t_t35_size > 0) {
>          ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKADDITIONS);
> -        ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
> -        /* Until dbc50f8a our demuxer used a wrong default value
> -         * of BlockAddID, so we write it unconditionally. */
> -        ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
> -        ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
> -                             side_data + 8, side_data_size - 8);
> -        ebml_writer_close_master(&writer);
> +	if (has_codec_specific_blockmore) {
> +            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
> +            /* Until dbc50f8a our demuxer used a wrong default value
> +             * of BlockAddID, so we write it unconditionally. */
> +            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
> +            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
> +                                 side_data + 8, side_data_size - 8);
> +            ebml_writer_close_master(&writer);
> +	}
> +	if (hdr10_plus_itu_t_t35_size > 0) {
> +            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
> +            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS);
> +            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
> +                                 hdr10_plus_itu_t_t35, hdr10_plus_itu_t_t35_size);
> +            ebml_writer_close_master(&writer);
> +	}
>          ebml_writer_close_master(&writer);
>      }
> -

1. You are writing the WebM HDR10+ metadata in Matroska, although this
is not yet specified for Matroska (see
https://github.com/ietf-wg-cellar/matroska-specification/issues/345).
This is invalid.
2. You increased the potential maximum of elements to write, but not the
size of the ebml writer's buffer.
3. Seems like hdr10_plus_itu_t_t35 leaks here (is the allocation even
necessary? Is there an upper bound for the size of the created data?).
4. You are adding lots of checks for whether you should open the
blockadditions master; they are all unnecessary: Open it unconditionally
and close it with ebml_writer_close_or_discard_master() (I added this
function for purposes like this.)

- Andreas

>      if (!force_blockgroup && writer.nb_elements == 2) {
>          /* Nothing except the BlockGroup + Block. Can use a SimpleBlock. */
>          writer.elements++;    // Skip the BlockGroup.
> diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
> index 63e81f121b..068f178d9b 100644
> --- a/tests/fate/matroska.mak
> +++ b/tests/fate/matroska.mak
> @@ -148,6 +148,12 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call TRANSCODE, PCM_S32LE MP2, MATROSKA,      \
>                                                 ARESAMPLE_FILTER              \
>                                                 PCM_S32BE_ENCODER)            \
>                                 += fate-matroska-h264-remux
> +
> +# The input file of the following test contains HDR10+ metadata and so this
> +# test tests correct encoding and decoding HDR10+ for VP9/MKV.
> +FATE_MATROSKA_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER) += fate-matroska-hdr10-plus-metadata
> +fate-matroska-hdr10-plus-metadata: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_frames -select_streams v -v 0 $(TARGET_SAMPLES)/mkv/hdr10_plus_vp9_sample.mkv
> +

This does not test muxing HDR10+ at all, as it just runs ffprobe on an
already existing file. You would need to use a "transcode" test to test
muxing.

>  fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "-show_entries stream=index,codec_name:stream_tags=title,language"
>  
>  # Tests writing BlockAdditional and BlockGroups with ReferenceBlock elements;
> diff --git a/tests/ref/fate/matroska-hdr10-plus-metadata b/tests/ref/fate/matroska-hdr10-plus-metadata
> new file mode 100644
> index 0000000000..0f9ad331ad
> --- /dev/null
> +++ b/tests/ref/fate/matroska-hdr10-plus-metadata
> @@ -0,0 +1,74 @@
> +[FRAME]
> +media_type=video
> +stream_index=0
> +key_frame=1
> +pts=0
> +pts_time=0.000000
> +pkt_dts=0
> +pkt_dts_time=0.000000
> +best_effort_timestamp=0
> +best_effort_timestamp_time=0.000000
> +pkt_duration=40
> +pkt_duration_time=0.040000
> +duration=40
> +duration_time=0.040000
> +pkt_pos=561
> +pkt_size=13350
> +width=1280
> +height=720
> +pix_fmt=yuv420p10le
> +sample_aspect_ratio=1:1
> +pict_type=I
> +coded_picture_number=0
> +display_picture_number=0
> +interlaced_frame=0
> +top_field_first=0
> +repeat_pict=0
> +color_range=tv
> +color_space=unknown
> +color_primaries=unknown
> +color_transfer=unknown
> +chroma_location=unspecified
> +[SIDE_DATA]
> +side_data_type=HDR Dynamic Metadata SMPTE2094-40 (HDR10+)
> +application version=1
> +num_windows=1
> +targeted_system_display_maximum_luminance=400/1
> +maxscl=3340/100000
> +maxscl=2870/100000
> +maxscl=2720/100000
> +average_maxrgb=510/100000
> +num_distribution_maxrgb_percentiles=9
> +distribution_maxrgb_percentage=1
> +distribution_maxrgb_percentile=30/100000
> +distribution_maxrgb_percentage=5
> +distribution_maxrgb_percentile=2940/100000
> +distribution_maxrgb_percentage=10
> +distribution_maxrgb_percentile=255/100000
> +distribution_maxrgb_percentage=25
> +distribution_maxrgb_percentile=70/100000
> +distribution_maxrgb_percentage=50
> +distribution_maxrgb_percentile=1340/100000
> +distribution_maxrgb_percentage=75
> +distribution_maxrgb_percentile=1600/100000
> +distribution_maxrgb_percentage=90
> +distribution_maxrgb_percentile=1850/100000
> +distribution_maxrgb_percentage=95
> +distribution_maxrgb_percentile=1950/100000
> +distribution_maxrgb_percentage=99
> +distribution_maxrgb_percentile=2940/100000
> +fraction_bright_pixels=1/1000
> +knee_point_x=0/4095
> +knee_point_y=0/4095
> +num_bezier_curve_anchors=9
> +bezier_curve_anchors=102/1023
> +bezier_curve_anchors=205/1023
> +bezier_curve_anchors=307/1023
> +bezier_curve_anchors=410/1023
> +bezier_curve_anchors=512/1023
> +bezier_curve_anchors=614/1023
> +bezier_curve_anchors=717/1023
> +bezier_curve_anchors=819/1023
> +bezier_curve_anchors=922/1023
> +[/SIDE_DATA]
> +[/FRAME]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-10-25 23:38         ` Mohammad Izadi
@ 2022-10-26 20:17           ` Michael Niedermayer
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Niedermayer @ 2022-10-26 20:17 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 357 bytes --]

On Tue, Oct 25, 2022 at 04:38:48PM -0700, Mohammad Izadi wrote:
> Michael, I appreciate it if you can take a look and give me your feedback.

It seems Andreas already reveiwed the code

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
  2022-03-30 23:22 ` Mohammad Izadi
@ 2022-04-17 17:15   ` Michael Niedermayer
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Niedermayer @ 2022-04-17 17:15 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1599 bytes --]

On Wed, Mar 30, 2022 at 04:22:23PM -0700, Mohammad Izadi wrote:
> From: Gyan Doshi <ffmpeg@gyani.pro>
> 
> The fate test file can be found here: https://drive.google.com/file/d/1jGW3f94rglLfr5WGmMQe3SEnp1YkbMRy/view?usp=drivesdk
> The video file needs to be copied to fate-suite/mkv/
> ---
>  libavcodec/dynamic_hdr10_plus.c             | 269 +++++++++++++++++---
>  libavcodec/dynamic_hdr10_plus.h             |  35 ++-
>  libavformat/matroska.h                      |   5 +
>  libavformat/matroskadec.c                   |  30 ++-
>  libavformat/matroskaenc.c                   |  44 +++-
>  tests/fate/matroska.mak                     |   6 +
>  tests/ref/fate/matroska-hdr10-plus-metadata | 152 +++++++++++
>  7 files changed, 484 insertions(+), 57 deletions(-)
>  create mode 100644 tests/ref/fate/matroska-hdr10-plus-metadata

fails to build with shared libs

LD	ffplay_g
libavformat/libavformat.so: undefined reference to `ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus'
libavformat/libavformat.so: undefined reference to `ff_write_dynamic_hdr10_plus_to_full_itu_t_t35'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Makefile:127: recipe for target 'ffplay_g' failed
make: *** [ffplay_g] Error 1
make: *** Waiting for unfinished jobs....


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska.
       [not found] <20210818205538.GF4067@pb2>
@ 2022-03-30 23:22 ` Mohammad Izadi
  2022-04-17 17:15   ` Michael Niedermayer
  0 siblings, 1 reply; 12+ messages in thread
From: Mohammad Izadi @ 2022-03-30 23:22 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Gyan Doshi

From: Gyan Doshi <ffmpeg@gyani.pro>

The fate test file can be found here: https://drive.google.com/file/d/1jGW3f94rglLfr5WGmMQe3SEnp1YkbMRy/view?usp=drivesdk
The video file needs to be copied to fate-suite/mkv/
---
 libavcodec/dynamic_hdr10_plus.c             | 269 +++++++++++++++++---
 libavcodec/dynamic_hdr10_plus.h             |  35 ++-
 libavformat/matroska.h                      |   5 +
 libavformat/matroskadec.c                   |  30 ++-
 libavformat/matroskaenc.c                   |  44 +++-
 tests/fate/matroska.mak                     |   6 +
 tests/ref/fate/matroska-hdr10-plus-metadata | 152 +++++++++++
 7 files changed, 484 insertions(+), 57 deletions(-)
 create mode 100644 tests/ref/fate/matroska-hdr10-plus-metadata

diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
index 34a44aac65..ce15569196 100644
--- a/libavcodec/dynamic_hdr10_plus.c
+++ b/libavcodec/dynamic_hdr10_plus.c
@@ -18,6 +18,12 @@
 
 #include "dynamic_hdr10_plus.h"
 #include "get_bits.h"
+#include "put_bits.h"
+
+static const uint8_t usa_country_code = 0xB5;
+static const uint16_t smpte_provider_code = 0x003C;
+static const uint16_t smpte2094_40_provider_oriented_code = 0x0001;
+static const uint16_t smpte2094_40_application_identifier = 0x04;
 
 static const int64_t luminance_den = 1;
 static const int32_t peak_luminance_den = 15;
@@ -27,8 +33,8 @@ static const int32_t knee_point_den = 4095;
 static const int32_t bezier_anchor_den = 1023;
 static const int32_t saturation_weight_den = 8;
 
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size)
+int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size)
 {
     GetBitContext gbc, *gb = &gbc;
     int ret;
@@ -40,10 +46,12 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     if (ret < 0)
         return ret;
 
-    if (get_bits_left(gb) < 10)
+    if (get_bits_left(gb) < 8)
         return AVERROR_INVALIDDATA;
+     s->application_version = get_bits(gb, 8);
 
-    s->application_version = get_bits(gb, 8);
+    if (get_bits_left(gb) < 2)
+        return AVERROR_INVALIDDATA;
     s->num_windows = get_bits(gb, 2);
 
     if (s->num_windows < 1 || s->num_windows > 3) {
@@ -56,15 +64,11 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     for (int w = 1; w < s->num_windows; w++) {
         // The corners are set to absolute coordinates here. They should be
         // converted to the relative coordinates (in [0, 1]) in the decoder.
-        AVHDRPlusColorTransformParams *params = &s->params[w];
-        params->window_upper_left_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_upper_left_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
+        AVHDRPlusColorTransformParams* params = &s->params[w];
+        params->window_upper_left_corner_x = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_upper_left_corner_y = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_lower_right_corner_x = (AVRational) { get_bits(gb, 16), 1 };
+        params->window_lower_right_corner_y = (AVRational) { get_bits(gb, 16), 1 };
 
         params->center_of_ellipse_x = get_bits(gb, 16);
         params->center_of_ellipse_y = get_bits(gb, 16);
@@ -78,8 +82,7 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
     if (get_bits_left(gb) < 28)
         return AVERROR_INVALIDDATA;
 
-    s->targeted_system_display_maximum_luminance =
-        (AVRational){get_bits_long(gb, 27), luminance_den};
+    s->targeted_system_display_maximum_luminance = (AVRational) { get_bits_long(gb, 27), luminance_den };
     s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
 
     if (s->targeted_system_display_actual_peak_luminance_flag) {
@@ -99,22 +102,19 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < rows; i++) {
             for (int j = 0; j < cols; j++) {
-                s->targeted_system_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+                s->targeted_system_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
             }
         }
     }
     for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
+        AVHDRPlusColorTransformParams* params = &s->params[w];
         if (get_bits_left(gb) < (3 * 17 + 17 + 4))
             return AVERROR_INVALIDDATA;
 
         for (int i = 0; i < 3; i++) {
-            params->maxscl[i] =
-                (AVRational){get_bits(gb, 17), rgb_den};
+            params->maxscl[i] = (AVRational) { get_bits(gb, 17), rgb_den };
         }
-        params->average_maxrgb =
-            (AVRational){get_bits(gb, 17), rgb_den};
+        params->average_maxrgb = (AVRational) { get_bits(gb, 17), rgb_den };
         params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
 
         if (get_bits_left(gb) <
@@ -123,14 +123,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
             params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
-            params->distribution_maxrgb[i].percentile =
-                (AVRational){get_bits(gb, 17), rgb_den};
+            params->distribution_maxrgb[i].percentile = (AVRational) { get_bits(gb, 17), rgb_den };
         }
 
         if (get_bits_left(gb) < 10)
             return AVERROR_INVALIDDATA;
 
-        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
+        params->fraction_bright_pixels = (AVRational) { get_bits(gb, 10), fraction_pixel_den };
     }
     if (get_bits_left(gb) < 1)
         return AVERROR_INVALIDDATA;
@@ -152,14 +151,13 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
         for (int i = 0; i < rows; i++) {
             for (int j = 0; j < cols; j++) {
-                s->mastering_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+                s->mastering_display_actual_peak_luminance[i][j] = (AVRational) { get_bits(gb, 4), peak_luminance_den };
             }
         }
     }
 
     for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
+        AVHDRPlusColorTransformParams* params = &s->params[w];
         if (get_bits_left(gb) < 1)
             return AVERROR_INVALIDDATA;
 
@@ -168,18 +166,15 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
             if (get_bits_left(gb) < 28)
                 return AVERROR_INVALIDDATA;
 
-            params->knee_point_x =
-                (AVRational){get_bits(gb, 12), knee_point_den};
-            params->knee_point_y =
-                (AVRational){get_bits(gb, 12), knee_point_den};
+            params->knee_point_x = (AVRational) { get_bits(gb, 12), knee_point_den };
+            params->knee_point_y = (AVRational) { get_bits(gb, 12), knee_point_den };
             params->num_bezier_curve_anchors = get_bits(gb, 4);
 
             if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
                 return AVERROR_INVALIDDATA;
 
             for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
-                params->bezier_curve_anchors[i] =
-                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
+                params->bezier_curve_anchors[i] = (AVRational) { get_bits(gb, 10), bezier_anchor_den };
             }
         }
 
@@ -196,3 +191,209 @@ int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t
 
     return 0;
 }
+
+int ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size)
+{
+    uint8_t country_code;
+    uint16_t provider_code;
+    uint16_t provider_oriented_code;
+    uint8_t application_identifier;
+    GetBitContext gbc, *gb = &gbc;
+    int ret, offset;
+
+    if (!s)
+        return AVERROR(ENOMEM);
+
+    if (size < 7)
+        return AVERROR_INVALIDDATA;
+
+    ret = init_get_bits8(gb, data, size);
+    if (ret < 0)
+        return ret;
+
+    country_code = get_bits(gb, 8);
+    provider_code = get_bits(gb, 16);
+
+    if (country_code != usa_country_code ||
+        provider_code != smpte_provider_code)
+        return AVERROR_INVALIDDATA;
+
+    // A/341 Amendment – 2094-40
+    provider_oriented_code = get_bits(gb, 16);
+    application_identifier = get_bits(gb, 8);
+    if (provider_oriented_code != smpte2094_40_provider_oriented_code ||
+        application_identifier != smpte2094_40_application_identifier)
+        return AVERROR_INVALIDDATA;
+
+    offset = get_bits_count(gb) / 8;
+
+    return ff_parse_itu_t_t35_to_dynamic_hdr10_plus(s, gb->buffer + offset, size - offset);
+}
+
+int ff_itu_t_t35_buffer_size(const AVDynamicHDRPlus* s)
+{
+    int bit_count = 0;
+    int w, size;
+
+    if (!s)
+        return 0;
+
+    // 7 bytes for country code, provider code, and user identifier.
+    bit_count += 56;
+
+    if (s->num_windows < 1 || s->num_windows > 3)
+        return 0;
+    // Count bits for window params.
+    bit_count += 2 + ((19 * 8 + 1) * (s->num_windows - 1));
+
+    bit_count += 28;
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
+        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
+            return 0;
+
+        bit_count += (10 + rows * cols * 4);
+    }
+    for (w = 0; w < s->num_windows; w++) {
+        bit_count += (3 * 17 + 17 + 4 + 10) + (s->params[w].num_distribution_maxrgb_percentiles * 24);
+    }
+    bit_count++;
+
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        rows = s->num_rows_mastering_display_actual_peak_luminance;
+        cols = s->num_cols_mastering_display_actual_peak_luminance;
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25)))
+            return 0;
+
+        bit_count += (10 + rows * cols * 4);
+    }
+
+    for (w = 0; w < s->num_windows; w++) {
+        bit_count++;
+        if (s->params[w].tone_mapping_flag)
+            bit_count += (28 + s->params[w].num_bezier_curve_anchors * 10);
+
+        bit_count++;
+        if (s->params[w].color_saturation_mapping_flag)
+            bit_count += 6;
+    }
+    size = bit_count / 8;
+    if (bit_count % 8 != 0)
+        size++;
+    return size;
+}
+
+int ff_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size)
+{
+    int w, i, j;
+    PutBitContext pbc, *pb = &pbc;
+
+    if (!s || !size)
+        return AVERROR(EINVAL);
+
+    *size = ff_itu_t_t35_buffer_size(s);
+    if (*size <= 0)
+        return AVERROR(EINVAL);
+    *data = av_mallocz(*size);
+    init_put_bits(pb, *data, *size);
+    if (put_bits_left(pb) < *size) {
+        av_freep(data);
+        return AVERROR(EINVAL);
+    }
+    put_bits(pb, 8, usa_country_code);
+
+    put_bits(pb, 16, smpte_provider_code);
+    put_bits(pb, 16, smpte2094_40_provider_oriented_code);
+    put_bits(pb, 8, smpte2094_40_application_identifier);
+    put_bits(pb, 8, s->application_version);
+
+    put_bits(pb, 2, s->num_windows);
+
+    for (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) {
+        int rows, cols;
+        rows = s->num_rows_targeted_system_display_actual_peak_luminance;
+        cols = s->num_cols_targeted_system_display_actual_peak_luminance;
+        put_bits(pb, 5, rows);
+        put_bits(pb, 5, cols);
+        for (i = 0; i < rows; i++) {
+            for (j = 0; j < cols; 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 (w = 0; w < s->num_windows; w++) {
+        for (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 (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) {
+        int rows, cols;
+        rows = s->num_rows_mastering_display_actual_peak_luminance;
+        cols = s->num_cols_mastering_display_actual_peak_luminance;
+        put_bits(pb, 5, rows);
+        put_bits(pb, 5, cols);
+        for (i = 0; i < rows; i++) {
+            for (j = 0; j < cols; 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 (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 (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 0;
+}
diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
index cd7acf0432..dafe548d2d 100644
--- a/libavcodec/dynamic_hdr10_plus.h
+++ b/libavcodec/dynamic_hdr10_plus.h
@@ -29,7 +29,38 @@
  *
  * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
  */
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size);
+int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size);
+
+/**
+ * Parse the user data registered ITU-T T.35 with header to AVDynamicHDRPlus. At first check
+ * the header if the provider code is SMPTE-2094-40. Then will parse the data to AVDynamicHDRPlus.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ * @param data The byte array containing the raw ITU-T T.35 data with header.
+ * @param size Size of the data array in bytes.
+ *
+ * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
+ */
+int ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus* s, const uint8_t* data,
+					     int size);
+
+/**
+ * Get the size of buffer required to save the encoded bit stream of
+ * AVDynamicHDRPlus in the form of the user data registered ITU-T T.35.
+ * @param s The AVDynamicHDRPlus structure.
+ *
+ * @return The size of bit stream required for encoding. 0 if the data is invalid.
+ */
+int ff_itu_t_t35_buffer_size(const AVDynamicHDRPlus* s);
+
+/**
+ * Encode and write AVDynamicHDRPlus to the user data registered ITU-T T.3 with header (containing the provider code).
+ * @param s A pointer containing the AVDynamicHDRPlus structure.
+ * @param data The byte array containing the raw ITU-T T.35 data with header.
+ * @param size The size of the raw ITU-T T.35 data.
+ *
+ * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
+ */
+int ff_write_dynamic_hdr10_plus_to_full_itu_t_t35(const AVDynamicHDRPlus* s, uint8_t** data, size_t* size);
 
 #endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 45077ed33f..c3cf9eef53 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -358,6 +358,11 @@ typedef enum {
   MATROSKA_VIDEO_PROJECTION_TYPE_MESH               = 3,
 } MatroskaVideoProjectionType;
 
+typedef enum {
+  MATROSKA_BLOCK_ADD_ID_DEFAULT                     = 0,
+  MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS          = 4,
+} MatroskaBlockAddID;
+
 /*
  * Matroska Codec IDs, strings
  */
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index d97fc33d44..de8adf12ee 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -49,6 +49,7 @@
 #include "libavutil/spherical.h"
 
 #include "libavcodec/bytestream.h"
+#include "libavcodec/dynamic_hdr10_plus.h"
 #include "libavcodec/flac.h"
 #include "libavcodec/mpeg4audio.h"
 #include "libavcodec/packet_internal.h"
@@ -3632,15 +3633,28 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska,
     pkt->stream_index = st->index;
 
     if (additional_size > 0) {
-        uint8_t *side_data = av_packet_new_side_data(pkt,
-                                                     AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
-                                                     additional_size + 8);
-        if (!side_data) {
-            av_packet_unref(pkt);
-            return AVERROR(ENOMEM);
+        if (additional_id == MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS) {
+            AVDynamicHDRPlus hdr10_plus;
+            if (!ff_parse_full_itu_t_t35_to_dynamic_hdr10_plus(&hdr10_plus, additional, additional_size)) {
+                uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, sizeof(hdr10_plus));
+                if (!side_data) {
+                    av_packet_unref(pkt);
+                    av_free(pkt);
+                    return AVERROR(ENOMEM);
+                }
+                memcpy(side_data, (uint8_t*)(&hdr10_plus), sizeof(hdr10_plus));
+                }
+	} else {
+	    uint8_t *side_data = av_packet_new_side_data(pkt,
+                                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
+                                                         additional_size + 8);
+            if (!side_data) {
+                av_packet_unref(pkt);
+                return AVERROR(ENOMEM);
+            }
+            AV_WB64(side_data, additional_id);
+            memcpy(side_data + 8, additional, additional_size);
         }
-        AV_WB64(side_data, additional_id);
-        memcpy(side_data + 8, additional, additional_size);
     }
 
     if (discard_padding) {
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 3b8ca11f28..7a775e7d4e 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -56,6 +56,7 @@
 #include "libavutil/samplefmt.h"
 #include "libavutil/stereo3d.h"
 
+#include "libavcodec/dynamic_hdr10_plus.h"
 #include "libavcodec/xiph.h"
 #include "libavcodec/mpeg4audio.h"
 
@@ -2412,7 +2413,7 @@ static int mkv_write_header(AVFormatContext *s)
 
 #if CONFIG_MATROSKA_MUXER
 static int mkv_reformat_h2645(MatroskaMuxContext *mkv, AVIOContext *pb,
-                              const AVPacket *pkt, int *size)
+                               const AVPacket *pkt, int *size)
 {
     int ret;
     if (pb) {
@@ -2513,9 +2514,12 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
 {
     uint8_t *side_data;
     size_t side_data_size;
-    uint64_t additional_id;
+    uint64_t additional_id = 0;
     int64_t discard_padding = 0;
     unsigned track_number = track->track_num;
+    int has_codec_specific_blockmore = 0;
+    uint8_t *hdr10_plus_itu_t_t35;
+    size_t hdr10_plus_itu_t_t35_size = 0;
     EBML_WRITER(9);
 
     mkv->cur_block.track  = track;
@@ -2547,24 +2551,38 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
                                        (AVRational){1, 1000000000});
         ebml_writer_add_sint(&writer, MATROSKA_ID_DISCARDPADDING, discard_padding);
     }
+    side_data = av_packet_get_side_data(pkt,
+                                        AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
+                                        &side_data_size);
+    if (side_data && side_data_size > 0)
+        ff_write_dynamic_hdr10_plus_to_full_itu_t_t35((AVDynamicHDRPlus*)side_data, &hdr10_plus_itu_t_t35, &hdr10_plus_itu_t_t35_size);
 
     side_data = av_packet_get_side_data(pkt,
                                         AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
                                         &side_data_size);
-    if (side_data && side_data_size >= 8 &&
-        // Only the Codec-specific BlockMore (id == 1) is currently supported.
-        (additional_id = AV_RB64(side_data)) == 1) {
+    has_codec_specific_blockmore = side_data && side_data_size >= 8 &&
+                                   (additional_id = AV_RB64(side_data)) == 1;
+    // The Codec-specific BlockMore (id == 1) and HDR10+ (id == 4) are supported.
+    if (has_codec_specific_blockmore || hdr10_plus_itu_t_t35_size > 0) {
         ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKADDITIONS);
-        ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
-        /* Until dbc50f8a our demuxer used a wrong default value
-         * of BlockAddID, so we write it unconditionally. */
-        ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
-        ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
-                             side_data + 8, side_data_size - 8);
-        ebml_writer_close_master(&writer);
+	if (has_codec_specific_blockmore) {
+            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
+            /* Until dbc50f8a our demuxer used a wrong default value
+             * of BlockAddID, so we write it unconditionally. */
+            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, additional_id);
+            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
+                                 side_data + 8, side_data_size - 8);
+            ebml_writer_close_master(&writer);
+	}
+	if (hdr10_plus_itu_t_t35_size > 0) {
+            ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
+            ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKADDID, MATROSKA_BLOCK_ADD_ID_DYNAMIC_HDR10_PLUS);
+            ebml_writer_add_bin (&writer, MATROSKA_ID_BLOCKADDITIONAL,
+                                 hdr10_plus_itu_t_t35, hdr10_plus_itu_t_t35_size);
+            ebml_writer_close_master(&writer);
+	}
         ebml_writer_close_master(&writer);
     }
-
     if (!force_blockgroup && writer.nb_elements == 2) {
         /* Nothing except the BlockGroup + Block. Can use a SimpleBlock. */
         writer.elements++;    // Skip the BlockGroup.
diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index 3073b0a061..43800cb2d7 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -135,6 +135,12 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER MATROSKA_MUXER \
                                += fate-matroska-spherical-mono-remux
 fate-matroska-spherical-mono-remux: CMD = transcode matroska $(TARGET_SAMPLES)/mkv/spherical.mkv matroska "-map 0 -map 0 -c copy -disposition:0 -default+forced -disposition:1 -default -default_mode passthrough -color_primaries:1 bt709 -color_trc:1 smpte170m -colorspace:1 bt2020c -color_range:1 pc"  "-map 0 -c copy -t 0" "" "-show_entries stream_side_data_list:stream_disposition=default,forced:stream=color_range,color_space,color_primaries,color_transfer"
 
+# The input file of the following test contains HDR10+ metadata and so this
+# test tests correct encoding and decoding HDR10+ for VP9/MKV.
+FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER MATROSKA_MUXER) \
+                               += fate-matroska-hdr10-plus-metadata
+fate-matroska-hdr10-plus-metadata: CMD = transcode matroska $(TARGET_SAMPLES)/mkv/hdr10_plus_vp9_sample.mkv matroska "-map 0 -map 0 -c copy -pixel_format yuv420p10le" "" "" "-show_frames"
+
 # The input file of the following test contains Content Light Level as well as
 # Mastering Display Metadata and so this test tests correct muxing and demuxing
 # of these. It furthermore also tests that this data is correctly propagated
diff --git a/tests/ref/fate/matroska-hdr10-plus-metadata b/tests/ref/fate/matroska-hdr10-plus-metadata
new file mode 100644
index 0000000000..a461fb9674
--- /dev/null
+++ b/tests/ref/fate/matroska-hdr10-plus-metadata
@@ -0,0 +1,152 @@
+8e184fbb8ec8bf26c54ceb15c21108f4 *tests/data/fate/matroska-hdr10-plus-metadata.matroska
+27476 tests/data/fate/matroska-hdr10-plus-metadata.matroska
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 1280x720
+#sar 0: 1/1
+0,          0,          0,        1,  2764800, 0xf2617bf2
+[FRAME]
+media_type=video
+stream_index=0
+key_frame=1
+pts=0
+pts_time=0.000000
+pkt_dts=0
+pkt_dts_time=0.000000
+best_effort_timestamp=0
+best_effort_timestamp_time=0.000000
+pkt_duration=40
+pkt_duration_time=0.040000
+pkt_pos=559
+pkt_size=13350
+width=1280
+height=720
+pix_fmt=yuv420p10le
+sample_aspect_ratio=1:1
+pict_type=I
+coded_picture_number=0
+display_picture_number=0
+interlaced_frame=0
+top_field_first=0
+repeat_pict=0
+color_range=tv
+color_space=unknown
+color_primaries=unknown
+color_transfer=unknown
+chroma_location=unspecified
+[SIDE_DATA]
+side_data_type=HDR Dynamic Metadata SMPTE2094-40 (HDR10+)
+application version=1
+num_windows=1
+targeted_system_display_maximum_luminance=400/1
+maxscl=3340/100000
+maxscl=2870/100000
+maxscl=2720/100000
+average_maxrgb=510/100000
+num_distribution_maxrgb_percentiles=9
+distribution_maxrgb_percentage=1
+distribution_maxrgb_percentile=30/100000
+distribution_maxrgb_percentage=5
+distribution_maxrgb_percentile=2940/100000
+distribution_maxrgb_percentage=10
+distribution_maxrgb_percentile=255/100000
+distribution_maxrgb_percentage=25
+distribution_maxrgb_percentile=70/100000
+distribution_maxrgb_percentage=50
+distribution_maxrgb_percentile=1340/100000
+distribution_maxrgb_percentage=75
+distribution_maxrgb_percentile=1600/100000
+distribution_maxrgb_percentage=90
+distribution_maxrgb_percentile=1850/100000
+distribution_maxrgb_percentage=95
+distribution_maxrgb_percentile=1950/100000
+distribution_maxrgb_percentage=99
+distribution_maxrgb_percentile=2940/100000
+fraction_bright_pixels=1/1000
+knee_point_x=0/4095
+knee_point_y=0/4095
+num_bezier_curve_anchors=9
+bezier_curve_anchors=102/1023
+bezier_curve_anchors=205/1023
+bezier_curve_anchors=307/1023
+bezier_curve_anchors=410/1023
+bezier_curve_anchors=512/1023
+bezier_curve_anchors=614/1023
+bezier_curve_anchors=717/1023
+bezier_curve_anchors=819/1023
+bezier_curve_anchors=922/1023
+[/SIDE_DATA]
+[/FRAME]
+[FRAME]
+media_type=video
+stream_index=1
+key_frame=1
+pts=0
+pts_time=0.000000
+pkt_dts=0
+pkt_dts_time=0.000000
+best_effort_timestamp=0
+best_effort_timestamp_time=0.000000
+pkt_duration=40
+pkt_duration_time=0.040000
+pkt_pos=14008
+pkt_size=13350
+width=1280
+height=720
+pix_fmt=yuv420p10le
+sample_aspect_ratio=1:1
+pict_type=I
+coded_picture_number=0
+display_picture_number=0
+interlaced_frame=0
+top_field_first=0
+repeat_pict=0
+color_range=tv
+color_space=unknown
+color_primaries=unknown
+color_transfer=unknown
+chroma_location=unspecified
+[SIDE_DATA]
+side_data_type=HDR Dynamic Metadata SMPTE2094-40 (HDR10+)
+application version=1
+num_windows=1
+targeted_system_display_maximum_luminance=400/1
+maxscl=3340/100000
+maxscl=2870/100000
+maxscl=2720/100000
+average_maxrgb=510/100000
+num_distribution_maxrgb_percentiles=9
+distribution_maxrgb_percentage=1
+distribution_maxrgb_percentile=30/100000
+distribution_maxrgb_percentage=5
+distribution_maxrgb_percentile=2940/100000
+distribution_maxrgb_percentage=10
+distribution_maxrgb_percentile=255/100000
+distribution_maxrgb_percentage=25
+distribution_maxrgb_percentile=70/100000
+distribution_maxrgb_percentage=50
+distribution_maxrgb_percentile=1340/100000
+distribution_maxrgb_percentage=75
+distribution_maxrgb_percentile=1600/100000
+distribution_maxrgb_percentage=90
+distribution_maxrgb_percentile=1850/100000
+distribution_maxrgb_percentage=95
+distribution_maxrgb_percentile=1950/100000
+distribution_maxrgb_percentage=99
+distribution_maxrgb_percentile=2940/100000
+fraction_bright_pixels=1/1000
+knee_point_x=0/4095
+knee_point_y=0/4095
+num_bezier_curve_anchors=9
+bezier_curve_anchors=102/1023
+bezier_curve_anchors=205/1023
+bezier_curve_anchors=307/1023
+bezier_curve_anchors=410/1023
+bezier_curve_anchors=512/1023
+bezier_curve_anchors=614/1023
+bezier_curve_anchors=717/1023
+bezier_curve_anchors=819/1023
+bezier_curve_anchors=922/1023
+[/SIDE_DATA]
+[/FRAME]
-- 
2.35.1.1021.g381101b075-goog

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-10-26 20:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220417172658.GN3529341@pb2>
2022-09-06 21:47 ` [FFmpeg-devel] [PATCH] avformat/matroska: Support HDR10+ metadata in Matroska Mohammad Izadi
2022-09-06 22:29   ` Michael Niedermayer
2022-09-07 13:12   ` Derek Buitenhuis
2022-09-08  0:07     ` Mohammad Izadi
2022-09-08  1:22     ` Mohammad Izadi
2022-10-26  0:16       ` Andreas Rheinhardt
2022-09-08 16:31     ` Michael Niedermayer
2022-09-08 17:03       ` Mohammad Izadi
2022-10-25 23:38         ` Mohammad Izadi
2022-10-26 20:17           ` Michael Niedermayer
     [not found] <20210818205538.GF4067@pb2>
2022-03-30 23:22 ` Mohammad Izadi
2022-04-17 17:15   ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git