From 9d4961e5964dbaaacdf9376a3339af434a262a07 Mon Sep 17 00:00:00 2001
From: Per K <perkj@webrtc.org>
Date: Fri, 9 Feb 2024 13:01:35 +0100
Subject: [PATCH] Check IsRunning() in VideoSendStreamImpl::SignalEncoderActive
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ensure VideoSendStreamImpl does not register allocation on stray encoded
image if there is no active encodings.

Bug: chromium:41497180
Change-Id: I32afd7cc71f154dff240934e2be1745d8ead127c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338920
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41708}
---
 video/video_send_stream_impl.cc          |  3 +-
 video/video_send_stream_impl_unittest.cc | 51 ++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index 8c748dd81d..ec63b51816 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -522,6 +522,7 @@ VideoSendStreamImpl::~VideoSendStreamImpl() {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   RTC_LOG(LS_INFO) << "~VideoSendStreamImpl: " << config_.ToString();
   RTC_DCHECK(!started());
+  RTC_DCHECK(!IsRunning());
   transport_->DestroyRtpVideoSender(rtp_video_sender_);
 }
 
@@ -764,7 +765,7 @@ void VideoSendStreamImpl::OnVideoLayersAllocationUpdated(
 
 void VideoSendStreamImpl::SignalEncoderActive() {
   RTC_DCHECK_RUN_ON(&thread_checker_);
-  if (rtp_video_sender_->IsActive()) {
+  if (IsRunning()) {
     RTC_LOG(LS_INFO) << "SignalEncoderActive, Encoder is active.";
     bitrate_allocator_->AddObserver(this, GetAllocationConfig());
   }
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc
index fe5f999a39..f6b43cb958 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -70,6 +70,7 @@ namespace internal {
 namespace {
 using ::testing::_;
 using ::testing::AllOf;
+using ::testing::AnyNumber;
 using ::testing::Field;
 using ::testing::Invoke;
 using ::testing::Mock;
@@ -271,6 +272,56 @@ TEST_F(VideoSendStreamImplTest,
   vss_impl->Stop();
 }
 
+TEST_F(VideoSendStreamImplTest,
+       DoNotRegistersAsBitrateObserverOnStrayEncodedImage) {
+  auto vss_impl = CreateVideoSendStreamImpl(TestVideoEncoderConfig());
+
+  EncodedImage encoded_image;
+  CodecSpecificInfo codec_specific;
+  ON_CALL(rtp_video_sender_, OnEncodedImage)
+      .WillByDefault(Return(
+          EncodedImageCallback::Result(EncodedImageCallback::Result::OK)));
+
+  EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _))
+      .Times(AnyNumber());
+  vss_impl->Start();
+  time_controller_.AdvanceTime(TimeDelta::Zero());
+
+  // VideoSendStreamImpl gets an allocated bitrate.
+  const uint32_t kBitrateBps = 100000;
+  EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps())
+      .Times(1)
+      .WillOnce(Return(kBitrateBps));
+  static_cast<BitrateAllocatorObserver*>(vss_impl.get())
+      ->OnBitrateUpdated(CreateAllocation(kBitrateBps));
+  // A frame is encoded.
+  encoder_queue_->PostTask([&] {
+    static_cast<EncodedImageCallback*>(vss_impl.get())
+        ->OnEncodedImage(encoded_image, &codec_specific);
+  });
+
+  // Expect allocation to be removed if encoder stop producing frames.
+  EXPECT_CALL(bitrate_allocator_, RemoveObserver(vss_impl.get())).Times(1);
+  time_controller_.AdvanceTime(TimeDelta::Seconds(5));
+  Mock::VerifyAndClearExpectations(&bitrate_allocator_);
+
+  EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _)).Times(0);
+
+  VideoEncoderConfig no_active_encodings = TestVideoEncoderConfig();
+  no_active_encodings.simulcast_layers[0].active = false;
+  vss_impl->ReconfigureVideoEncoder(std::move(no_active_encodings));
+
+  // Expect that allocation in not resumed if a stray encoded image is received.
+  encoder_queue_->PostTask([&] {
+    static_cast<EncodedImageCallback*>(vss_impl.get())
+        ->OnEncodedImage(encoded_image, &codec_specific);
+  });
+
+  time_controller_.AdvanceTime(TimeDelta::Zero());
+
+  vss_impl->Stop();
+}
+
 TEST_F(VideoSendStreamImplTest, UpdatesObserverOnConfigurationChange) {
   const bool kSuspend = false;
   config_.suspend_below_min_bitrate = kSuspend;