admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug fix for applying a remote description twice without stream IDs.

Browse Source 
A downstream bug ocurred because of a lack of symmetry when adding and
removing a remote sender in Plan B that specifies SSRCs, but doesn't
specify stream IDs. The issue when the first remote description is
applied "default" for the stream ID on the remote sender, but the
second time it's applied the current remote sender's "default" stream
ID does not match the new remote description's empty stream ID. This
was incorrectly interpreted as a new remote sender (which removed/added
the sender).

Bug: webrtc:7933
Change-Id: I87191b9e887b3450ef15111b5e867023c723a86e
Reviewed-on: https://webrtc-review.googlesource.com/67191
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Noah Richards <noahric@chromium.org>
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22760}
This commit is contained in:
Seth Hampson 2018-04-05 18:12:09 -07:00 committed by Commit Bot
parent 780dc38e41
commit 83d676bd15
2 changed files with 26 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
pc/peerconnection.cc
View File

@ -4044,8 +4044,14 @@ void PeerConnection::UpdateRemoteSendersList(
const RtpSenderInfo& info = *sender_it;
const cricket::StreamParams* params =
cricket::GetStreamBySsrc(streams, info.first_ssrc);
std::string params_stream_id;
if (params) {
params_stream_id =
(!params->first_stream_id().empty() ? params->first_stream_id()
: kDefaultStreamId);
}
bool sender_exists = params && params->id == info.sender_id &&
params->first_stream_id() == info.stream_id;
params_stream_id == info.stream_id;
// If this is a default track, and we still need it, don't remove it.
if ((info.stream_id == kDefaultStreamId && default_sender_needed) ||
sender_exists) {
@ -4072,8 +4078,8 @@ void PeerConnection::UpdateRemoteSendersList(
// not supported in Plan B, we just take the first here and create the
// default stream ID if none is specified.
const std::string& stream_id =
(!params.stream_ids().empty() ? params.stream_ids()[0]
: kDefaultStreamId);
(!params.first_stream_id().empty() ? params.first_stream_id()
: kDefaultStreamId);
const std::string& sender_id = params.id;
uint32_t ssrc = params.first_ssrc();
@ -4137,6 +4143,10 @@ void PeerConnection::OnRemoteSenderAdded(const RtpSenderInfo& sender_info,
void PeerConnection::OnRemoteSenderRemoved(const RtpSenderInfo& sender_info,
cricket::MediaType media_type) {
RTC_LOG(LS_INFO) << "Removing " << cricket::MediaTypeToString(media_type)
<< " receiver for track_id=" << sender_info.sender_id
<< " and stream_id=" << sender_info.stream_id;
MediaStreamInterface* stream = remote_streams_->find(sender_info.stream_id);
rtc::scoped_refptr<RtpReceiverInterface> receiver;

13
pc/peerconnectioninterface_unittest.cc
View File

@ -3122,6 +3122,19 @@ TEST_F(PeerConnectionInterfaceTestPlanB,
MediaStreamInterface* remote_stream = observer_.remote_streams()->at(0);
EXPECT_EQ("default", remote_stream->id());
ASSERT_EQ(1u, remote_stream->GetAudioTracks().size());
// Previously a bug ocurred when setting the remote description a second time.
// This is because we checked equality of the remote StreamParams stream ID
// (empty), and the previously set stream ID for the remote sender
// ("default"). This cause a track to be removed, then added, when really
// nothing should occur because it is the same track.
CreateAndSetRemoteOffer(sdp_string);
EXPECT_EQ(0u, observer_.remove_track_events_.size());
EXPECT_EQ(1u, observer_.add_track_events_.size());
EXPECT_EQ("audiotrack0", observer_.last_added_track_label_);
remote_stream = observer_.remote_streams()->at(0);
EXPECT_EQ("default", remote_stream->id());
ASSERT_EQ(1u, remote_stream->GetAudioTracks().size());
}
// This tests that an RtpSender is created when the local description is set