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

["Raphaël Zumer" <raphael.zumer@vimeo.com>]

Hi,

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

Please let me know if I missed anything.

Thanks,
Raphaël Zumer

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

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