Fix error adding receive stream after packet received from non-primary SSRC.

The non-primary SSRC being RTX, for example. Normally a default stream
wouldn't be created from RTX packets, but there is a window of time
where packets can be received before the video engine has receive
parameters/payload type mappings, so it creates one anyway.

Then in AddRecvStream, normally the default stream would be destroyed
before creating a new one, but this only happens for sp.first_ssrc().
Resulting in the error "Receive stream with SSRC 'X' already exists".

Fixed by simply iterating over all SSRCs.

Bug: webrtc:13171
Change-Id: Iaf4e4a3ceafddee3d9b2d1e24af68be56f9695de
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231633
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34971}
This commit is contained in:
Taylor Brandstetter
2021-09-09 18:34:06 -07:00
committed by WebRTC LUCI CQ
parent 3a9640a5aa
commit b9b0890541
2 changed files with 30 additions and 12 deletions

View File

@ -1452,18 +1452,18 @@ bool WebRtcVideoChannel::AddRecvStream(const StreamParams& sp,
if (!ValidateStreamParams(sp))
return false;
uint32_t ssrc = sp.first_ssrc();
// Remove running stream if this was a default stream.
const auto& prev_stream = receive_streams_.find(ssrc);
if (prev_stream != receive_streams_.end()) {
if (default_stream || !prev_stream->second->IsDefaultStream()) {
RTC_LOG(LS_ERROR) << "Receive stream for SSRC '" << ssrc
<< "' already exists.";
return false;
for (uint32_t ssrc : sp.ssrcs) {
// Remove running stream if this was a default stream.
const auto& prev_stream = receive_streams_.find(ssrc);
if (prev_stream != receive_streams_.end()) {
if (default_stream || !prev_stream->second->IsDefaultStream()) {
RTC_LOG(LS_ERROR) << "Receive stream for SSRC '" << ssrc
<< "' already exists.";
return false;
}
DeleteReceiveStream(prev_stream->second);
receive_streams_.erase(prev_stream);
}
DeleteReceiveStream(prev_stream->second);
receive_streams_.erase(prev_stream);
}
if (!ValidateReceiveSsrcAvailability(sp))
@ -1486,7 +1486,7 @@ bool WebRtcVideoChannel::AddRecvStream(const StreamParams& sp,
if (unsignaled_frame_transformer_ && !config.frame_transformer)
config.frame_transformer = unsignaled_frame_transformer_;
receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
receive_streams_[sp.first_ssrc()] = new WebRtcVideoReceiveStream(
this, call_, sp, std::move(config), default_stream, recv_codecs_,
flexfec_config);

View File

@ -8623,6 +8623,24 @@ TEST_F(WebRtcVideoChannelTest,
EXPECT_FALSE(rtp_parameters.encodings[0].ssrc);
}
// Test that if a default stream is created for a non-primary stream (for
// example, RTX before we know it's RTX), we are still able to explicitly add
// the stream later.
TEST_F(WebRtcVideoChannelTest,
AddReceiveStreamAfterReceivingNonPrimaryUnsignaledSsrc) {
// Receive VP8 RTX packet.
RtpPacket rtp_packet;
const cricket::VideoCodec vp8 = GetEngineCodec("VP8");
rtp_packet.SetPayloadType(default_apt_rtx_types_[vp8.id]);
rtp_packet.SetSsrc(2);
ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
cricket::StreamParams params = cricket::StreamParams::CreateLegacy(1);
params.AddFidSsrc(1, 2);
EXPECT_TRUE(channel_->AddRecvStream(params));
}
void WebRtcVideoChannelTest::TestReceiverLocalSsrcConfiguration(
bool receiver_first) {
EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));