From 9973fa88aeaf0cbb43803a920c08ea95cc21b0a7 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Wed, 7 Nov 2018 14:39:26 +0100 Subject: [PATCH] Pass HdrMetadata between VideoFrame and EncodedImage for VP9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:8651 Change-Id: Ie4d7ee19bead84eda7788076662c4066edc3f024 Reviewed-on: https://webrtc-review.googlesource.com/c/109583 Reviewed-by: Erik Språng Reviewed-by: Karl Wiberg Reviewed-by: Niels Moller Commit-Queue: Johannes Kron Cr-Commit-Position: refs/heads/master@{#25581} --- api/video/encoded_image.h | 18 +++-- api/video/hdr_metadata.cc | 14 ---- api/video/hdr_metadata.h | 9 --- api/video/video_frame.cc | 5 +- api/video/video_frame.h | 6 +- .../codecs/vp9/test/vp9_impl_unittest.cc | 69 +++++++++++++++++++ modules/video_coding/codecs/vp9/vp9_impl.cc | 9 ++- modules/video_coding/codecs/vp9/vp9_impl.h | 3 +- 8 files changed, 96 insertions(+), 37 deletions(-) diff --git a/api/video/encoded_image.h b/api/video/encoded_image.h index 5c4a82d5fe..b04e1ca524 100644 --- a/api/video/encoded_image.h +++ b/api/video/encoded_image.h @@ -14,6 +14,7 @@ #include #include "absl/types/optional.h" +#include "api/video/hdr_metadata.h" #include "api/video/video_bitrate_allocation.h" #include "api/video/video_content_type.h" #include "api/video/video_rotation.h" @@ -49,14 +50,20 @@ class RTC_EXPORT EncodedImage { void SetEncodeTime(int64_t encode_start_ms, int64_t encode_finish_ms); absl::optional SpatialIndex() const { - if (spatial_index_ < 0) - return absl::nullopt; return spatial_index_; } void SetSpatialIndex(absl::optional spatial_index) { RTC_DCHECK_GE(spatial_index.value_or(0), 0); RTC_DCHECK_LT(spatial_index.value_or(0), kMaxSpatialLayers); - spatial_index_ = spatial_index.value_or(-1); + spatial_index_ = spatial_index; + } + + const webrtc::HdrMetadata* HdrMetadata() const { + return hdr_metadata_ ? &*hdr_metadata_ : nullptr; + } + void SetHdrMetadata(const webrtc::HdrMetadata* hdr_metadata) { + hdr_metadata_ = + hdr_metadata ? absl::make_optional(*hdr_metadata) : absl::nullopt; } uint32_t _encodedWidth = 0; @@ -92,9 +99,8 @@ class RTC_EXPORT EncodedImage { private: uint32_t timestamp_rtp_ = 0; - // -1 means not set. Use a plain int rather than optional, to keep this class - // copyable with memcpy. - int spatial_index_ = -1; + absl::optional spatial_index_; + absl::optional hdr_metadata_; }; } // namespace webrtc diff --git a/api/video/hdr_metadata.cc b/api/video/hdr_metadata.cc index bfe54ce2e5..e2a669c98a 100644 --- a/api/video/hdr_metadata.cc +++ b/api/video/hdr_metadata.cc @@ -13,23 +13,9 @@ namespace webrtc { HdrMasteringMetadata::Chromaticity::Chromaticity() = default; -HdrMasteringMetadata::Chromaticity::Chromaticity(const Chromaticity& rhs) = - default; -HdrMasteringMetadata::Chromaticity::Chromaticity(Chromaticity&& rhs) = default; -HdrMasteringMetadata::Chromaticity& HdrMasteringMetadata::Chromaticity:: -operator=(const Chromaticity& rhs) = default; HdrMasteringMetadata::HdrMasteringMetadata() = default; -HdrMasteringMetadata::HdrMasteringMetadata(const HdrMasteringMetadata& rhs) = - default; -HdrMasteringMetadata::HdrMasteringMetadata(HdrMasteringMetadata&& rhs) = - default; -HdrMasteringMetadata& HdrMasteringMetadata::operator=( - const HdrMasteringMetadata& rhs) = default; HdrMetadata::HdrMetadata() = default; -HdrMetadata::HdrMetadata(const HdrMetadata& rhs) = default; -HdrMetadata::HdrMetadata(HdrMetadata&& rhs) = default; -HdrMetadata& HdrMetadata::operator=(const HdrMetadata& rhs) = default; } // namespace webrtc diff --git a/api/video/hdr_metadata.h b/api/video/hdr_metadata.h index be0c17373b..676a900c99 100644 --- a/api/video/hdr_metadata.h +++ b/api/video/hdr_metadata.h @@ -30,9 +30,6 @@ struct HdrMasteringMetadata { } Chromaticity(); - Chromaticity(const Chromaticity& rhs); - Chromaticity(Chromaticity&& rhs); - Chromaticity& operator=(const Chromaticity& rhs); }; // The nominal primaries of the mastering display. @@ -54,9 +51,6 @@ struct HdrMasteringMetadata { float luminance_min = 0.0f; HdrMasteringMetadata(); - HdrMasteringMetadata(const HdrMasteringMetadata& rhs); - HdrMasteringMetadata(HdrMasteringMetadata&& rhs); - HdrMasteringMetadata& operator=(const HdrMasteringMetadata& rhs); bool operator==(const HdrMasteringMetadata& rhs) const { return ((primary_r == rhs.primary_r) && (primary_g == rhs.primary_g) && @@ -79,9 +73,6 @@ struct HdrMetadata { uint32_t max_frame_average_light_level = 0; HdrMetadata(); - HdrMetadata(const HdrMetadata& rhs); - HdrMetadata(HdrMetadata&& rhs); - HdrMetadata& operator=(const HdrMetadata& rhs); bool operator==(const HdrMetadata& rhs) const { return ( diff --git a/api/video/video_frame.cc b/api/video/video_frame.cc index 12da43f69f..23d3140773 100644 --- a/api/video/video_frame.cc +++ b/api/video/video_frame.cc @@ -65,8 +65,9 @@ VideoFrame::Builder& VideoFrame::Builder::set_color_space( } VideoFrame::Builder& VideoFrame::Builder::set_hdr_metadata( - const HdrMetadata& hdr_metadata) { - hdr_metadata_ = hdr_metadata; + const HdrMetadata* hdr_metadata) { + hdr_metadata_ = + hdr_metadata ? absl::make_optional(*hdr_metadata) : absl::nullopt; return *this; } diff --git a/api/video/video_frame.h b/api/video/video_frame.h index 58362b0bd8..321eb97c0b 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_hdr_metadata(const HdrMetadata* hdr_metadata); private: rtc::scoped_refptr video_frame_buffer_; @@ -119,7 +119,9 @@ class RTC_EXPORT VideoFrame { absl::optional color_space() const { return color_space_; } // Get HDR metadata when available. - absl::optional hdr_metadata() const { return hdr_metadata_; } + const HdrMetadata* hdr_metadata() const { + return hdr_metadata_ ? &*hdr_metadata_ : nullptr; + } // Get render time in milliseconds. // TODO(nisse): Deprecated. Migrate all users to timestamp_us(). 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 c4703c0ded..8cc919ba42 100644 --- a/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc +++ b/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc @@ -104,6 +104,24 @@ class TestVp9Impl : public VideoCodecUnitTest { codec_settings_.spatialLayers[i] = layers[i]; } } + + HdrMetadata CreateTestHdrMetadata() { + // Random but reasonable HDR metadata. + HdrMetadata hdr_metadata; + hdr_metadata.mastering_metadata.luminance_max = 2000.0; + hdr_metadata.mastering_metadata.luminance_min = 2.0001; + hdr_metadata.mastering_metadata.primary_r.x = 0.30; + hdr_metadata.mastering_metadata.primary_r.y = 0.40; + hdr_metadata.mastering_metadata.primary_g.x = 0.32; + hdr_metadata.mastering_metadata.primary_g.y = 0.46; + hdr_metadata.mastering_metadata.primary_b.x = 0.34; + hdr_metadata.mastering_metadata.primary_b.y = 0.49; + hdr_metadata.mastering_metadata.white_point.x = 0.41; + hdr_metadata.mastering_metadata.white_point.y = 0.48; + hdr_metadata.max_content_light_level = 2345; + hdr_metadata.max_frame_average_light_level = 1789; + return hdr_metadata; + } }; // Disabled on ios as flake, see https://crbug.com/webrtc/7057 @@ -157,6 +175,57 @@ TEST_F(TestVp9Impl, EncodedRotationEqualsInputRotation) { EXPECT_EQ(kVideoRotation_90, encoded_frame.rotation_); } +TEST_F(TestVp9Impl, EncodedHdrMetadataEqualsInputHdrMetadata) { + // Video frame without HDR metadata. + 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()); + + // Video frame with HDR metadata. + HdrMetadata hdr_metadata = CreateTestHdrMetadata(); + VideoFrame input_frame_w_hdr = + VideoFrame::Builder() + .set_video_frame_buffer(input_frame->video_frame_buffer()) + .set_hdr_metadata(&hdr_metadata) + .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()); +} + +TEST_F(TestVp9Impl, DecodedHdrMetadataEqualsEncodedHdrMetadata) { + EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, + encoder_->Encode(*NextInputFrame(), nullptr, nullptr)); + EncodedImage encoded_frame; + CodecSpecificInfo codec_specific_info; + ASSERT_TRUE(WaitForEncodedFrame(&encoded_frame, &codec_specific_info)); + + // Encoded frame without HDR metadata. + 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()); + + // Encoded frame with HDR metadata. + HdrMetadata hdr_metadata = CreateTestHdrMetadata(); + encoded_frame.SetHdrMetadata(&hdr_metadata); + 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()); +} + TEST_F(TestVp9Impl, DecodedQpEqualsEncodedQp) { EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->Encode(*NextInputFrame(), nullptr, nullptr)); diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index 996c733f71..7dea25fe23 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -1214,6 +1214,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()); return WEBRTC_VIDEO_CODEC_OK; } @@ -1356,8 +1357,8 @@ int VP9DecoderImpl::Decode(const EncodedImage& input_image, vpx_codec_err_t vpx_ret = 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); + int ret = ReturnFrame(img, input_image.Timestamp(), input_image.ntp_time_ms_, + qp, input_image.HdrMetadata()); if (ret != 0) { return ret; } @@ -1367,7 +1368,8 @@ int VP9DecoderImpl::Decode(const EncodedImage& input_image, int VP9DecoderImpl::ReturnFrame(const vpx_image_t* img, uint32_t timestamp, int64_t ntp_time_ms, - int qp) { + int qp, + const HdrMetadata* hdr_metadata) { if (img == nullptr) { // Decoder OK and nullptr image => No show frame. return WEBRTC_VIDEO_CODEC_NO_OUTPUT; @@ -1417,6 +1419,7 @@ int VP9DecoderImpl::ReturnFrame(const vpx_image_t* img, .set_rotation(webrtc::kVideoRotation_0) .set_color_space(ExtractVP9ColorSpace( img->cs, img->range, img->bit_depth)) + .set_hdr_metadata(hdr_metadata) .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 e647f3f074..a968e4cc03 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.h +++ b/modules/video_coding/codecs/vp9/vp9_impl.h @@ -167,7 +167,8 @@ class VP9DecoderImpl : public VP9Decoder { int ReturnFrame(const vpx_image_t* img, uint32_t timestamp, int64_t ntp_time_ms, - int qp); + int qp, + const HdrMetadata* hdr_metadata); // Memory pool used to share buffers between libvpx and webrtc. Vp9FrameBufferPool frame_buffer_pool_;