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

[James Almer]

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

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

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

So i think this function should not set the country code at all as part 
of the payload, and start with itu_t_t35_terminal_provider_code.