zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Keep transport_queue_safety_ alive until stopped permanently.

Browse Source 
After a send stream is stopped, it can still be re-used and implicitly
restarted by activating layers. This change removes marking the flag
we use for async operations as 'not alive' inside Stop() and only doing
so when the send stream is stopped permanently.

The effect this has is that an implicit start via
UpdateActiveSimulcastLayers() will run and correctly update the states.
Before, if a stream had been stopped, the safety flag would prevent
the async operation from running.

Bug: chromium:1241213
Change-Id: Iebdfabba3e1955aafa364760eebd4f66281bcc60
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/229304
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34809}
This commit is contained in:
Tommi
2021-08-19 19:18:16 +02:00
committed by WebRTC LUCI CQ
parent 1039392209
commit 51238e6c28
3 changed files with 35 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
video/video_send_stream.cc
View File

@ -244,7 +244,10 @@ void VideoSendStream::Stop() {
RTC_DLOG(LS_INFO) << "VideoSendStream::Stop";
running_ = false;
rtp_transport_queue_->PostTask(ToQueuedTask(transport_queue_safety_, [this] {
transport_queue_safety_->SetNotAlive();
// As the stream can get re-used and implicitly restarted via changing
// the state of the active layers, we do not mark the
// `transport_queue_safety_` flag with `SetNotAlive()` here. That's only
// done when we stop permanently via `StopPermanentlyAndGetRtpStates()`.
send_stream_.Stop();
}));
}

42
video/video_send_stream_tests.cc
View File

@ -95,6 +95,9 @@ enum : int { // The first valid value is 1.
kVideoTimingExtensionId,
};
// Readability convenience enum for `WaitBitrateChanged()`.
enum class WaitUntil : bool { kZero = false, kNonZero = true };
constexpr int64_t kRtcpIntervalMs = 1000;
enum VideoFormat {
@ -2162,7 +2165,7 @@ class StartStopBitrateObserver : public test::FakeEncoder {
return encoder_init_.Wait(VideoSendStreamTest::kDefaultTimeoutMs);
}
bool WaitBitrateChanged(bool non_zero) {
bool WaitBitrateChanged(WaitUntil until) {
do {
absl::optional<int> bitrate_kbps;
{
@ -2172,8 +2175,8 @@ class StartStopBitrateObserver : public test::FakeEncoder {
if (!bitrate_kbps)
continue;
if ((non_zero && *bitrate_kbps > 0) ||
(!non_zero && *bitrate_kbps == 0)) {
if ((until == WaitUntil::kNonZero && *bitrate_kbps > 0) ||
(until == WaitUntil::kZero && *bitrate_kbps == 0)) {
return true;
}
} while (bitrate_changed_.Wait(VideoSendStreamTest::kDefaultTimeoutMs));
@ -2220,15 +2223,15 @@ TEST_F(VideoSendStreamTest, VideoSendStreamStopSetEncoderRateToZero) {
SendTask(RTC_FROM_HERE, task_queue(),
[this]() { GetVideoSendStream()->Start(); });
EXPECT_TRUE(encoder.WaitBitrateChanged(true));
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kNonZero));
SendTask(RTC_FROM_HERE, task_queue(),
[this]() { GetVideoSendStream()->Stop(); });
EXPECT_TRUE(encoder.WaitBitrateChanged(false));
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kZero));
SendTask(RTC_FROM_HERE, task_queue(),
[this]() { GetVideoSendStream()->Start(); });
EXPECT_TRUE(encoder.WaitBitrateChanged(true));
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kNonZero));
SendTask(RTC_FROM_HERE, task_queue(), [this]() {
DestroyStreams();
@ -2277,7 +2280,7 @@ TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {
GetVideoSendStream()->UpdateActiveSimulcastLayers({true, true});
EXPECT_TRUE(GetVideoSendStream()->started());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(true));
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kNonZero));
GetVideoEncoderConfig()->simulcast_layers[0].active = true;
GetVideoEncoderConfig()->simulcast_layers[1].active = false;
@ -2285,7 +2288,7 @@ TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {
GetVideoSendStream()->ReconfigureVideoEncoder(
GetVideoEncoderConfig()->Copy());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(true));
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kNonZero));
// Turning off both simulcast layers should trigger a bitrate change of 0.
GetVideoEncoderConfig()->simulcast_layers[0].active = false;
@ -2294,7 +2297,7 @@ TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {
GetVideoSendStream()->UpdateActiveSimulcastLayers({false, false});
EXPECT_FALSE(GetVideoSendStream()->started());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(false));
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kZero));
// Re-activating a layer should resume sending and trigger a bitrate change.
GetVideoEncoderConfig()->simulcast_layers[0].active = true;
@ -2302,12 +2305,23 @@ TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {
GetVideoSendStream()->UpdateActiveSimulcastLayers({true, false});
EXPECT_TRUE(GetVideoSendStream()->started());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(true));
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kNonZero));
// Stop and clean up.
SendTask(RTC_FROM_HERE, task_queue(),
[this]() { GetVideoSendStream()->Stop(); });
EXPECT_TRUE(encoder.WaitBitrateChanged(false));
// Stop the stream and make sure the bit rate goes to zero again.
SendTask(RTC_FROM_HERE, task_queue(), [this]() {
GetVideoSendStream()->Stop();
EXPECT_FALSE(GetVideoSendStream()->started());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kZero));
// One last test to verify that after `Stop()` we can still implicitly start
// the stream if needed. This is what will happen when a send stream gets
// re-used. See crbug.com/1241213.
SendTask(RTC_FROM_HERE, task_queue(), [this]() {
GetVideoSendStream()->UpdateActiveSimulcastLayers({true, true});
EXPECT_TRUE(GetVideoSendStream()->started());
});
EXPECT_TRUE(encoder.WaitBitrateChanged(WaitUntil::kNonZero));
SendTask(RTC_FROM_HERE, task_queue(), [this]() {
DestroyStreams();

3
video/video_stream_encoder.cc
View File

@ -1780,6 +1780,9 @@ void VideoStreamEncoder::SendKeyFrame() {
TRACE_EVENT0("webrtc", "OnKeyFrameRequest");
RTC_DCHECK(!next_frame_types_.empty());
if (!encoder_)
return; // Shutting down.
// TODO(webrtc:10615): Map keyframe request to spatial layer.
std::fill(next_frame_types_.begin(), next_frame_types_.end(),
VideoFrameType::kVideoFrameKey);