commit 37dfddd5e8ab105faf8301b98d8ffeb68290a94e
author Danil Chapovalov <danilchap@webrtc.org> Wed Jan 27 18:24:23 2021
committer Commit Bot <commit-bot@chromium.org> Thu Jan 28 12:11:50 2021
tree 6d02363c6968ea9d5fa68a12b241c91590602b73
parent 075fd4b7f9b69eab1670c7f35becf58da2a96061

Avoid treating VP8 key frame in simulcast as delta frame

Bug: None
Change-Id: Ief3c759571f02af6cd9517acb2851861e190ceaf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/203891
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33088}

call/rtp_video_sender.cc

diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index e8d5db9..8bba2be 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -314,14 +314,24 @@
     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.

call/rtp_video_sender_unittest.cc

diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index b738c21..25e0836 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -825,6 +825,64 @@
       sent_packets.back().HasExtension<RtpDependencyDescriptorExtension>());
 }
 
+TEST(RtpVideoSenderTest,
+     SupportsStoppingUsingDependencyDescriptorForVp8Simulcast) {
+  RtpVideoSenderTestFixture test({kSsrc1, kSsrc2}, {}, kPayloadType, {});
+  test.router()->SetActive(true);
+
+  RtpHeaderExtensionMap extensions;
+  extensions.Register<RtpDependencyDescriptorExtension>(
+      kDependencyDescriptorExtensionId);
+  std::vector<RtpPacket> 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<RtpDependencyDescriptorExtension>());
+
+  // 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<RtpDependencyDescriptorExtension>());
+}
+
 TEST(RtpVideoSenderTest, CanSetZeroBitrate) {
   RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {});
   test.router()->OnBitrateUpdated(CreateBitrateAllocationUpdate(0),