Keep `running_` state in sync with active layers.
When layers are activated/deactivated via UpdateActiveSimulcastLayers,
the flag wasn't being updated. This resulted in calls to Stop() getting
ignored after an implicit start via activating layers.
Bug: chromium:1234779
Change-Id: I4a72e624874526d27d3e97d6903112367c5e77fb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227700
Reviewed-by: Magnus Flodman <mflodman@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34654}
diff --git a/call/video_send_stream.h b/call/video_send_stream.h
index 65c2aba..487f304 100644
--- a/call/video_send_stream.h
+++ b/call/video_send_stream.h
@@ -218,6 +218,15 @@
// When a stream is stopped, it can't receive, process or deliver packets.
virtual void Stop() = 0;
+ // Accessor for determining if the stream is active. This is an inexpensive
+ // call that must be made on the same thread as `Start()` and `Stop()` methods
+ // are called on and will return `true` iff activity has been started either
+ // via `Start()` or `UpdateActiveSimulcastLayers()`. If activity is either
+ // stopped or is in the process of being stopped as a result of a call to
+ // either `Stop()` or `UpdateActiveSimulcastLayers()` where all layers were
+ // deactivated, the return value will be `false`.
+ virtual bool started() = 0;
+
// If the resource is overusing, the VideoSendStream will try to reduce
// resolution or frame rate until no resource is overusing.
// TODO(https://crbug.com/webrtc/11565): When the ResourceAdaptationProcessor
diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h
index 20e65d4..aeef954 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -196,6 +196,7 @@
const std::vector<bool> active_layers) override;
void Start() override;
void Stop() override;
+ bool started() override { return IsSending(); }
void AddAdaptationResource(
rtc::scoped_refptr<webrtc::Resource> resource) override;
std::vector<rtc::scoped_refptr<webrtc::Resource>> GetAdaptationResources()
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index 46bf0db..a947af9 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -186,10 +186,16 @@
const std::vector<bool> active_layers) {
RTC_DCHECK_RUN_ON(&thread_checker_);
+ // Keep our `running_` flag expected state in sync with active layers since
+ // the `send_stream_` will be implicitly stopped/started depending on the
+ // state of the layers.
+ bool running = false;
+
rtc::StringBuilder active_layers_string;
active_layers_string << "{";
for (size_t i = 0; i < active_layers.size(); ++i) {
if (active_layers[i]) {
+ running = true;
active_layers_string << "1";
} else {
active_layers_string << "0";
@@ -202,10 +208,17 @@
RTC_LOG(LS_INFO) << "UpdateActiveSimulcastLayers: "
<< active_layers_string.str();
- rtp_transport_queue_->PostTask(
- ToQueuedTask(transport_queue_safety_, [this, active_layers] {
+ rtp_transport_queue_->PostTask(ToQueuedTask(
+ transport_queue_safety_, [this, active_layers, was_running = running_] {
send_stream_.UpdateActiveSimulcastLayers(active_layers);
+ const bool running = rtp_video_sender_->IsActive();
+ if (was_running != running) {
+ running ? transport_queue_safety_->SetAlive()
+ : transport_queue_safety_->SetNotAlive();
+ }
}));
+
+ running_ = running;
}
void VideoSendStream::Start() {
@@ -241,6 +254,11 @@
}));
}
+bool VideoSendStream::started() {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
+ return running_;
+}
+
void VideoSendStream::AddAdaptationResource(
rtc::scoped_refptr<Resource> resource) {
RTC_DCHECK_RUN_ON(&thread_checker_);
diff --git a/video/video_send_stream.h b/video/video_send_stream.h
index b52871c..0d132dd 100644
--- a/video/video_send_stream.h
+++ b/video/video_send_stream.h
@@ -78,6 +78,7 @@
const std::vector<bool> active_layers) override;
void Start() override;
void Stop() override;
+ bool started() override;
void AddAdaptationResource(rtc::scoped_refptr<Resource> resource) override;
std::vector<rtc::scoped_refptr<Resource>> GetAdaptationResources() override;
@@ -94,8 +95,6 @@
private:
friend class test::VideoSendStreamPeer;
- class ConstructionTask;
-
absl::optional<float> GetPacingFactorOverride() const;
RTC_NO_UNIQUE_ADDRESS SequenceChecker thread_checker_;
@@ -110,7 +109,7 @@
const VideoEncoderConfig::ContentType content_type_;
std::unique_ptr<VideoStreamEncoderInterface> video_stream_encoder_;
EncoderRtcpFeedback encoder_feedback_;
- RtpVideoSenderInterface* const rtp_video_sender_ = nullptr;
+ RtpVideoSenderInterface* const rtp_video_sender_;
VideoSendStreamImpl send_stream_;
bool running_ RTC_GUARDED_BY(thread_checker_) = false;
};
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 5703f15..85dbbaf 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -2251,6 +2251,8 @@
CreateVideoStreams();
+ EXPECT_FALSE(GetVideoSendStream()->started());
+
// Inject a frame, to force encoder creation.
GetVideoSendStream()->Start();
GetVideoSendStream()->SetSource(&forwarder,
@@ -2264,6 +2266,7 @@
// which in turn updates the VideoEncoder's bitrate.
SendTask(RTC_FROM_HERE, task_queue(), [this]() {
GetVideoSendStream()->UpdateActiveSimulcastLayers({true, true});
+ EXPECT_TRUE(GetVideoSendStream()->started());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(true));
@@ -2280,6 +2283,7 @@
GetVideoEncoderConfig()->simulcast_layers[1].active = false;
SendTask(RTC_FROM_HERE, task_queue(), [this]() {
GetVideoSendStream()->UpdateActiveSimulcastLayers({false, false});
+ EXPECT_FALSE(GetVideoSendStream()->started());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(false));
