From fb8641c4e61ec30c42a7705e87d6748b890b92e9 Mon Sep 17 00:00:00 2001
From: Per Kjellander <perkj@webrtc.org>
Date: Thu, 11 Aug 2022 09:25:00 +0200
Subject: [PATCH] Drop frames in RtpVideoSender::OnEncodedImage if stream
 disabled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Drops frames if the encoder has been configured with a new set of rtp
streams and a stray frame is returned from an encoder. This can happen with
hardware encoders that may deliver frames on a separate thread than were
they are configured.

This cl disable sending media on the RTP module a video layer is connected to and there by, old frames are dropped.

Bug: webrtc:1200, b/201798527
Change-Id: Id6bcfc3a846f6b8ed3b645cbbde571b819611a75
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271122
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37744}
---
 call/rtp_video_sender.cc          | 10 ++++++++-
 call/rtp_video_sender_unittest.cc | 35 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 1f55eb8a0d..a7e2f692ab 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -511,7 +511,7 @@ void RtpVideoSender::SetActiveModulesLocked(
     }
 
     RtpRtcpInterface& rtp_module = *rtp_streams_[i].rtp_rtcp;
-    const bool was_active = rtp_module.SendingMedia();
+    const bool was_active = rtp_module.Sending();
     const bool should_be_active = active_modules[i];
 
     // Sends a kRtcpByeCode when going from true to false.
@@ -669,6 +669,14 @@ void RtpVideoSender::OnVideoLayersAllocationUpdated(
       stream_allocation.rtp_stream_index = i;
       rtp_streams_[i].sender_video->SetVideoLayersAllocation(
           std::move(stream_allocation));
+      // Only send video frames on the rtp module if the encoder is configured
+      // to send. This is to prevent stray frames to be sent after an encoder
+      // has been reconfigured.
+      rtp_streams_[i].rtp_rtcp->SetSendingMediaStatus(
+          absl::c_any_of(allocation.active_spatial_layers,
+                         [&i](const VideoLayersAllocation::SpatialLayer layer) {
+                           return layer.rtp_stream_index == static_cast<int>(i);
+                         }));
     }
   }
 }
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index fef0495ee3..ee3e8458dd 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -330,6 +330,41 @@ TEST(RtpVideoSenderTest, SendSimulcastSetActiveModules) {
             test.router()->OnEncodedImage(encoded_image_1, &codec_info).error);
 }
 
+TEST(
+    RtpVideoSenderTest,
+    DiscardsHigherSpatialVideoFramesAfterLayerDisabledInVideoLayersAllocation) {
+  constexpr uint8_t kPayload = 'a';
+  EncodedImage encoded_image_1;
+  encoded_image_1.SetTimestamp(1);
+  encoded_image_1.capture_time_ms_ = 2;
+  encoded_image_1._frameType = VideoFrameType::kVideoFrameKey;
+  encoded_image_1.SetEncodedData(EncodedImageBuffer::Create(&kPayload, 1));
+  EncodedImage encoded_image_2(encoded_image_1);
+  encoded_image_2.SetSpatialIndex(1);
+  CodecSpecificInfo codec_info;
+  codec_info.codecType = kVideoCodecVP8;
+  RtpVideoSenderTestFixture test({kSsrc1, kSsrc2}, {kRtxSsrc1, kRtxSsrc2},
+                                 kPayloadType, {});
+  test.SetActiveModules({true, true});
+  // A layer is sent on both rtp streams.
+  test.router()->OnVideoLayersAllocationUpdated(
+      {.active_spatial_layers = {{.rtp_stream_index = 0},
+                                 {.rtp_stream_index = 1}}});
+
+  EXPECT_EQ(EncodedImageCallback::Result::OK,
+            test.router()->OnEncodedImage(encoded_image_1, &codec_info).error);
+  EXPECT_EQ(EncodedImageCallback::Result::OK,
+            test.router()->OnEncodedImage(encoded_image_2, &codec_info).error);
+
+  // Only rtp stream index 0 is configured to send a stream.
+  test.router()->OnVideoLayersAllocationUpdated(
+      {.active_spatial_layers = {{.rtp_stream_index = 0}}});
+  EXPECT_EQ(EncodedImageCallback::Result::OK,
+            test.router()->OnEncodedImage(encoded_image_1, &codec_info).error);
+  EXPECT_NE(EncodedImageCallback::Result::OK,
+            test.router()->OnEncodedImage(encoded_image_2, &codec_info).error);
+}
+
 TEST(RtpVideoSenderTest, CreateWithNoPreviousStates) {
   RtpVideoSenderTestFixture test({kSsrc1, kSsrc2}, {kRtxSsrc1, kRtxSsrc2},
                                  kPayloadType, {});