Change HdrMetadataExtension to ColorSpaceExtension
Bug: webrtc:8651
Change-Id: Ica6f8c6bd13bb07f89700b9c0a359b9a58feefbb
Reviewed-on: https://webrtc-review.googlesource.com/c/111758
Commit-Queue: Johannes Kron <kron@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25800}diff --git a/api/rtp_headers.h b/api/rtp_headers.h
index 3e51f43..c766899 100644
--- a/api/rtp_headers.h
+++ b/api/rtp_headers.h
@@ -17,7 +17,7 @@
#include "absl/types/optional.h"
#include "api/array_view.h"
-#include "api/video/hdr_metadata.h"
+#include "api/video/color_space.h"
#include "api/video/video_content_type.h"
#include "api/video/video_frame_marking.h"
#include "api/video/video_rotation.h"
@@ -129,7 +129,7 @@
// https://tools.ietf.org/html/draft-ietf-mmusic-sdp-bundle-negotiation-38
Mid mid;
- absl::optional<HdrMetadata> hdr_metadata;
+ absl::optional<ColorSpace> color_space;
};
struct RTPHeader {
diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
index a8bf5a7..ab4fcae 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -104,7 +104,7 @@
kRtpExtensionRepairedRtpStreamId,
kRtpExtensionMid,
kRtpExtensionGenericFrameDescriptor,
- kRtpExtensionHdrMetadata,
+ kRtpExtensionColorSpace,
kRtpExtensionNumberOfExtensions // Must be the last entity in the enum.
};
diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/modules/rtp_rtcp/source/rtp_header_extension_map.cc
index 49857a0..8e0a484 100644
--- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc
+++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc
@@ -43,7 +43,7 @@
CreateExtensionInfo<RepairedRtpStreamId>(),
CreateExtensionInfo<RtpMid>(),
CreateExtensionInfo<RtpGenericFrameDescriptorExtension>(),
- CreateExtensionInfo<HdrMetadataExtension>(),
+ CreateExtensionInfo<ColorSpaceExtension>(),
};
// Because of kRtpExtensionNone, NumberOfExtension is 1 bigger than the actual
diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.cc b/modules/rtp_rtcp/source/rtp_header_extensions.cc
index fe327b3..92694cd 100644
--- a/modules/rtp_rtcp/source/rtp_header_extensions.cc
+++ b/modules/rtp_rtcp/source/rtp_header_extensions.cc
@@ -434,15 +434,18 @@
return true;
}
-// HDR Metadata.
+// Color space including HDR metadata as an optional field.
//
// RTP header extension to carry HDR metadata.
// Float values are upscaled by a static factor and transmitted as integers.
//
+// Data layout with HDR metadata
// 0 1 2 3
// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | ID | length | luminance_max |
+// | ID | length=30 | Primaries | Transfer |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// | Matrix | Range | luminance_max |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
// | | luminance_min |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -454,77 +457,111 @@
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
// | mastering_metadata.white.x and .y |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | max_content_light_level |
+// | max_content_light_level | max_frame_average_light_level |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | max_frame_average_light_level |
+//
+// Data layout without HDR metadata
+// 0 1 2 3
+// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-constexpr RTPExtensionType HdrMetadataExtension::kId;
-constexpr uint8_t HdrMetadataExtension::kValueSizeBytes;
-constexpr const char HdrMetadataExtension::kUri[];
+// | ID | length=4 | Primaries | Transfer |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// | Matrix | Range |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
-bool HdrMetadataExtension::Parse(rtc::ArrayView<const uint8_t> data,
- HdrMetadata* hdr_metadata) {
- RTC_DCHECK(hdr_metadata);
- if (data.size() != kValueSizeBytes)
+constexpr RTPExtensionType ColorSpaceExtension::kId;
+constexpr uint8_t ColorSpaceExtension::kValueSizeBytes;
+constexpr const char ColorSpaceExtension::kUri[];
+
+bool ColorSpaceExtension::Parse(rtc::ArrayView<const uint8_t> data,
+ ColorSpace* color_space) {
+ RTC_DCHECK(color_space);
+ if (data.size() != kValueSizeBytes &&
+ data.size() != kValueSizeBytesWithoutHdrMetadata)
return false;
size_t offset = 0;
- offset += ParseLuminance(data.data() + offset,
- &hdr_metadata->mastering_metadata.luminance_max,
- kLuminanceMaxDenominator);
- offset += ParseLuminance(data.data() + offset,
- &hdr_metadata->mastering_metadata.luminance_min,
- kLuminanceMinDenominator);
- offset += ParseChromaticity(data.data() + offset,
- &hdr_metadata->mastering_metadata.primary_r);
- offset += ParseChromaticity(data.data() + offset,
- &hdr_metadata->mastering_metadata.primary_g);
- offset += ParseChromaticity(data.data() + offset,
- &hdr_metadata->mastering_metadata.primary_b);
- offset += ParseChromaticity(data.data() + offset,
- &hdr_metadata->mastering_metadata.white_point);
- // TODO(kron): Do we need 32 bit here or is it enough with 16 bits?
- // Also, what resolution is needed?
- hdr_metadata->max_content_light_level =
- ByteReader<uint32_t>::ReadBigEndian(data.data() + offset);
- offset += 4;
- hdr_metadata->max_frame_average_light_level =
- ByteReader<uint32_t>::ReadBigEndian(data.data() + offset);
- RTC_DCHECK_EQ(kValueSizeBytes, offset + 4);
+ // Read color space information.
+ if (!color_space->set_primaries_from_uint8(data.data()[offset++]))
+ return false;
+ if (!color_space->set_transfer_from_uint8(data.data()[offset++]))
+ return false;
+ if (!color_space->set_matrix_from_uint8(data.data()[offset++]))
+ return false;
+ if (!color_space->set_range_from_uint8(data.data()[offset++]))
+ return false;
+
+ // Read HDR metadata if it exists, otherwise clear it.
+ if (data.size() == kValueSizeBytesWithoutHdrMetadata) {
+ color_space->set_hdr_metadata(nullptr);
+ } else {
+ HdrMetadata hdr_metadata;
+ offset += ParseLuminance(data.data() + offset,
+ &hdr_metadata.mastering_metadata.luminance_max,
+ kLuminanceMaxDenominator);
+ offset += ParseLuminance(data.data() + offset,
+ &hdr_metadata.mastering_metadata.luminance_min,
+ kLuminanceMinDenominator);
+ offset += ParseChromaticity(data.data() + offset,
+ &hdr_metadata.mastering_metadata.primary_r);
+ offset += ParseChromaticity(data.data() + offset,
+ &hdr_metadata.mastering_metadata.primary_g);
+ offset += ParseChromaticity(data.data() + offset,
+ &hdr_metadata.mastering_metadata.primary_b);
+ offset += ParseChromaticity(data.data() + offset,
+ &hdr_metadata.mastering_metadata.white_point);
+ hdr_metadata.max_content_light_level =
+ ByteReader<uint16_t>::ReadBigEndian(data.data() + offset);
+ offset += 2;
+ hdr_metadata.max_frame_average_light_level =
+ ByteReader<uint16_t>::ReadBigEndian(data.data() + offset);
+ offset += 2;
+ color_space->set_hdr_metadata(&hdr_metadata);
+ }
+ RTC_DCHECK_EQ(ValueSize(*color_space), offset);
return true;
}
-bool HdrMetadataExtension::Write(rtc::ArrayView<uint8_t> data,
- const HdrMetadata& hdr_metadata) {
- RTC_DCHECK_EQ(data.size(), kValueSizeBytes);
+bool ColorSpaceExtension::Write(rtc::ArrayView<uint8_t> data,
+ const ColorSpace& color_space) {
+ RTC_DCHECK(data.size() >= ValueSize(color_space));
size_t offset = 0;
- offset += WriteLuminance(data.data() + offset,
- hdr_metadata.mastering_metadata.luminance_max,
- kLuminanceMaxDenominator);
- offset += WriteLuminance(data.data() + offset,
- hdr_metadata.mastering_metadata.luminance_min,
- kLuminanceMinDenominator);
- offset += WriteChromaticity(data.data() + offset,
- hdr_metadata.mastering_metadata.primary_r);
- offset += WriteChromaticity(data.data() + offset,
- hdr_metadata.mastering_metadata.primary_g);
- offset += WriteChromaticity(data.data() + offset,
- hdr_metadata.mastering_metadata.primary_b);
- offset += WriteChromaticity(data.data() + offset,
- hdr_metadata.mastering_metadata.white_point);
+ // Write color space information.
+ data.data()[offset++] = static_cast<uint8_t>(color_space.primaries());
+ data.data()[offset++] = static_cast<uint8_t>(color_space.transfer());
+ data.data()[offset++] = static_cast<uint8_t>(color_space.matrix());
+ data.data()[offset++] = static_cast<uint8_t>(color_space.range());
- // TODO(kron): Do we need 32 bit here or is it enough with 16 bits?
- // Also, what resolution is needed?
- ByteWriter<uint32_t>::WriteBigEndian(data.data() + offset,
- hdr_metadata.max_content_light_level);
- offset += 4;
- ByteWriter<uint32_t>::WriteBigEndian(
- data.data() + offset, hdr_metadata.max_frame_average_light_level);
- RTC_DCHECK_EQ(kValueSizeBytes, offset + 4);
+ // Write HDR metadata if it exists.
+ if (color_space.hdr_metadata()) {
+ const HdrMetadata& hdr_metadata = *color_space.hdr_metadata();
+ offset += WriteLuminance(data.data() + offset,
+ hdr_metadata.mastering_metadata.luminance_max,
+ kLuminanceMaxDenominator);
+ offset += WriteLuminance(data.data() + offset,
+ hdr_metadata.mastering_metadata.luminance_min,
+ kLuminanceMinDenominator);
+ offset += WriteChromaticity(data.data() + offset,
+ hdr_metadata.mastering_metadata.primary_r);
+ offset += WriteChromaticity(data.data() + offset,
+ hdr_metadata.mastering_metadata.primary_g);
+ offset += WriteChromaticity(data.data() + offset,
+ hdr_metadata.mastering_metadata.primary_b);
+ offset += WriteChromaticity(data.data() + offset,
+ hdr_metadata.mastering_metadata.white_point);
+
+ ByteWriter<uint16_t>::WriteBigEndian(data.data() + offset,
+ hdr_metadata.max_content_light_level);
+ offset += 2;
+ ByteWriter<uint16_t>::WriteBigEndian(
+ data.data() + offset, hdr_metadata.max_frame_average_light_level);
+ offset += 2;
+ }
+ RTC_DCHECK_EQ(ValueSize(color_space), offset);
return true;
}
-size_t HdrMetadataExtension::ParseChromaticity(
+size_t ColorSpaceExtension::ParseChromaticity(
const uint8_t* data,
HdrMasteringMetadata::Chromaticity* p) {
uint16_t chromaticity_x_scaled = ByteReader<uint16_t>::ReadBigEndian(data);
@@ -535,15 +572,15 @@
return 4; // Return number of bytes read.
}
-size_t HdrMetadataExtension::ParseLuminance(const uint8_t* data,
- float* f,
- int denominator) {
+size_t ColorSpaceExtension::ParseLuminance(const uint8_t* data,
+ float* f,
+ int denominator) {
uint32_t luminance_scaled = ByteReader<uint32_t, 3>::ReadBigEndian(data);
*f = static_cast<float>(luminance_scaled) / denominator;
return 3; // Return number of bytes read.
}
-size_t HdrMetadataExtension::WriteChromaticity(
+size_t ColorSpaceExtension::WriteChromaticity(
uint8_t* data,
const HdrMasteringMetadata::Chromaticity& p) {
RTC_DCHECK_GE(p.x, 0.0f);
@@ -555,9 +592,9 @@
return 4; // Return number of bytes written.
}
-size_t HdrMetadataExtension::WriteLuminance(uint8_t* data,
- float f,
- int denominator) {
+size_t ColorSpaceExtension::WriteLuminance(uint8_t* data,
+ float f,
+ int denominator) {
RTC_DCHECK_GE(f, 0.0f);
ByteWriter<uint32_t, 3>::WriteBigEndian(data, std::round(f * denominator));
return 3; // Return number of bytes written.
diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.h b/modules/rtp_rtcp/source/rtp_header_extensions.h
index ba43415..42a6216 100644
--- a/modules/rtp_rtcp/source/rtp_header_extensions.h
+++ b/modules/rtp_rtcp/source/rtp_header_extensions.h
@@ -16,7 +16,7 @@
#include "api/array_view.h"
#include "api/rtp_headers.h"
-#include "api/video/hdr_metadata.h"
+#include "api/video/color_space.h"
#include "api/video/video_content_type.h"
#include "api/video/video_frame_marking.h"
#include "api/video/video_rotation.h"
@@ -182,19 +182,23 @@
static bool IsScalable(uint8_t temporal_id, uint8_t layer_id);
};
-class HdrMetadataExtension {
+class ColorSpaceExtension {
public:
- using value_type = HdrMetadata;
- static constexpr RTPExtensionType kId = kRtpExtensionHdrMetadata;
+ using value_type = ColorSpace;
+ static constexpr RTPExtensionType kId = kRtpExtensionColorSpace;
static constexpr uint8_t kValueSizeBytes = 30;
+ static constexpr uint8_t kValueSizeBytesWithoutHdrMetadata = 4;
// TODO(webrtc:8651): Change to a valid uri.
- static constexpr const char kUri[] = "rtp-hdr-metadata-uri-placeholder";
+ static constexpr const char kUri[] = "rtp-colorspace-uri-placeholder";
static bool Parse(rtc::ArrayView<const uint8_t> data,
- HdrMetadata* hdr_metadata);
- static size_t ValueSize(const HdrMetadata&) { return kValueSizeBytes; }
+ ColorSpace* color_space);
+ static size_t ValueSize(const ColorSpace& color_space) {
+ return color_space.hdr_metadata() ? kValueSizeBytes
+ : kValueSizeBytesWithoutHdrMetadata;
+ }
static bool Write(rtc::ArrayView<uint8_t> data,
- const HdrMetadata& hdr_metadata);
+ const ColorSpace& color_space);
private:
static constexpr int kChromaticityDenominator = 10000; // 0.0001 resolution.
diff --git a/modules/rtp_rtcp/source/rtp_packet_received.cc b/modules/rtp_rtcp/source/rtp_packet_received.cc
index ff7b4e8..f80fad6 100644
--- a/modules/rtp_rtcp/source/rtp_packet_received.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_received.cc
@@ -69,7 +69,7 @@
GetExtension<RepairedRtpStreamId>(&header->extension.repaired_stream_id);
GetExtension<RtpMid>(&header->extension.mid);
GetExtension<PlayoutDelayLimits>(&header->extension.playout_delay);
- header->extension.hdr_metadata = GetExtension<HdrMetadataExtension>();
+ header->extension.color_space = GetExtension<ColorSpaceExtension>();
}
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_packet_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
index 37a9a53..b1c0e42 100644
--- a/modules/rtp_rtcp/source/rtp_packet_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
@@ -203,6 +203,34 @@
hdr_metadata.max_frame_average_light_level = 1789;
return hdr_metadata;
}
+
+ColorSpace CreateTestColorSpace(bool with_hdr_metadata) {
+ ColorSpace color_space(
+ ColorSpace::PrimaryID::kBT709, ColorSpace::TransferID::kGAMMA22,
+ ColorSpace::MatrixID::kSMPTE2085, ColorSpace::RangeID::kFull);
+ if (with_hdr_metadata) {
+ HdrMetadata hdr_metadata = CreateTestHdrMetadata();
+ color_space.set_hdr_metadata(&hdr_metadata);
+ }
+ return color_space;
+}
+
+void TestCreateAndParseColorSpaceExtension(bool with_hdr_metadata) {
+ // Create packet with extension.
+ RtpPacket::ExtensionManager extensions(/*extmap-allow-mixed=*/true);
+ extensions.Register<ColorSpaceExtension>(1);
+ RtpPacket packet(&extensions);
+ const ColorSpace kColorSpace = CreateTestColorSpace(with_hdr_metadata);
+ EXPECT_TRUE(packet.SetExtension<ColorSpaceExtension>(kColorSpace));
+ packet.SetPayloadSize(42);
+
+ // Read packet with the extension.
+ RtpPacketReceived parsed(&extensions);
+ EXPECT_TRUE(parsed.Parse(packet.Buffer()));
+ ColorSpace parsed_color_space;
+ EXPECT_TRUE(parsed.GetExtension<ColorSpaceExtension>(&parsed_color_space));
+ EXPECT_EQ(kColorSpace, parsed_color_space);
+}
} // namespace
TEST(RtpPacketTest, CreateMinimum) {
@@ -819,21 +847,12 @@
EXPECT_EQ(receivied_timing.flags, 0);
}
-TEST(RtpPacketTest, CreateAndParseHdrMetadataExtension) {
- // Create packet with extension.
- RtpPacket::ExtensionManager extensions(/*extmap-allow-mixed=*/true);
- extensions.Register<HdrMetadataExtension>(1);
- RtpPacket packet(&extensions);
- const HdrMetadata kHdrMetadata = CreateTestHdrMetadata();
- EXPECT_TRUE(packet.SetExtension<HdrMetadataExtension>(kHdrMetadata));
- packet.SetPayloadSize(42);
+TEST(RtpPacketTest, CreateAndParseColorSpaceExtension) {
+ TestCreateAndParseColorSpaceExtension(/*with_hdr_metadata=*/true);
+}
- // Read packet with the extension.
- RtpPacketReceived parsed(&extensions);
- EXPECT_TRUE(parsed.Parse(packet.Buffer()));
- HdrMetadata parsed_hdr_metadata;
- EXPECT_TRUE(parsed.GetExtension<HdrMetadataExtension>(&parsed_hdr_metadata));
- EXPECT_EQ(kHdrMetadata, parsed_hdr_metadata);
+TEST(RtpPacketTest, CreateAndParseColorSpaceExtensionWithoutHdrMetadata) {
+ TestCreateAndParseColorSpaceExtension(/*with_hdr_metadata=*/false);
}
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_utility.cc b/modules/rtp_rtcp/source/rtp_utility.cc
index 80f0eab..44c671f 100644
--- a/modules/rtp_rtcp/source/rtp_utility.cc
+++ b/modules/rtp_rtcp/source/rtp_utility.cc
@@ -507,9 +507,9 @@
RTC_LOG(WARNING)
<< "RtpGenericFrameDescriptor unsupported by rtp header parser.";
break;
- case kRtpExtensionHdrMetadata:
+ case kRtpExtensionColorSpace:
RTC_LOG(WARNING)
- << "RtpExtensionHdrMetadata unsupported by rtp header parser.";
+ << "RtpExtensionColorSpace unsupported by rtp header parser.";
break;
case kRtpExtensionNone:
case kRtpExtensionNumberOfExtensions: {
diff --git a/test/fuzzers/rtp_packet_fuzzer.cc b/test/fuzzers/rtp_packet_fuzzer.cc
index 469fb36..f774c0c 100644
--- a/test/fuzzers/rtp_packet_fuzzer.cc
+++ b/test/fuzzers/rtp_packet_fuzzer.cc
@@ -121,9 +121,9 @@
packet.GetExtension<RtpGenericFrameDescriptorExtension>(&descriptor);
break;
}
- case kRtpExtensionHdrMetadata: {
- HdrMetadata hdr_metadata;
- packet.GetExtension<HdrMetadataExtension>(&hdr_metadata);
+ case kRtpExtensionColorSpace: {
+ ColorSpace color_space;
+ packet.GetExtension<ColorSpaceExtension>(&color_space);
break;
}
}
