Skip to content
This repository was archived by the owner on Oct 25, 2024. It is now read-only.

Commit de6fa1e

Browse files
rasmusbrandtCommit Bot
authored andcommitted
Reland "Let WebRtcVideoChannel::ResetUnsignaledRecvStream delete all default streams."
This is a reland of d335426. 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 <[email protected]> > Reviewed-by: Magnus Flodman <[email protected]> > Commit-Queue: Rasmus Brandt <[email protected]> > Cr-Commit-Position: refs/heads/master@{#30971} [email protected],[email protected] Bug: webrtc:11477 Change-Id: I70b8fa47b4d1d0aa36fed4d8612e13fa7f992925 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172782 Reviewed-by: Rasmus Brandt <[email protected]> Commit-Queue: Rasmus Brandt <[email protected]> Cr-Commit-Position: refs/heads/master@{#30986}
1 parent 68063a2 commit de6fa1e

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

media/engine/webrtc_video_engine.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,6 +1482,21 @@ void WebRtcVideoChannel::ResetUnsignaledRecvStream() {
14821482
RTC_DCHECK_RUN_ON(&thread_checker_);
14831483
RTC_LOG(LS_INFO) << "ResetUnsignaledRecvStream.";
14841484
unsignaled_stream_params_ = StreamParams();
1485+
1486+
// Delete any created default streams.
1487+
auto it = receive_streams_.begin();
1488+
while (it != receive_streams_.end()) {
1489+
auto delete_it = receive_streams_.end();
1490+
if (it->second->IsDefaultStream()) {
1491+
delete_it = it;
1492+
}
1493+
++it;
1494+
if (delete_it != receive_streams_.end()) {
1495+
DeleteReceiveStream(delete_it->second);
1496+
// |it| is not invalidated by this erase.
1497+
receive_streams_.erase(delete_it->first);
1498+
}
1499+
}
14851500
}
14861501

14871502
bool WebRtcVideoChannel::SetSink(

media/engine/webrtc_video_engine_unittest.cc

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5836,7 +5836,6 @@ TEST_F(WebRtcVideoChannelTest, RecvUnsignaledSsrcWithSignaledStreamId) {
58365836
// Reset the unsignaled stream to clear the cache. This time when
58375837
// a default video receive stream is created it won't have a sync_group.
58385838
channel_->ResetUnsignaledRecvStream();
5839-
ASSERT_TRUE(channel_->RemoveRecvStream(kIncomingUnsignalledSsrc));
58405839
EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size());
58415840

58425841
channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
@@ -5845,6 +5844,37 @@ TEST_F(WebRtcVideoChannelTest, RecvUnsignaledSsrcWithSignaledStreamId) {
58455844
fake_call_->GetVideoReceiveStreams()[0]->GetConfig().sync_group.empty());
58465845
}
58475846

5847+
TEST_F(WebRtcVideoChannelTest,
5848+
ResetUnsignaledRecvStreamDeletesAllDefaultStreams) {
5849+
// No receive streams to start with.
5850+
EXPECT_TRUE(fake_call_->GetVideoReceiveStreams().empty());
5851+
5852+
// Packet with unsignaled SSRC is received.
5853+
const size_t kDataLength = 12;
5854+
uint8_t data[kDataLength];
5855+
memset(data, 0, sizeof(data));
5856+
rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc);
5857+
rtc::CopyOnWriteBuffer packet(data, kDataLength);
5858+
channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
5859+
5860+
// Default receive stream created.
5861+
const auto& receivers1 = fake_call_->GetVideoReceiveStreams();
5862+
ASSERT_EQ(receivers1.size(), 1u);
5863+
EXPECT_EQ(receivers1[0]->GetConfig().rtp.remote_ssrc,
5864+
kIncomingUnsignalledSsrc);
5865+
5866+
// Stream with another SSRC gets signaled.
5867+
channel_->ResetUnsignaledRecvStream();
5868+
constexpr uint32_t kIncomingSignalledSsrc = kIncomingUnsignalledSsrc + 1;
5869+
ASSERT_TRUE(channel_->AddRecvStream(
5870+
cricket::StreamParams::CreateLegacy(kIncomingSignalledSsrc)));
5871+
5872+
// New receiver is for the signaled stream.
5873+
const auto& receivers2 = fake_call_->GetVideoReceiveStreams();
5874+
ASSERT_EQ(receivers2.size(), 1u);
5875+
EXPECT_EQ(receivers2[0]->GetConfig().rtp.remote_ssrc, kIncomingSignalledSsrc);
5876+
}
5877+
58485878
// Test BaseMinimumPlayoutDelayMs on receive streams.
58495879
TEST_F(WebRtcVideoChannelTest, BaseMinimumPlayoutDelayMs) {
58505880
// Test that set won't work for non-existing receive streams.

0 commit comments

Comments
 (0)