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

Fixing issue with default stream upon setting 2nd remote description.

Browse Source 
If a description is set that requires making a default stream, and one
already exists, we'll now keep the existing default audio/video tracks,
rather than destroying them and recreating them. Destroying them caused
the blink MediaStream to go to an "ended" state, which is the root cause
of the bug.

BUG=webrtc:5250

Review URL: https://codereview.webrtc.org/1469833006

Cr-Commit-Position: refs/heads/master@{#10946}
This commit is contained in:
deadbeef
2015-12-08 17:13:40 -08:00
committed by Commit bot
parent d02b0fab76
commit bda7e0b932
3 changed files with 95 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

134
talk/app/webrtc/peerconnection.cc
View File

@ -1137,6 +1137,22 @@ void PeerConnection::SetRemoteDescription(
}
const cricket::SessionDescription* remote_desc = desc->description();
const cricket::ContentInfo* audio_content = GetFirstAudioContent(remote_desc);
const cricket::ContentInfo* video_content = GetFirstVideoContent(remote_desc);
const cricket::AudioContentDescription* audio_desc =
GetFirstAudioContentDescription(remote_desc);
const cricket::VideoContentDescription* video_desc =
GetFirstVideoContentDescription(remote_desc);
const cricket::DataContentDescription* data_desc =
GetFirstDataContentDescription(remote_desc);
// Check if the descriptions include streams, just in case the peer supports
// MSID, but doesn't indicate so with "a=msid-semantic".
if (remote_desc->msid_supported() ||
(audio_desc && !audio_desc->streams().empty()) ||
(video_desc && !video_desc->streams().empty())) {
remote_peer_supports_msid_ = true;
}
// We wait to signal new streams until we finish processing the description,
// since only at that point will new streams have all their tracks.
@ -1144,49 +1160,39 @@ void PeerConnection::SetRemoteDescription(
// Find all audio rtp streams and create corresponding remote AudioTracks
// and MediaStreams.
const cricket::ContentInfo* audio_content = GetFirstAudioContent(remote_desc);
if (audio_content) {
if (audio_content->rejected) {
RemoveTracks(cricket::MEDIA_TYPE_AUDIO);
} else {
const cricket::AudioContentDescription* desc =
static_cast<const cricket::AudioContentDescription*>(
audio_content->description);
UpdateRemoteStreamsList(GetActiveStreams(desc), desc->type(),
bool default_audio_track_needed =
!remote_peer_supports_msid_ &&
MediaContentDirectionHasSend(audio_desc->direction());
UpdateRemoteStreamsList(GetActiveStreams(audio_desc),
default_audio_track_needed, audio_desc->type(),
new_streams);
remote_info_.default_audio_track_needed =
!remote_desc->msid_supported() && desc->streams().empty() &&
MediaContentDirectionHasSend(desc->direction());
}
}
// Find all video rtp streams and create corresponding remote VideoTracks
// and MediaStreams.
const cricket::ContentInfo* video_content = GetFirstVideoContent(remote_desc);
if (video_content) {
if (video_content->rejected) {
RemoveTracks(cricket::MEDIA_TYPE_VIDEO);
} else {
const cricket::VideoContentDescription* desc =
static_cast<const cricket::VideoContentDescription*>(
video_content->description);
UpdateRemoteStreamsList(GetActiveStreams(desc), desc->type(),
bool default_video_track_needed =
!remote_peer_supports_msid_ &&
MediaContentDirectionHasSend(video_desc->direction());
UpdateRemoteStreamsList(GetActiveStreams(video_desc),
default_video_track_needed, video_desc->type(),
new_streams);
remote_info_.default_video_track_needed =
!remote_desc->msid_supported() && desc->streams().empty() &&
MediaContentDirectionHasSend(desc->direction());
}
}
// Update the DataChannels with the information from the remote peer.
const cricket::ContentInfo* data_content = GetFirstDataContent(remote_desc);
if (data_content) {
const cricket::DataContentDescription* desc =
static_cast<const cricket::DataContentDescription*>(
data_content->description);
if (rtc::starts_with(desc->protocol().data(),
if (data_desc) {
if (rtc::starts_with(data_desc->protocol().data(),
cricket::kMediaProtocolRtpPrefix)) {
UpdateRemoteRtpDataChannels(GetActiveStreams(desc));
UpdateRemoteRtpDataChannels(GetActiveStreams(data_desc));
}
}
@ -1197,15 +1203,7 @@ void PeerConnection::SetRemoteDescription(
observer_->OnAddStream(new_stream);
}
// Find removed MediaStreams.
if (remote_info_.IsDefaultMediaStreamNeeded() &&
remote_streams_->find(kDefaultStreamLabel) != nullptr) {
// The default media stream already exists. No need to do anything.
} else {
UpdateEndedRemoteMediaStreams();
remote_info_.msid_supported |= remote_streams_->count() > 0;
}
MaybeCreateDefaultStream();
UpdateEndedRemoteMediaStreams();
SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer);
signaling_thread()->Post(this, MSG_SET_SESSIONDESCRIPTION_SUCCESS, msg);
@ -1507,12 +1505,13 @@ bool PeerConnection::GetOptionsForAnswer(
void PeerConnection::RemoveTracks(cricket::MediaType media_type) {
UpdateLocalTracks(std::vector<cricket::StreamParams>(), media_type);
UpdateRemoteStreamsList(std::vector<cricket::StreamParams>(), media_type,
nullptr);
UpdateRemoteStreamsList(std::vector<cricket::StreamParams>(), false,
media_type, nullptr);
}
void PeerConnection::UpdateRemoteStreamsList(
const cricket::StreamParamsVec& streams,
bool default_track_needed,
cricket::MediaType media_type,
StreamCollection* new_streams) {
TrackInfos* current_tracks = GetRemoteTracks(media_type);
@ -1524,11 +1523,14 @@ void PeerConnection::UpdateRemoteStreamsList(
const TrackInfo& info = *track_it;
const cricket::StreamParams* params =
cricket::GetStreamBySsrc(streams, info.ssrc);
if (!params || params->id != info.track_id) {
bool track_exists = params && params->id == info.track_id;
// If this is a default track, and we still need it, don't remove it.
if ((info.stream_label == kDefaultStreamLabel && default_track_needed) ||
track_exists) {
++track_it;
} else {
OnRemoteTrackRemoved(info.stream_label, info.track_id, media_type);
track_it = current_tracks->erase(track_it);
} else {
++track_it;
}
}
@ -1556,6 +1558,29 @@ void PeerConnection::UpdateRemoteStreamsList(
OnRemoteTrackSeen(stream_label, track_id, ssrc, media_type);
}
}
// Add default track if necessary.
if (default_track_needed) {
rtc::scoped_refptr<MediaStreamInterface> default_stream =
remote_streams_->find(kDefaultStreamLabel);
if (!default_stream) {
// Create the new default MediaStream.
default_stream =
remote_stream_factory_->CreateMediaStream(kDefaultStreamLabel);
remote_streams_->AddStream(default_stream);
new_streams->AddStream(default_stream);
}
std::string default_track_id = (media_type == cricket::MEDIA_TYPE_AUDIO)
? kDefaultAudioTrackLabel
: kDefaultVideoTrackLabel;
const TrackInfo* default_track_info =
FindTrackInfo(*current_tracks, kDefaultStreamLabel, default_track_id);
if (!default_track_info) {
current_tracks->push_back(
TrackInfo(kDefaultStreamLabel, default_track_id, 0));
OnRemoteTrackSeen(kDefaultStreamLabel, default_track_id, 0, media_type);
}
}
}
void PeerConnection::OnRemoteTrackSeen(const std::string& stream_label,
@ -1618,41 +1643,6 @@ void PeerConnection::UpdateEndedRemoteMediaStreams() {
}
}
void PeerConnection::MaybeCreateDefaultStream() {
if (!remote_info_.IsDefaultMediaStreamNeeded()) {
return;
}
bool default_created = false;
rtc::scoped_refptr<MediaStreamInterface> default_remote_stream =
remote_streams_->find(kDefaultStreamLabel);
if (default_remote_stream == nullptr) {
default_created = true;
default_remote_stream =
remote_stream_factory_->CreateMediaStream(kDefaultStreamLabel);
remote_streams_->AddStream(default_remote_stream);
}
if (remote_info_.default_audio_track_needed &&
default_remote_stream->GetAudioTracks().size() == 0) {
remote_audio_tracks_.push_back(
TrackInfo(kDefaultStreamLabel, kDefaultAudioTrackLabel, 0));
OnRemoteTrackSeen(kDefaultStreamLabel, kDefaultAudioTrackLabel, 0,
cricket::MEDIA_TYPE_AUDIO);
}
if (remote_info_.default_video_track_needed &&
default_remote_stream->GetVideoTracks().size() == 0) {
remote_video_tracks_.push_back(
TrackInfo(kDefaultStreamLabel, kDefaultVideoTrackLabel, 0));
OnRemoteTrackSeen(kDefaultStreamLabel, kDefaultVideoTrackLabel, 0,
cricket::MEDIA_TYPE_VIDEO);
}
if (default_created) {
stats_->AddStream(default_remote_stream);
observer_->OnAddStream(default_remote_stream);
}
}
void PeerConnection::EndRemoteTracks(cricket::MediaType media_type) {
TrackInfos* current_tracks = GetRemoteTracks(media_type);
for (TrackInfos::iterator track_it = current_tracks->begin();

37
talk/app/webrtc/peerconnection.h
View File

@ -161,32 +161,16 @@ class PeerConnection : public PeerConnectionInterface,
const std::string track_id,
uint32_t ssrc)
: stream_label(stream_label), track_id(track_id), ssrc(ssrc) {}
bool operator==(const TrackInfo& other) {
return this->stream_label == other.stream_label &&
this->track_id == other.track_id && this->ssrc == other.ssrc;
}
std::string stream_label;
std::string track_id;
uint32_t ssrc;
};
typedef std::vector<TrackInfo> TrackInfos;
struct RemotePeerInfo {
RemotePeerInfo()
: msid_supported(false),
default_audio_track_needed(false),
default_video_track_needed(false) {}
// True if it has been discovered that the remote peer support MSID.
bool msid_supported;
// The remote peer indicates in the session description that audio will be
// sent but no MSID is given.
bool default_audio_track_needed;
// The remote peer indicates in the session description that video will be
// sent but no MSID is given.
bool default_video_track_needed;
bool IsDefaultMediaStreamNeeded() {
return !msid_supported &&
(default_audio_track_needed || default_video_track_needed);
}
};
// Implements MessageHandler.
void OnMessage(rtc::Message* msg) override;
@ -247,12 +231,15 @@ class PeerConnection : public PeerConnectionInterface,
// Called when a media type is rejected (m-line set to port 0).
void RemoveTracks(cricket::MediaType media_type);
// Makes sure a MediaStream Track is created for each StreamParam in
// |streams|. |media_type| is the type of the |streams| and can be either
// audio or video.
// Makes sure a MediaStreamTrack is created for each StreamParam in |streams|,
// and existing MediaStreamTracks are removed if there is no corresponding
// StreamParam. If |default_track_needed| is true, a default MediaStreamTrack
// is created if it doesn't exist; if false, it's removed if it exists.
// |media_type| is the type of the |streams| and can be either audio or video.
// If a new MediaStream is created it is added to |new_streams|.
void UpdateRemoteStreamsList(
const std::vector<cricket::StreamParams>& streams,
bool default_track_needed,
cricket::MediaType media_type,
StreamCollection* new_streams);
@ -276,8 +263,6 @@ class PeerConnection : public PeerConnectionInterface,
// exist.
void UpdateEndedRemoteMediaStreams();
void MaybeCreateDefaultStream();
// Set the MediaStreamTrackInterface::TrackState to |kEnded| on all remote
// tracks of type |media_type|.
void EndRemoteTracks(cricket::MediaType media_type);
@ -390,7 +375,7 @@ class PeerConnection : public PeerConnectionInterface,
std::map<std::string, rtc::scoped_refptr<DataChannel>> rtp_data_channels_;
std::vector<rtc::scoped_refptr<DataChannel>> sctp_data_channels_;
RemotePeerInfo remote_info_;
bool remote_peer_supports_msid_ = false;
rtc::scoped_ptr<RemoteMediaStreamFactory> remote_stream_factory_;
std::vector<rtc::scoped_refptr<RtpSenderInterface>> senders_;

22
talk/app/webrtc/peerconnectioninterface_unittest.cc
View File

@ -1994,6 +1994,28 @@ TEST_F(PeerConnectionInterfaceTest, SdpWithMsidDontCreatesDefaultStream) {
EXPECT_EQ(0u, observer_.remote_streams()->count());
}
// This tests that when setting a new description, the old default tracks are
// not destroyed and recreated.
// See: https://bugs.chromium.org/p/webrtc/issues/detail?id=5250
TEST_F(PeerConnectionInterfaceTest, DefaultTracksNotDestroyedAndRecreated) {
FakeConstraints constraints;
constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
true);
CreatePeerConnection(&constraints);
CreateAndSetRemoteOffer(kSdpStringWithoutStreamsAudioOnly);
ASSERT_EQ(1u, observer_.remote_streams()->count());
MediaStreamInterface* remote_stream = observer_.remote_streams()->at(0);
ASSERT_EQ(1u, remote_stream->GetAudioTracks().size());
// Set the track to "disabled", then set a new description and ensure the
// track is still disabled, which ensures it hasn't been recreated.
remote_stream->GetAudioTracks()[0]->set_enabled(false);
CreateAndSetRemoteOffer(kSdpStringWithoutStreamsAudioOnly);
ASSERT_EQ(1u, remote_stream->GetAudioTracks().size());
EXPECT_FALSE(remote_stream->GetAudioTracks()[0]->enabled());
}
// This tests that a default MediaStream is not created if a remote session
// description is updated to not have any MediaStreams.
TEST_F(PeerConnectionInterfaceTest, VerifyDefaultStreamIsNotCreated) {