From 4749e4e221d2947bba46959b2cce11596d1c8f2d Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Wed, 21 Nov 2018 10:18:18 +0100 Subject: [PATCH] Move HdrMetadata to ColorSpace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move HdrMetadata to ColorSpace as part of preparing for joint transmission of these two objects. Bug: webrtc:8651 Change-Id: Ie948011a2c0106d5967cb5ef3b9565217e798272 Reviewed-on: https://webrtc-review.googlesource.com/c/111481 Commit-Queue: Johannes Kron Reviewed-by: Niels Moller Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/master@{#25730} --- api/video/color_space.cc | 18 +++++++- api/video/color_space.h | 20 ++++++++ api/video/encoded_image.h | 14 +++--- api/video/video_frame.cc | 16 +++---- api/video/video_frame.h | 14 ++---- .../codecs/h264/test/h264_impl_unittest.cc | 2 +- .../codecs/vp9/test/vp9_impl_unittest.cc | 46 ++++++++++++------- modules/video_coding/codecs/vp9/vp9_impl.cc | 31 +++++++------ modules/video_coding/codecs/vp9/vp9_impl.h | 2 +- 9 files changed, 104 insertions(+), 59 deletions(-) diff --git a/api/video/color_space.cc b/api/video/color_space.cc index a8be5cd828..2f07e003a3 100644 --- a/api/video/color_space.cc +++ b/api/video/color_space.cc @@ -13,15 +13,27 @@ namespace webrtc { ColorSpace::ColorSpace() = default; +ColorSpace::ColorSpace(const ColorSpace& other) = default; +ColorSpace::ColorSpace(ColorSpace&& other) = default; +ColorSpace& ColorSpace::operator=(const ColorSpace& other) = default; ColorSpace::ColorSpace(PrimaryID primaries, TransferID transfer, MatrixID matrix, RangeID range) + : ColorSpace(primaries, transfer, matrix, range, nullptr) {} + +ColorSpace::ColorSpace(PrimaryID primaries, + TransferID transfer, + MatrixID matrix, + RangeID range, + const HdrMetadata* hdr_metadata) : primaries_(primaries), transfer_(transfer), matrix_(matrix), - range_(range) {} + range_(range), + hdr_metadata_(hdr_metadata ? absl::make_optional(*hdr_metadata) + : absl::nullopt) {} ColorSpace::PrimaryID ColorSpace::primaries() const { return primaries_; @@ -39,4 +51,8 @@ ColorSpace::RangeID ColorSpace::range() const { return range_; } +const HdrMetadata* ColorSpace::hdr_metadata() const { + return hdr_metadata_ ? &*hdr_metadata_ : nullptr; +} + } // namespace webrtc diff --git a/api/video/color_space.h b/api/video/color_space.h index cce3d92eb3..385734a450 100644 --- a/api/video/color_space.h +++ b/api/video/color_space.h @@ -13,6 +13,9 @@ #include +#include "absl/types/optional.h" +#include "api/video/hdr_metadata.h" + namespace webrtc { // This class represents color information as specified in T-REC H.273, @@ -101,21 +104,38 @@ class ColorSpace { }; ColorSpace(); + ColorSpace(const ColorSpace& other); + ColorSpace(ColorSpace&& other); + ColorSpace& operator=(const ColorSpace& other); ColorSpace(PrimaryID primaries, TransferID transfer, MatrixID matrix, RangeID full_range); + ColorSpace(PrimaryID primaries, + TransferID transfer, + MatrixID matrix, + RangeID range, + const HdrMetadata* hdr_metadata); + bool operator==(const ColorSpace& other) const { + return primaries_ == other.primaries() && transfer_ == other.transfer() && + matrix_ == other.matrix() && range_ == other.range() && + ((hdr_metadata_.has_value() && other.hdr_metadata() && + *hdr_metadata_ == *other.hdr_metadata()) || + (!hdr_metadata_.has_value() && other.hdr_metadata() == nullptr)); + } PrimaryID primaries() const; TransferID transfer() const; MatrixID matrix() const; RangeID range() const; + const HdrMetadata* hdr_metadata() const; private: PrimaryID primaries_ = PrimaryID::kInvalid; TransferID transfer_ = TransferID::kInvalid; MatrixID matrix_ = MatrixID::kInvalid; RangeID range_ = RangeID::kInvalid; + absl::optional hdr_metadata_; }; } // namespace webrtc diff --git a/api/video/encoded_image.h b/api/video/encoded_image.h index 4cf410b390..a7c719ce3a 100644 --- a/api/video/encoded_image.h +++ b/api/video/encoded_image.h @@ -14,7 +14,7 @@ #include #include "absl/types/optional.h" -#include "api/video/hdr_metadata.h" +#include "api/video/color_space.h" #include "api/video/video_bitrate_allocation.h" #include "api/video/video_codec_type.h" #include "api/video/video_content_type.h" @@ -59,12 +59,12 @@ class RTC_EXPORT EncodedImage { spatial_index_ = spatial_index; } - const webrtc::HdrMetadata* HdrMetadata() const { - return hdr_metadata_ ? &*hdr_metadata_ : nullptr; + const webrtc::ColorSpace* ColorSpace() const { + return color_space_ ? &*color_space_ : nullptr; } - void SetHdrMetadata(const webrtc::HdrMetadata* hdr_metadata) { - hdr_metadata_ = - hdr_metadata ? absl::make_optional(*hdr_metadata) : absl::nullopt; + void SetColorSpace(const webrtc::ColorSpace* color_space) { + color_space_ = + color_space ? absl::make_optional(*color_space) : absl::nullopt; } uint32_t _encodedWidth = 0; @@ -101,7 +101,7 @@ class RTC_EXPORT EncodedImage { private: uint32_t timestamp_rtp_ = 0; absl::optional spatial_index_; - absl::optional hdr_metadata_; + absl::optional color_space_; }; } // namespace webrtc diff --git a/api/video/video_frame.cc b/api/video/video_frame.cc index 23d3140773..eaae33b2b3 100644 --- a/api/video/video_frame.cc +++ b/api/video/video_frame.cc @@ -21,7 +21,7 @@ VideoFrame::Builder::~Builder() = default; VideoFrame VideoFrame::Builder::build() { return VideoFrame(video_frame_buffer_, timestamp_us_, timestamp_rtp_, - ntp_time_ms_, rotation_, color_space_, hdr_metadata_); + ntp_time_ms_, rotation_, color_space_); } VideoFrame::Builder& VideoFrame::Builder::set_video_frame_buffer( @@ -64,10 +64,10 @@ VideoFrame::Builder& VideoFrame::Builder::set_color_space( return *this; } -VideoFrame::Builder& VideoFrame::Builder::set_hdr_metadata( - const HdrMetadata* hdr_metadata) { - hdr_metadata_ = - hdr_metadata ? absl::make_optional(*hdr_metadata) : absl::nullopt; +VideoFrame::Builder& VideoFrame::Builder::set_color_space( + const ColorSpace* color_space) { + color_space_ = + color_space ? absl::make_optional(*color_space) : absl::nullopt; return *this; } @@ -97,15 +97,13 @@ VideoFrame::VideoFrame(const rtc::scoped_refptr& buffer, uint32_t timestamp_rtp, int64_t ntp_time_ms, VideoRotation rotation, - const absl::optional& color_space, - const absl::optional& hdr_metadata) + const absl::optional& color_space) : video_frame_buffer_(buffer), timestamp_rtp_(timestamp_rtp), ntp_time_ms_(ntp_time_ms), timestamp_us_(timestamp_us), rotation_(rotation), - color_space_(color_space), - hdr_metadata_(hdr_metadata) {} + color_space_(color_space) {} VideoFrame::~VideoFrame() = default; diff --git a/api/video/video_frame.h b/api/video/video_frame.h index 321eb97c0b..2c5d081ba6 100644 --- a/api/video/video_frame.h +++ b/api/video/video_frame.h @@ -40,7 +40,7 @@ class RTC_EXPORT VideoFrame { Builder& set_ntp_time_ms(int64_t ntp_time_ms); Builder& set_rotation(VideoRotation rotation); Builder& set_color_space(const ColorSpace& color_space); - Builder& set_hdr_metadata(const HdrMetadata* hdr_metadata); + Builder& set_color_space(const ColorSpace* color_space); private: rtc::scoped_refptr video_frame_buffer_; @@ -49,7 +49,6 @@ class RTC_EXPORT VideoFrame { int64_t ntp_time_ms_ = 0; VideoRotation rotation_ = kVideoRotation_0; absl::optional color_space_; - absl::optional hdr_metadata_; }; // To be deprecated. Migrate all use to Builder. @@ -116,11 +115,8 @@ class RTC_EXPORT VideoFrame { void set_rotation(VideoRotation rotation) { rotation_ = rotation; } // Get color space when available. - absl::optional color_space() const { return color_space_; } - - // Get HDR metadata when available. - const HdrMetadata* hdr_metadata() const { - return hdr_metadata_ ? &*hdr_metadata_ : nullptr; + const ColorSpace* color_space() const { + return color_space_ ? &*color_space_ : nullptr; } // Get render time in milliseconds. @@ -143,8 +139,7 @@ class RTC_EXPORT VideoFrame { uint32_t timestamp_rtp, int64_t ntp_time_ms, VideoRotation rotation, - const absl::optional& color_space, - const absl::optional& hdr_metadata); + const absl::optional& color_space); // An opaque reference counted handle that stores the pixel data. rtc::scoped_refptr video_frame_buffer_; @@ -153,7 +148,6 @@ class RTC_EXPORT VideoFrame { int64_t timestamp_us_; VideoRotation rotation_; absl::optional color_space_; - absl::optional hdr_metadata_; }; } // namespace webrtc diff --git a/modules/video_coding/codecs/h264/test/h264_impl_unittest.cc b/modules/video_coding/codecs/h264/test/h264_impl_unittest.cc index 14bb6bcc47..82c14b7fd5 100644 --- a/modules/video_coding/codecs/h264/test/h264_impl_unittest.cc +++ b/modules/video_coding/codecs/h264/test/h264_impl_unittest.cc @@ -56,7 +56,7 @@ TEST_F(TestH264Impl, MAYBE_EncodeDecode) { ASSERT_TRUE(decoded_frame); EXPECT_GT(I420PSNR(input_frame, decoded_frame.get()), 36); - const ColorSpace color_space = decoded_frame->color_space().value(); + const ColorSpace color_space = *decoded_frame->color_space(); EXPECT_EQ(ColorSpace::PrimaryID::kInvalid, color_space.primaries()); EXPECT_EQ(ColorSpace::TransferID::kInvalid, color_space.transfer()); EXPECT_EQ(ColorSpace::MatrixID::kInvalid, color_space.matrix()); diff --git a/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc b/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc index 8cc919ba42..85fa278a0d 100644 --- a/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc +++ b/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc @@ -105,7 +105,7 @@ class TestVp9Impl : public VideoCodecUnitTest { } } - HdrMetadata CreateTestHdrMetadata() { + HdrMetadata CreateTestHdrMetadata() const { // Random but reasonable HDR metadata. HdrMetadata hdr_metadata; hdr_metadata.mastering_metadata.luminance_max = 2000.0; @@ -122,6 +122,15 @@ class TestVp9Impl : public VideoCodecUnitTest { hdr_metadata.max_frame_average_light_level = 1789; return hdr_metadata; } + + ColorSpace CreateTestColorSpace() const { + HdrMetadata hdr_metadata = CreateTestHdrMetadata(); + ColorSpace color_space(ColorSpace::PrimaryID::kBT709, + ColorSpace::TransferID::kGAMMA22, + ColorSpace::MatrixID::kSMPTE2085, + ColorSpace::RangeID::kFull, &hdr_metadata); + return color_space; + } }; // Disabled on ios as flake, see https://crbug.com/webrtc/7057 @@ -146,7 +155,7 @@ TEST_F(TestVp9Impl, EncodeDecode) { ASSERT_TRUE(decoded_frame); EXPECT_GT(I420PSNR(input_frame, decoded_frame.get()), 36); - const ColorSpace color_space = decoded_frame->color_space().value(); + const ColorSpace color_space = *decoded_frame->color_space(); EXPECT_EQ(ColorSpace::PrimaryID::kInvalid, color_space.primaries()); EXPECT_EQ(ColorSpace::TransferID::kInvalid, color_space.transfer()); EXPECT_EQ(ColorSpace::MatrixID::kInvalid, color_space.matrix()); @@ -175,28 +184,28 @@ TEST_F(TestVp9Impl, EncodedRotationEqualsInputRotation) { EXPECT_EQ(kVideoRotation_90, encoded_frame.rotation_); } -TEST_F(TestVp9Impl, EncodedHdrMetadataEqualsInputHdrMetadata) { - // Video frame without HDR metadata. +TEST_F(TestVp9Impl, EncodedColorSpaceEqualsInputColorSpace) { + // Video frame without explicit color space information. VideoFrame* input_frame = NextInputFrame(); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->Encode(*input_frame, nullptr, nullptr)); EncodedImage encoded_frame; CodecSpecificInfo codec_specific_info; ASSERT_TRUE(WaitForEncodedFrame(&encoded_frame, &codec_specific_info)); - EXPECT_FALSE(encoded_frame.HdrMetadata()); + EXPECT_FALSE(encoded_frame.ColorSpace()); - // Video frame with HDR metadata. - HdrMetadata hdr_metadata = CreateTestHdrMetadata(); + // Video frame with explicit color space information. + ColorSpace color_space = CreateTestColorSpace(); VideoFrame input_frame_w_hdr = VideoFrame::Builder() .set_video_frame_buffer(input_frame->video_frame_buffer()) - .set_hdr_metadata(&hdr_metadata) + .set_color_space(&color_space) .build(); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->Encode(input_frame_w_hdr, nullptr, nullptr)); ASSERT_TRUE(WaitForEncodedFrame(&encoded_frame, &codec_specific_info)); - EXPECT_TRUE(encoded_frame.HdrMetadata()); - EXPECT_EQ(hdr_metadata, *encoded_frame.HdrMetadata()); + ASSERT_TRUE(encoded_frame.ColorSpace()); + EXPECT_EQ(*encoded_frame.ColorSpace(), color_space); } TEST_F(TestVp9Impl, DecodedHdrMetadataEqualsEncodedHdrMetadata) { @@ -206,24 +215,27 @@ TEST_F(TestVp9Impl, DecodedHdrMetadataEqualsEncodedHdrMetadata) { CodecSpecificInfo codec_specific_info; ASSERT_TRUE(WaitForEncodedFrame(&encoded_frame, &codec_specific_info)); - // Encoded frame without HDR metadata. + // Encoded frame without explicit color space information. EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, decoder_->Decode(encoded_frame, false, nullptr, 0)); std::unique_ptr decoded_frame; absl::optional decoded_qp; ASSERT_TRUE(WaitForDecodedFrame(&decoded_frame, &decoded_qp)); ASSERT_TRUE(decoded_frame); - EXPECT_FALSE(decoded_frame->hdr_metadata()); + // Color space present from encoded bitstream. + ASSERT_TRUE(decoded_frame->color_space()); + // No HDR metadata present. + EXPECT_FALSE(decoded_frame->color_space()->hdr_metadata()); - // Encoded frame with HDR metadata. - HdrMetadata hdr_metadata = CreateTestHdrMetadata(); - encoded_frame.SetHdrMetadata(&hdr_metadata); + // Encoded frame with explicit color space information. + ColorSpace color_space = CreateTestColorSpace(); + encoded_frame.SetColorSpace(&color_space); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, decoder_->Decode(encoded_frame, false, nullptr, 0)); ASSERT_TRUE(WaitForDecodedFrame(&decoded_frame, &decoded_qp)); ASSERT_TRUE(decoded_frame); - ASSERT_TRUE(decoded_frame->hdr_metadata()); - EXPECT_EQ(hdr_metadata, *decoded_frame->hdr_metadata()); + ASSERT_TRUE(decoded_frame->color_space()); + EXPECT_EQ(color_space, *decoded_frame->color_space()); } TEST_F(TestVp9Impl, DecodedQpEqualsEncodedQp) { diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index a19316cf28..427e446b3a 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -1199,7 +1199,7 @@ int VP9EncoderImpl::GetEncodedLayerFrame(const vpx_codec_cx_pkt* pkt) { int qp = -1; vpx_codec_control(encoder_, VP8E_GET_LAST_QUANTIZER, &qp); encoded_image_.qp_ = qp; - encoded_image_.SetHdrMetadata(input_image_->hdr_metadata()); + encoded_image_.SetColorSpace(input_image_->color_space()); return WEBRTC_VIDEO_CODEC_OK; } @@ -1339,7 +1339,7 @@ int VP9DecoderImpl::Decode(const EncodedImage& input_image, vpx_codec_control(decoder_, VPXD_GET_LAST_QUANTIZER, &qp); RTC_DCHECK_EQ(vpx_ret, VPX_CODEC_OK); int ret = ReturnFrame(img, input_image.Timestamp(), input_image.ntp_time_ms_, - qp, input_image.HdrMetadata()); + qp, input_image.ColorSpace()); if (ret != 0) { return ret; } @@ -1350,7 +1350,7 @@ int VP9DecoderImpl::ReturnFrame(const vpx_image_t* img, uint32_t timestamp, int64_t ntp_time_ms, int qp, - const HdrMetadata* hdr_metadata) { + const ColorSpace* explicit_color_space) { if (img == nullptr) { // Decoder OK and nullptr image => No show frame. return WEBRTC_VIDEO_CODEC_NO_OUTPUT; @@ -1392,16 +1392,21 @@ int VP9DecoderImpl::ReturnFrame(const vpx_image_t* img, return WEBRTC_VIDEO_CODEC_NO_OUTPUT; } - VideoFrame decoded_image = VideoFrame::Builder() - .set_video_frame_buffer(img_wrapped_buffer) - .set_timestamp_ms(0) - .set_timestamp_rtp(timestamp) - .set_ntp_time_ms(ntp_time_ms) - .set_rotation(webrtc::kVideoRotation_0) - .set_color_space(ExtractVP9ColorSpace( - img->cs, img->range, img->bit_depth)) - .set_hdr_metadata(hdr_metadata) - .build(); + auto builder = VideoFrame::Builder() + .set_video_frame_buffer(img_wrapped_buffer) + .set_timestamp_ms(0) + .set_timestamp_rtp(timestamp) + .set_ntp_time_ms(ntp_time_ms) + .set_rotation(webrtc::kVideoRotation_0); + if (explicit_color_space) { + builder.set_color_space(explicit_color_space); + } else { + builder.set_color_space( + ExtractVP9ColorSpace(img->cs, img->range, img->bit_depth)); + } + + VideoFrame decoded_image = builder.build(); + decode_complete_callback_->Decoded(decoded_image, absl::nullopt, qp); return WEBRTC_VIDEO_CODEC_OK; } diff --git a/modules/video_coding/codecs/vp9/vp9_impl.h b/modules/video_coding/codecs/vp9/vp9_impl.h index d26c53d21f..072e8408aa 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.h +++ b/modules/video_coding/codecs/vp9/vp9_impl.h @@ -170,7 +170,7 @@ class VP9DecoderImpl : public VP9Decoder { uint32_t timestamp, int64_t ntp_time_ms, int qp, - const HdrMetadata* hdr_metadata); + const ColorSpace* explicit_color_space); // Memory pool used to share buffers between libvpx and webrtc. Vp9FrameBufferPool frame_buffer_pool_;