[FFmpeg-devel] [PATCH 1/2] cbs_av1: Add tracing headers for metadata types

[FFmpeg-devel] [PATCH 1/2] cbs_av1: Add tracing headers for metadata types

[Mark Thompson]

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

_______________________________________________
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] [PATCH 2/2] cbs_av1: Don't reject unknown metadata

[Mark Thompson]

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, &current->metadata.timecode));
          break;
      default:
-        // Unknown metadata type.
-        return AVERROR_PATCHWELCOME;
+        CHECK(FUNC(metadata_unknown)(ctx, rw, &current->metadata.unknown));
      }

      return 0;
-- 
2.39.0
_______________________________________________
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] [PATCH 3/2] cbs_av1: Remove constraint on MDCV luminance values

[Mark Thompson]

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

Re: [FFmpeg-devel] [PATCH 3/2] cbs_av1: Remove constraint on MDCV luminance values

[James Almer]

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

Re: [FFmpeg-devel] [PATCH 3/2] cbs_av1: Remove constraint on MDCV luminance values

[Mark Thompson]

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

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