From 37dfddd5e8ab105faf8301b98d8ffeb68290a94e Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 27 Jan 2021 19:24:23 +0100 Subject: [PATCH] Avoid treating VP8 key frame in simulcast as delta frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: None Change-Id: Ief3c759571f02af6cd9517acb2851861e190ceaf Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/203891 Reviewed-by: Åsa Persson Reviewed-by: Erik Språng Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#33088} --- call/rtp_video_sender.cc | 26 +++++++++----- call/rtp_video_sender_unittest.cc | 58 +++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index e8d5db9e46..8bba2be2f9 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -314,14 +314,24 @@ bool IsFirstFrameOfACodedVideoSequence( return false; } - if (codec_specific_info != nullptr && - codec_specific_info->generic_frame_info.has_value()) { - // This function is used before - // `codec_specific_info->generic_frame_info->frame_diffs` are calculated, so - // need to use more complicated way to check for presence of dependencies. - return absl::c_none_of( - codec_specific_info->generic_frame_info->encoder_buffers, - [](const CodecBufferUsage& buffer) { return buffer.referenced; }); + if (codec_specific_info != nullptr) { + if (codec_specific_info->generic_frame_info.has_value()) { + // This function is used before + // `codec_specific_info->generic_frame_info->frame_diffs` are calculated, + // so need to use a more complicated way to check for presence of the + // dependencies. + return absl::c_none_of( + codec_specific_info->generic_frame_info->encoder_buffers, + [](const CodecBufferUsage& buffer) { return buffer.referenced; }); + } + + if (codec_specific_info->codecType == VideoCodecType::kVideoCodecVP8 || + codec_specific_info->codecType == VideoCodecType::kVideoCodecH264 || + codec_specific_info->codecType == VideoCodecType::kVideoCodecGeneric) { + // These codecs do not support intra picture dependencies, so a frame + // marked as a key frame should be a key frame. + return true; + } } // Without depenedencies described in generic format do an educated guess. diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index b738c21447..25e08368c8 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -825,6 +825,64 @@ TEST(RtpVideoSenderTest, SupportsStoppingUsingDependencyDescriptor) { sent_packets.back().HasExtension()); } +TEST(RtpVideoSenderTest, + SupportsStoppingUsingDependencyDescriptorForVp8Simulcast) { + RtpVideoSenderTestFixture test({kSsrc1, kSsrc2}, {}, kPayloadType, {}); + test.router()->SetActive(true); + + RtpHeaderExtensionMap extensions; + extensions.Register( + kDependencyDescriptorExtensionId); + std::vector sent_packets; + ON_CALL(test.transport(), SendRtp) + .WillByDefault([&](const uint8_t* packet, size_t length, + const PacketOptions& options) { + sent_packets.emplace_back(&extensions); + EXPECT_TRUE(sent_packets.back().Parse(packet, length)); + return true; + }); + + const uint8_t kPayload[1] = {'a'}; + EncodedImage encoded_image; + encoded_image.SetTimestamp(1); + encoded_image.capture_time_ms_ = 2; + encoded_image.SetEncodedData( + EncodedImageBuffer::Create(kPayload, sizeof(kPayload))); + // VP8 simulcast uses spatial index to communicate simulcast stream. + encoded_image.SetSpatialIndex(1); + + CodecSpecificInfo codec_specific; + codec_specific.codecType = VideoCodecType::kVideoCodecVP8; + codec_specific.template_structure.emplace(); + codec_specific.template_structure->num_decode_targets = 1; + codec_specific.template_structure->templates = { + FrameDependencyTemplate().T(0).Dtis("S")}; + + // Send two tiny images, mapping to single RTP packets. + // Send in a key frame. + encoded_image._frameType = VideoFrameType::kVideoFrameKey; + codec_specific.generic_frame_info = + GenericFrameInfo::Builder().T(0).Dtis("S").Build(); + codec_specific.generic_frame_info->encoder_buffers = {{0, false, true}}; + EXPECT_EQ(test.router()->OnEncodedImage(encoded_image, &codec_specific).error, + EncodedImageCallback::Result::OK); + test.AdvanceTime(TimeDelta::Millis(33)); + ASSERT_THAT(sent_packets, SizeIs(1)); + EXPECT_TRUE( + sent_packets.back().HasExtension()); + + // Send in a new key frame without the support for the dependency descriptor. + encoded_image._frameType = VideoFrameType::kVideoFrameKey; + codec_specific.template_structure = absl::nullopt; + codec_specific.generic_frame_info = absl::nullopt; + EXPECT_EQ(test.router()->OnEncodedImage(encoded_image, &codec_specific).error, + EncodedImageCallback::Result::OK); + test.AdvanceTime(TimeDelta::Millis(33)); + ASSERT_THAT(sent_packets, SizeIs(2)); + EXPECT_FALSE( + sent_packets.back().HasExtension()); +} + TEST(RtpVideoSenderTest, CanSetZeroBitrate) { RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {}); test.router()->OnBitrateUpdated(CreateBitrateAllocationUpdate(0),