Reland "Let WebRtcVideoChannel::ResetUnsignaledRecvStream delete all default streams."
This is a reland of d335426a39d34389a00f8f7ae652d535f0fa2073.
The revert was premature: the failing tests were known to be flaky
(crbug.com/1066515, crbug.com/1066453, crbug.com/1066407, crbug.com/1066399)
Original change's description:
> Let WebRtcVideoChannel::ResetUnsignaledRecvStream delete all default streams.
>
> This CL changes WebRtcVideoChannel::ResetUnsignaledRecvStream so
> that it deletes all default streams created by
> WebRtcVideoChannel::AddRecvStream. This is needed for the case that
> there are lingering default streams, whose SSRCs are different
> from the SSRCs that were subsequently signaled. This can happen
> when there are multiple "m= sections" and the early media is
> sent to an "m= section" that is later not supposed to be the
> sink for that particular SSRC.
>
> Default streams whose SSRC match the subsequently signaled
> SSRC is already handled here: https://source.chromium.org/chromium/chromium/src/+/master:third_party/webrtc/media/engine/webrtc_video_engine.cc;l=1386;drc=22387b44ff173d263b434889d394cea90368ab06?originalUrl=https:%2F%2Fcs.chromium.org%2F
>
> Bug: webrtc:11477
> Change-Id: I96ed7e35b4904fb0757fe5824f8afa6f1b9a565e
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172622
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Magnus Flodman <mflodman@webrtc.org>
> Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30971}
TBR=mflodman@webrtc.org,hta@webrtc.org
Bug: webrtc:11477
Change-Id: I70b8fa47b4d1d0aa36fed4d8612e13fa7f992925
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172782
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30986}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 22856b0..a19d444 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1482,6 +1482,21 @@
RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_LOG(LS_INFO) << "ResetUnsignaledRecvStream.";
unsignaled_stream_params_ = StreamParams();
+
+ // Delete any created default streams.
+ auto it = receive_streams_.begin();
+ while (it != receive_streams_.end()) {
+ auto delete_it = receive_streams_.end();
+ if (it->second->IsDefaultStream()) {
+ delete_it = it;
+ }
+ ++it;
+ if (delete_it != receive_streams_.end()) {
+ DeleteReceiveStream(delete_it->second);
+ // |it| is not invalidated by this erase.
+ receive_streams_.erase(delete_it->first);
+ }
+ }
}
bool WebRtcVideoChannel::SetSink(
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 395d38a..27206db 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -5836,7 +5836,6 @@
// Reset the unsignaled stream to clear the cache. This time when
// a default video receive stream is created it won't have a sync_group.
channel_->ResetUnsignaledRecvStream();
- ASSERT_TRUE(channel_->RemoveRecvStream(kIncomingUnsignalledSsrc));
EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size());
channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
@@ -5845,6 +5844,37 @@
fake_call_->GetVideoReceiveStreams()[0]->GetConfig().sync_group.empty());
}
+TEST_F(WebRtcVideoChannelTest,
+ ResetUnsignaledRecvStreamDeletesAllDefaultStreams) {
+ // No receive streams to start with.
+ EXPECT_TRUE(fake_call_->GetVideoReceiveStreams().empty());
+
+ // Packet with unsignaled SSRC is received.
+ const size_t kDataLength = 12;
+ uint8_t data[kDataLength];
+ memset(data, 0, sizeof(data));
+ rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc);
+ rtc::CopyOnWriteBuffer packet(data, kDataLength);
+ channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+
+ // Default receive stream created.
+ const auto& receivers1 = fake_call_->GetVideoReceiveStreams();
+ ASSERT_EQ(receivers1.size(), 1u);
+ EXPECT_EQ(receivers1[0]->GetConfig().rtp.remote_ssrc,
+ kIncomingUnsignalledSsrc);
+
+ // Stream with another SSRC gets signaled.
+ channel_->ResetUnsignaledRecvStream();
+ constexpr uint32_t kIncomingSignalledSsrc = kIncomingUnsignalledSsrc + 1;
+ ASSERT_TRUE(channel_->AddRecvStream(
+ cricket::StreamParams::CreateLegacy(kIncomingSignalledSsrc)));
+
+ // New receiver is for the signaled stream.
+ const auto& receivers2 = fake_call_->GetVideoReceiveStreams();
+ ASSERT_EQ(receivers2.size(), 1u);
+ EXPECT_EQ(receivers2[0]->GetConfig().rtp.remote_ssrc, kIncomingSignalledSsrc);
+}
+
// Test BaseMinimumPlayoutDelayMs on receive streams.
TEST_F(WebRtcVideoChannelTest, BaseMinimumPlayoutDelayMs) {
// Test that set won't work for non-existing receive streams.