Skip to content

Commit

Permalink
Keep running_ state in sync with active layers.
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
Tommi authored and WebRTC LUCI CQ committed Aug 5, 2021
1 parent 2ff9d49 commit 35b1cb4
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 5 deletions.
9 changes: 9 additions & 0 deletions call/video_send_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,15 @@ class VideoSendStream {
// 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
Expand Down
1 change: 1 addition & 0 deletions media/engine/fake_webrtc_call.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ class FakeVideoSendStream final
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()
Expand Down
22 changes: 20 additions & 2 deletions video/video_send_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,16 @@ void VideoSendStream::UpdateActiveSimulcastLayers(
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";
Expand All @@ -202,10 +208,17 @@ void VideoSendStream::UpdateActiveSimulcastLayers(
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() {
Expand Down Expand Up @@ -241,6 +254,11 @@ void VideoSendStream::Stop() {
}));
}

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_);
Expand Down
5 changes: 2 additions & 3 deletions video/video_send_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class VideoSendStream : public webrtc::VideoSendStream {
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;
Expand All @@ -94,8 +95,6 @@ class VideoSendStream : public webrtc::VideoSendStream {
private:
friend class test::VideoSendStreamPeer;

class ConstructionTask;

absl::optional<float> GetPacingFactorOverride() const;

RTC_NO_UNIQUE_ADDRESS SequenceChecker thread_checker_;
Expand All @@ -110,7 +109,7 @@ class VideoSendStream : public webrtc::VideoSendStream {
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;
};
Expand Down
4 changes: 4 additions & 0 deletions video/video_send_stream_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2251,6 +2251,8 @@ TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {

CreateVideoStreams();

EXPECT_FALSE(GetVideoSendStream()->started());

// Inject a frame, to force encoder creation.
GetVideoSendStream()->Start();
GetVideoSendStream()->SetSource(&forwarder,
Expand All @@ -2264,6 +2266,7 @@ TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {
// 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));

Expand All @@ -2280,6 +2283,7 @@ TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {
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));

Expand Down

0 comments on commit 35b1cb4

Please sign in to comment.