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

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Raphaël Zumer:
> Co-authored-by: Mohammad Izadi <moh.izadi@gmail.com>
> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
> ---
>  doc/APIchanges                   |   5 ++
>  libavutil/hdr_dynamic_metadata.c | 145 +++++++++++++++++++++++++++++++
>  libavutil/hdr_dynamic_metadata.h |  13 +++
>  libavutil/version.h              |   2 +-
>  4 files changed, 164 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 14737223cb..3a61d61931 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,11 @@ The last version increases of all libraries were on 2023-02-09
>  
>  API changes, most recent first:
>  
> +2023-03-13 - xxxxxxxxxx - lavu 58.4.100 - hdr_dynamic_metadata.h
> +  Add av_dynamic_hdr_plus_from_t35() and av_dynamic_hdr_plus_to_t35()
> +  functions to convert between raw T.35 payloads containing dynamic
> +  HDR10+ metadata and their parsed representations as AVDynamicHDRPlus.
> +
>  2023-03-02 - xxxxxxxxxx - lavc 60.6.100 - avcodec.h
>    Add FF_PROFILE_EAC3_DDP_ATMOS, FF_PROFILE_TRUEHD_ATMOS,
>    FF_PROFILE_DTS_HD_MA_X and FF_PROFILE_DTS_HD_MA_X_IMAX.
> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
> index 98f399b032..9e35d929c7 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -225,3 +225,148 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>  
>      return 0;
>  }
> +
> +int av_dynamic_hdr_plus_to_t35(uint8_t **data, size_t *size, const AVDynamicHDRPlus *s)
> +{
> +    uint8_t *buf;
> +    size_t size_bits, size_bytes;
> +    PutBitContext pbc, *pb = &pbc;
> +
> +    if (!data || !size || !s)

size being mandatory is different from similar APIs. There is even a
usecase without size: If you simply feed this to something that expects
the data to be serialized and trust the data to be complete, you don't
need the size.

> +        return AVERROR(EINVAL);
> +
> +    /**
> +     * 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_malloc(size_bytes);

You are allocating without any padding. This implies that one could not
use this buffer with our GetBit-API or in other places where one needed
a padded buffer.

> +    if (!buf)
> +        return AVERROR(ENOMEM);
> +
> +    init_put_bits(pb, buf, 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);
> +
> +    *data = buf;
> +    *size = size_bytes;
> +    return 0;
> +}
> diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
> index 1f953ef1f5..f2d78051ab 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,16 @@ 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 data A pointer to a byte buffer to be allocated and filled
> + *             with the serialized metadata.
> + * @param size A pointer to a size to be set to the returned buffer's size.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + *
> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
> + */
> +int av_dynamic_hdr_plus_to_t35(uint8_t **data, size_t *size, const 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".