From d51ec589d6d9ef8b9c9c7ca54e1c877b9b1da1dd Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Mon, 15 Apr 2019 13:32:41 +0200 Subject: [PATCH] Parse color space only in last packet of key frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Color space should only be transmitted in the last packet of a key frame, therefore, neglect it otherwise so that |last_color_space_| is not reset by mistake. Bug: webrtc:10543 Change-Id: I374f9e52739292b18f510cc2941666fe6ba6951e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132553 Reviewed-by: Erik Språng Commit-Queue: Johannes Kron Cr-Commit-Position: refs/heads/master@{#27717} --- video/BUILD.gn | 1 + video/rtp_video_stream_receiver.cc | 23 ++-- video/rtp_video_stream_receiver_unittest.cc | 126 ++++++++++++++++++++ 3 files changed, 141 insertions(+), 9 deletions(-) diff --git a/video/BUILD.gn b/video/BUILD.gn index 584a9e7d49..aca186656e 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -524,6 +524,7 @@ if (rtc_include_tests) { "../api/video:video_bitrate_allocation", "../api/video:video_frame", "../api/video:video_frame_i420", + "../api/video:video_frame_type", "../api/video_codecs:video_codecs_api", "../api/video_codecs:vp8_temporal_layers_factory", "../call:call_interfaces", diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index a281d5ecb6..f9a31d29a5 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -550,15 +550,20 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { packet.GetExtension(&video_header.playout_delay); packet.GetExtension(&video_header.frame_marking); - video_header.color_space = packet.GetExtension(); - if (video_header.color_space || - parsed_payload.frame_type == VideoFrameType::kVideoFrameKey) { - // Store color space since it's only transmitted when changed or for key - // frames. Color space will be cleared if a key frame is transmitted without - // color space information. - last_color_space_ = video_header.color_space; - } else if (last_color_space_) { - video_header.color_space = last_color_space_; + // Color space should only be transmitted in the last packet of a frame, + // therefore, neglect it otherwise so that last_color_space_ is not reset by + // mistake. + if (video_header.is_last_packet_in_frame) { + video_header.color_space = packet.GetExtension(); + if (video_header.color_space || + parsed_payload.frame_type == VideoFrameType::kVideoFrameKey) { + // Store color space since it's only transmitted when changed or for key + // frames. Color space will be cleared if a key frame is transmitted + // without color space information. + last_color_space_ = video_header.color_space; + } else if (last_color_space_) { + video_header.color_space = last_color_space_; + } } absl::optional generic_descriptor_wire; diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index 5fb1a8c58b..75cdd93930 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -12,12 +12,17 @@ #include "test/gtest.h" #include "absl/memory/memory.h" +#include "api/video/video_codec_type.h" +#include "api/video/video_frame_type.h" #include "common_video/h264/h264_common.h" #include "media/base/media_constants.h" #include "modules/pacing/packet_router.h" +#include "modules/rtp_rtcp/source/rtp_format.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h" +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" +#include "modules/rtp_rtcp/source/rtp_packet_to_send.h" #include "modules/utility/include/process_thread.h" #include "modules/video_coding/frame_object.h" #include "modules/video_coding/include/video_coding_defines.h" @@ -88,6 +93,9 @@ class MockOnCompleteFrameCallback } DoOnCompleteFrame(frame.get()); } + + void ClearExpectedBitstream() { buffer_.Clear(); } + void AppendExpectedBitstream(const uint8_t data[], size_t size_in_bytes) { // TODO(Johan): Let rtc::ByteBuffer handle uint8_t* instead of char*. buffer_.WriteBytes(reinterpret_cast(data), size_in_bytes); @@ -201,6 +209,124 @@ class RtpVideoStreamReceiverTest : public ::testing::Test { std::unique_ptr rtp_video_stream_receiver_; }; +TEST_F(RtpVideoStreamReceiverTest, CacheColorSpaceFromLastPacketOfKeyframe) { + // Test that color space is cached from the last packet of a key frame and + // that it's not reset by padding packets without color space. + constexpr int kPayloadType = 99; + const ColorSpace kColorSpace( + ColorSpace::PrimaryID::kFILM, ColorSpace::TransferID::kBT2020_12, + ColorSpace::MatrixID::kBT2020_NCL, ColorSpace::RangeID::kFull); + const std::vector kKeyFramePayload = {0, 1, 2, 3, 4, 5, + 6, 7, 8, 9, 10}; + const std::vector kDeltaFramePayload = {0, 1, 2, 3, 4}; + + // Anonymous helper class that generates received packets. + class { + public: + void SetPayload(const std::vector& payload, + VideoFrameType video_frame_type) { + video_frame_type_ = video_frame_type; + RtpPacketizer::PayloadSizeLimits pay_load_size_limits; + // Reduce max payload length to make sure the key frame generates two + // packets. + pay_load_size_limits.max_payload_len = 8; + RTPVideoHeader rtp_video_header; + RTPVideoHeaderVP9 rtp_video_header_vp9; + rtp_video_header_vp9.InitRTPVideoHeaderVP9(); + rtp_video_header_vp9.inter_pic_predicted = + (video_frame_type == VideoFrameType::kVideoFrameDelta); + rtp_video_header.video_type_header = rtp_video_header_vp9; + rtp_packetizer_ = RtpPacketizer::Create( + kVideoCodecVP9, rtc::MakeArrayView(payload.data(), payload.size()), + pay_load_size_limits, rtp_video_header, video_frame_type, nullptr); + } + + size_t NumPackets() { return rtp_packetizer_->NumPackets(); } + void SetColorSpace(const ColorSpace& color_space) { + color_space_ = color_space; + } + + RtpPacketReceived NextPacket() { + RtpHeaderExtensionMap extension_map; + extension_map.Register(1); + RtpPacketToSend packet_to_send(&extension_map); + packet_to_send.SetSequenceNumber(sequence_number_++); + packet_to_send.SetSsrc(kSsrc); + packet_to_send.SetPayloadType(kPayloadType); + bool include_color_space = + (rtp_packetizer_->NumPackets() == 1u && + video_frame_type_ == VideoFrameType::kVideoFrameKey); + if (include_color_space) { + EXPECT_TRUE( + packet_to_send.SetExtension(color_space_)); + } + rtp_packetizer_->NextPacket(&packet_to_send); + + RtpPacketReceived received_packet(&extension_map); + received_packet.Parse(packet_to_send.data(), packet_to_send.size()); + return received_packet; + } + + private: + uint16_t sequence_number_ = 0; + VideoFrameType video_frame_type_; + ColorSpace color_space_; + std::unique_ptr rtp_packetizer_; + } received_packet_generator; + received_packet_generator.SetColorSpace(kColorSpace); + + // Prepare the receiver for VP9. + VideoCodec codec; + codec.plType = kPayloadType; + codec.codecType = kVideoCodecVP9; + std::map codec_params; + rtp_video_stream_receiver_->AddReceiveCodec(codec, codec_params); + + // Generate key frame packets. + received_packet_generator.SetPayload(kKeyFramePayload, + VideoFrameType::kVideoFrameKey); + EXPECT_EQ(received_packet_generator.NumPackets(), 2u); + RtpPacketReceived key_frame_packet1 = received_packet_generator.NextPacket(); + RtpPacketReceived key_frame_packet2 = received_packet_generator.NextPacket(); + + // Generate delta frame packet. + received_packet_generator.SetPayload(kDeltaFramePayload, + VideoFrameType::kVideoFrameDelta); + EXPECT_EQ(received_packet_generator.NumPackets(), 1u); + RtpPacketReceived delta_frame_packet = received_packet_generator.NextPacket(); + + rtp_video_stream_receiver_->StartReceive(); + mock_on_complete_frame_callback_.AppendExpectedBitstream( + kKeyFramePayload.data(), kKeyFramePayload.size()); + + // Send the key frame and expect a callback with color space information. + EXPECT_FALSE(key_frame_packet1.GetExtension()); + EXPECT_TRUE(key_frame_packet2.GetExtension()); + rtp_video_stream_receiver_->OnRtpPacket(key_frame_packet1); + EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)) + .WillOnce(Invoke([kColorSpace](video_coding::EncodedFrame* frame) { + ASSERT_TRUE(frame->EncodedImage().ColorSpace()); + EXPECT_EQ(*frame->EncodedImage().ColorSpace(), kColorSpace); + })); + rtp_video_stream_receiver_->OnRtpPacket(key_frame_packet2); + // Resend the first key frame packet to simulate padding for example. + rtp_video_stream_receiver_->OnRtpPacket(key_frame_packet1); + + mock_on_complete_frame_callback_.ClearExpectedBitstream(); + mock_on_complete_frame_callback_.AppendExpectedBitstream( + kDeltaFramePayload.data(), kDeltaFramePayload.size()); + + // Expect delta frame to have color space set even though color space not + // included in the RTP packet. + EXPECT_FALSE(delta_frame_packet.GetExtension()); + EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)) + .WillOnce(Invoke([kColorSpace](video_coding::EncodedFrame* frame) { + ASSERT_TRUE(frame->EncodedImage().ColorSpace()); + EXPECT_EQ(*frame->EncodedImage().ColorSpace(), kColorSpace); + })); + rtp_video_stream_receiver_->OnRtpPacket(delta_frame_packet); +} + TEST_F(RtpVideoStreamReceiverTest, GenericKeyFrame) { RTPHeader rtp_header; RTPVideoHeader video_header;