[PATCH 1/2] cbs_av1: Add tracing headers for metadata types
Make it a little easier to interpret metadata in trace output. --- libavcodec/cbs_av1_syntax_template.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index d98d3d42de..46f4c5a6b8 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -1843,6 +1843,8 @@ static int FUNC(metadata_hdr_cll)(CodedBitstreamContext *ctx, RWContext *rw, { int err; + HEADER("HDR CLL Metadata"); + fb(16, max_cll); fb(16, max_fall); @@ -1854,6 +1856,8 @@ static int FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw, { int err, i; + HEADER("HDR MDCV Metadata"); + for (i = 0; i < 3; i++) { fbs(16, primary_chromaticity_x[i], 1, i); fbs(16, primary_chromaticity_y[i], 1, i); @@ -1923,6 +1927,8 @@ static int FUNC(metadata_scalability)(CodedBitstreamContext *ctx, RWContext *rw, { int err; + HEADER("Scalability Metadata"); + fb(8, scalability_mode_idc); if (current->scalability_mode_idc == AV1_SCALABILITY_SS) @@ -1937,6 +1943,8 @@ static int FUNC(metadata_itut_t35)(CodedBitstreamContext *ctx, RWContext *rw, int err; size_t i; + HEADER("ITU-T T.35 Metadata"); + fb(8, itu_t_t35_country_code); if (current->itu_t_t35_country_code == 0xff) fb(8, itu_t_t35_country_code_extension_byte); @@ -1964,6 +1972,8 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, { int err; + HEADER("Timecode Metadata"); + fb(5, counting_type); flag(full_timestamp_flag); flag(discontinuity_flag); -- 2.39.0
Accept it and pass it through unchanged. The standard requires that decoders ignore unknown metadata, and indeed this is tested by some of the Argon coverage streams. --- Artificial test example: [trace_headers @ 0x5596dda6ed80] OBU header [trace_headers @ 0x5596dda6ed80] 0 obu_forbidden_bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 1 obu_type 0101 = 5 [trace_headers @ 0x5596dda6ed80] 5 obu_extension_flag 0 = 0 [trace_headers @ 0x5596dda6ed80] 6 obu_has_size_field 1 = 1 [trace_headers @ 0x5596dda6ed80] 7 obu_reserved_1bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 8 leb128_byte[0] 00001101 = 13 [trace_headers @ 0x5596dda6ed80] 8 obu_size = 13 [trace_headers @ 0x5596dda6ed80] 16 leb128_byte[0] 00101010 = 42 [trace_headers @ 0x5596dda6ed80] 16 metadata_type = 42 [trace_headers @ 0x5596dda6ed80] Unknown Metadata [trace_headers @ 0x5596dda6ed80] 24 payload[0] 00000000 = 0 [trace_headers @ 0x5596dda6ed80] 32 payload[1] 00000001 = 1 [trace_headers @ 0x5596dda6ed80] 40 payload[2] 00000010 = 2 [trace_headers @ 0x5596dda6ed80] 48 payload[3] 00000011 = 3 [trace_headers @ 0x5596dda6ed80] 56 payload[4] 00000100 = 4 [trace_headers @ 0x5596dda6ed80] 64 payload[5] 00000101 = 5 [trace_headers @ 0x5596dda6ed80] 72 payload[6] 00000110 = 6 [trace_headers @ 0x5596dda6ed80] 80 payload[7] 00000111 = 7 [trace_headers @ 0x5596dda6ed80] 88 payload[8] 00001000 = 8 [trace_headers @ 0x5596dda6ed80] 96 payload[9] 00001001 = 9 [trace_headers @ 0x5596dda6ed80] 104 payload[10] 00001010 = 10 [trace_headers @ 0x5596dda6ed80] 112 trailing_one_bit 1 = 1 [trace_headers @ 0x5596dda6ed80] 113 trailing_zero_bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 114 trailing_zero_bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 115 trailing_zero_bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 116 trailing_zero_bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 117 trailing_zero_bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 118 trailing_zero_bit 0 = 0 [trace_headers @ 0x5596dda6ed80] 119 trailing_zero_bit 0 = 0 libavcodec/cbs_av1.c | 7 +++++++ libavcodec/cbs_av1.h | 7 +++++++ libavcodec/cbs_av1_syntax_template.c | 26 ++++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c index 45e1288a51..8788fee099 100644 --- a/libavcodec/cbs_av1.c +++ b/libavcodec/cbs_av1.c @@ -1306,9 +1306,16 @@ static void cbs_av1_free_metadata(void *unit, uint8_t *content) md = &obu->obu.metadata; switch (md->metadata_type) { + case AV1_METADATA_TYPE_HDR_CLL: + case AV1_METADATA_TYPE_HDR_MDCV: + case AV1_METADATA_TYPE_SCALABILITY: + case AV1_METADATA_TYPE_TIMECODE: + break; case AV1_METADATA_TYPE_ITUT_T35: av_buffer_unref(&md->metadata.itut_t35.payload_ref); break; + default: + av_buffer_unref(&md->metadata.unknown.payload_ref); } av_free(content); } diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h index 1fc80dcfa0..36848d4410 100644 --- a/libavcodec/cbs_av1.h +++ b/libavcodec/cbs_av1.h @@ -370,6 +370,12 @@ typedef struct AV1RawMetadataTimecode { uint32_t time_offset_value; } AV1RawMetadataTimecode; +typedef struct AV1RawMetadataUnknown { + uint8_t *payload; + AVBufferRef *payload_ref; + size_t payload_size; +} AV1RawMetadataUnknown; + typedef struct AV1RawMetadata { uint64_t metadata_type; union { @@ -378,6 +384,7 @@ typedef struct AV1RawMetadata { AV1RawMetadataScalability scalability; AV1RawMetadataITUTT35 itut_t35; AV1RawMetadataTimecode timecode; + AV1RawMetadataUnknown unknown; } metadata; } AV1RawMetadata; diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index 46f4c5a6b8..3cab02bdd9 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -2007,6 +2007,29 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, return 0; } +static int FUNC(metadata_unknown)(CodedBitstreamContext *ctx, RWContext *rw, + AV1RawMetadataUnknown *current) +{ + int err; + size_t i; + + HEADER("Unknown Metadata"); + +#ifdef READ + current->payload_size = cbs_av1_get_payload_bytes_left(rw); + + current->payload_ref = av_buffer_alloc(current->payload_size); + if (!current->payload_ref) + return AVERROR(ENOMEM); + current->payload = current->payload_ref->data; +#endif + + for (i = 0; i < current->payload_size; i++) + fbs(8, payload[i], 1, i); + + return 0; +} + static int FUNC(metadata_obu)(CodedBitstreamContext *ctx, RWContext *rw, AV1RawMetadata *current) { @@ -2031,8 +2054,7 @@ static int FUNC(metadata_obu)(CodedBitstreamContext *ctx, RWContext *rw, CHECK(FUNC(metadata_timecode)(ctx, rw, ¤t->metadata.timecode)); break; default: - // Unknown metadata type. - return AVERROR_PATCHWELCOME; + CHECK(FUNC(metadata_unknown)(ctx, rw, ¤t->metadata.unknown)); } return 0; -- 2.39.0
While desiring min to be less than max feels entirely sensible, unfortunately the standard does not actually have this requirement. --- Some of the Argon coverage streams test this. libavcodec/cbs_av1_syntax_template.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index 3cab02bdd9..dc6724cb59 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -1866,11 +1866,8 @@ static int FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw, fb(16, white_point_chromaticity_x); fb(16, white_point_chromaticity_y); - fc(32, luminance_max, 1, MAX_UINT_BITS(32)); - // luminance_min must be lower than luminance_max. Convert luminance_max from - // 24.8 fixed point to 18.14 fixed point in order to compare them. - fc(32, luminance_min, 0, FFMIN(((uint64_t)current->luminance_max << 6) - 1, - MAX_UINT_BITS(32))); + fb(32, luminance_max); + fb(32, luminance_min); return 0; } -- 2.39.0
On 1/18/2023 5:35 PM, Mark Thompson wrote:
While desiring min to be less than max feels entirely sensible, unfortunately the standard does not actually have this requirement.
Huh, it really doesn't.
--- Some of the Argon coverage streams test this.
libavcodec/cbs_av1_syntax_template.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index 3cab02bdd9..dc6724cb59 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -1866,11 +1866,8 @@ static int FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw, fb(16, white_point_chromaticity_x); fb(16, white_point_chromaticity_y);
- fc(32, luminance_max, 1, MAX_UINT_BITS(32)); - // luminance_min must be lower than luminance_max. Convert luminance_max from - // 24.8 fixed point to 18.14 fixed point in order to compare them. - fc(32, luminance_min, 0, FFMIN(((uint64_t)current->luminance_max << 6) - 1, - MAX_UINT_BITS(32))); + fb(32, luminance_max); + fb(32, luminance_min);
return 0; }
LGTM.
On 18/01/2023 20:38, James Almer wrote:
On 1/18/2023 5:35 PM, Mark Thompson wrote:
While desiring min to be less than max feels entirely sensible, unfortunately the standard does not actually have this requirement.
Huh, it really doesn't.
--- Some of the Argon coverage streams test this.
libavcodec/cbs_av1_syntax_template.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index 3cab02bdd9..dc6724cb59 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -1866,11 +1866,8 @@ static int FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw, fb(16, white_point_chromaticity_x); fb(16, white_point_chromaticity_y);
- fc(32, luminance_max, 1, MAX_UINT_BITS(32)); - // luminance_min must be lower than luminance_max. Convert luminance_max from - // 24.8 fixed point to 18.14 fixed point in order to compare them. - fc(32, luminance_min, 0, FFMIN(((uint64_t)current->luminance_max << 6) - 1, - MAX_UINT_BITS(32))); + fb(32, luminance_max); + fb(32, luminance_min);
return 0; }
LGTM.
Applied. Thanks, - Mark
participants (2)
-
James Almer -
Mark Thompson