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

When a track is added/removed directly to MediaStream notify observer->OnRenegotionNeeded

Browse Source 
There is an inconsistency in behavior of PeerConnection.
When I remove track from PeerConnection observer->OnRenegotiationNeeded is called, however if I remove track from MediaStream then there is no notification to renegotiate.
This patch adds missing OnRenegotiationNeeded calls.

BUG=webrtc:7966

Review-Url: https://codereview.webrtc.org/2977493002
Cr-Commit-Position: refs/heads/master@{#19125}
This commit is contained in:
korniltsev.anatoly 2017-07-24 17:00:25 -07:00 committed by Commit Bot
parent d083e851f6
commit ec390b5dfb
3 changed files with 143 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

171
webrtc/pc/peerconnection.cc
View File

@ -574,10 +574,10 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) {
stream_observers_.push_back(std::unique_ptr<MediaStreamObserver>(observer));
for (const auto& track : local_stream->GetAudioTracks()) {
OnAudioTrackAdded(track.get(), local_stream);
AddAudioTrack(track.get(), local_stream);
}
for (const auto& track : local_stream->GetVideoTracks()) {
OnVideoTrackAdded(track.get(), local_stream);
AddVideoTrack(track.get(), local_stream);
}
stats_->AddStream(local_stream);
@ -587,13 +587,14 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) {
void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) {
TRACE_EVENT0("webrtc", "PeerConnection::RemoveStream");
for (const auto& track : local_stream->GetAudioTracks()) {
OnAudioTrackRemoved(track.get(), local_stream);
if (!IsClosed()) {
for (const auto& track : local_stream->GetAudioTracks()) {
RemoveAudioTrack(track.get(), local_stream);
}
for (const auto& track : local_stream->GetVideoTracks()) {
RemoveVideoTrack(track.get(), local_stream);
}
}
for (const auto& track : local_stream->GetVideoTracks()) {
OnVideoTrackRemoved(track.get(), local_stream);
}
local_streams_->RemoveStream(local_stream);
stream_observers_.erase(
std::remove_if(
@ -1496,6 +1497,89 @@ void PeerConnection::DestroyReceiver(const std::string& track_id) {
}
}
void PeerConnection::AddAudioTrack(AudioTrackInterface* track,
MediaStreamInterface* stream) {
RTC_DCHECK(!IsClosed());
auto sender = FindSenderForTrack(track);
if (sender != senders_.end()) {
// We already have a sender for this track, so just change the stream_id
// so that it's correct in the next call to CreateOffer.
(*sender)->internal()->set_stream_id(stream->label());
return;
}
// Normal case; we've never seen this track before.
rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender =
RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
signaling_thread(),
new AudioRtpSender(track, stream->label(), session_->voice_channel(),
stats_.get()));
senders_.push_back(new_sender);
// If the sender has already been configured in SDP, we call SetSsrc,
// which will connect the sender to the underlying transport. This can
// occur if a local session description that contains the ID of the sender
// is set before AddStream is called. It can also occur if the local
// session description is not changed and RemoveStream is called, and
// later AddStream is called again with the same stream.
const TrackInfo* track_info =
FindTrackInfo(local_audio_tracks_, stream->label(), track->id());
if (track_info) {
new_sender->internal()->SetSsrc(track_info->ssrc);
}
}
// TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around
// indefinitely, when we have unified plan SDP.
void PeerConnection::RemoveAudioTrack(AudioTrackInterface* track,
MediaStreamInterface* stream) {
RTC_DCHECK(!IsClosed());
auto sender = FindSenderForTrack(track);
if (sender == senders_.end()) {
LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
<< " doesn't exist.";
return;
}
(*sender)->internal()->Stop();
senders_.erase(sender);
}
void PeerConnection::AddVideoTrack(VideoTrackInterface* track,
MediaStreamInterface* stream) {
RTC_DCHECK(!IsClosed());
auto sender = FindSenderForTrack(track);
if (sender != senders_.end()) {
// We already have a sender for this track, so just change the stream_id
// so that it's correct in the next call to CreateOffer.
(*sender)->internal()->set_stream_id(stream->label());
return;
}
// Normal case; we've never seen this track before.
rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender =
RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
signaling_thread(), new VideoRtpSender(track, stream->label(),
session_->video_channel()));
senders_.push_back(new_sender);
const TrackInfo* track_info =
FindTrackInfo(local_video_tracks_, stream->label(), track->id());
if (track_info) {
new_sender->internal()->SetSsrc(track_info->ssrc);
}
}
void PeerConnection::RemoveVideoTrack(VideoTrackInterface* track,
MediaStreamInterface* stream) {
RTC_DCHECK(!IsClosed());
auto sender = FindSenderForTrack(track);
if (sender == senders_.end()) {
LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
<< " doesn't exist.";
return;
}
(*sender)->internal()->Stop();
senders_.erase(sender);
}
void PeerConnection::OnIceConnectionStateChange(
PeerConnectionInterface::IceConnectionState new_state) {
RTC_DCHECK(signaling_thread()->IsCurrent());
@ -1563,49 +1647,17 @@ void PeerConnection::OnAudioTrackAdded(AudioTrackInterface* track,
if (IsClosed()) {
return;
}
auto sender = FindSenderForTrack(track);
if (sender != senders_.end()) {
// We already have a sender for this track, so just change the stream_id
// so that it's correct in the next call to CreateOffer.
(*sender)->internal()->set_stream_id(stream->label());
return;
}
// Normal case; we've never seen this track before.
rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender =
RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
signaling_thread(),
new AudioRtpSender(track, stream->label(), session_->voice_channel(),
stats_.get()));
senders_.push_back(new_sender);
// If the sender has already been configured in SDP, we call SetSsrc,
// which will connect the sender to the underlying transport. This can
// occur if a local session description that contains the ID of the sender
// is set before AddStream is called. It can also occur if the local
// session description is not changed and RemoveStream is called, and
// later AddStream is called again with the same stream.
const TrackInfo* track_info =
FindTrackInfo(local_audio_tracks_, stream->label(), track->id());
if (track_info) {
new_sender->internal()->SetSsrc(track_info->ssrc);
}
AddAudioTrack(track, stream);
observer_->OnRenegotiationNeeded();
}
// TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around
// indefinitely, when we have unified plan SDP.
void PeerConnection::OnAudioTrackRemoved(AudioTrackInterface* track,
MediaStreamInterface* stream) {
if (IsClosed()) {
return;
}
auto sender = FindSenderForTrack(track);
if (sender == senders_.end()) {
LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
<< " doesn't exist.";
return;
}
(*sender)->internal()->Stop();
senders_.erase(sender);
RemoveAudioTrack(track, stream);
observer_->OnRenegotiationNeeded();
}
void PeerConnection::OnVideoTrackAdded(VideoTrackInterface* track,
@ -1613,25 +1665,8 @@ void PeerConnection::OnVideoTrackAdded(VideoTrackInterface* track,
if (IsClosed()) {
return;
}
auto sender = FindSenderForTrack(track);
if (sender != senders_.end()) {
// We already have a sender for this track, so just change the stream_id
// so that it's correct in the next call to CreateOffer.
(*sender)->internal()->set_stream_id(stream->label());
return;
}
// Normal case; we've never seen this track before.
rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender =
RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
signaling_thread(), new VideoRtpSender(track, stream->label(),
session_->video_channel()));
senders_.push_back(new_sender);
const TrackInfo* track_info =
FindTrackInfo(local_video_tracks_, stream->label(), track->id());
if (track_info) {
new_sender->internal()->SetSsrc(track_info->ssrc);
}
AddVideoTrack(track, stream);
observer_->OnRenegotiationNeeded();
}
void PeerConnection::OnVideoTrackRemoved(VideoTrackInterface* track,
@ -1639,14 +1674,8 @@ void PeerConnection::OnVideoTrackRemoved(VideoTrackInterface* track,
if (IsClosed()) {
return;
}
auto sender = FindSenderForTrack(track);
if (sender == senders_.end()) {
LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
<< " doesn't exist.";
return;
}
(*sender)->internal()->Stop();
senders_.erase(sender);
RemoveVideoTrack(track, stream);
observer_->OnRenegotiationNeeded();
}
void PeerConnection::PostSetSessionDescriptionFailure(

14
webrtc/pc/peerconnection.h
View File

@ -193,11 +193,15 @@ class PeerConnection : public PeerConnectionInterface,
const std::string& track_id,
uint32_t ssrc);
void DestroyReceiver(const std::string& track_id);
void DestroyAudioSender(MediaStreamInterface* stream,
AudioTrackInterface* audio_track,
uint32_t ssrc);
void DestroyVideoSender(MediaStreamInterface* stream,
VideoTrackInterface* video_track);
// May be called either by AddStream/RemoveStream, or when a track is
// added/removed from a stream previously added via AddStream.
void AddAudioTrack(AudioTrackInterface* track, MediaStreamInterface* stream);
void RemoveAudioTrack(AudioTrackInterface* track,
MediaStreamInterface* stream);
void AddVideoTrack(VideoTrackInterface* track, MediaStreamInterface* stream);
void RemoveVideoTrack(VideoTrackInterface* track,
MediaStreamInterface* stream);
// Implements IceObserver
void OnIceConnectionStateChange(IceConnectionState new_state) override;

34
webrtc/pc/peerconnectioninterface_unittest.cc
View File

@ -3736,3 +3736,37 @@ TEST(RTCConfigurationTest, ComparisonOperators) {
PeerConnectionInterface::RTCConfigurationType::kAggressive);
EXPECT_NE(a, h);
}
// This test ensures OnRenegotiationNeeded is called when we add track with
// MediaStream -> AddTrack in the same way it is called when we add track with
// PeerConnection -> AddTrack.
// The test can be removed once addStream is rewritten in terms of addTrack
// https://bugs.chromium.org/p/webrtc/issues/detail?id=7815
TEST_F(PeerConnectionInterfaceTest, MediaStreamAddTrackRemoveTrackRenegotiate) {
CreatePeerConnectionWithoutDtls();
rtc::scoped_refptr<MediaStreamInterface> stream(
pc_factory_->CreateLocalMediaStream(kStreamLabel1));
pc_->AddStream(stream);
rtc::scoped_refptr<AudioTrackInterface> audio_track(
pc_factory_->CreateAudioTrack("audio_track", nullptr));
rtc::scoped_refptr<VideoTrackInterface> video_track(
pc_factory_->CreateVideoTrack(
"video_track", pc_factory_->CreateVideoSource(
std::unique_ptr<cricket::VideoCapturer>(
new cricket::FakeVideoCapturer()))));
stream->AddTrack(audio_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->AddTrack(video_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->RemoveTrack(audio_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
stream->RemoveTrack(video_track);
EXPECT_TRUE_WAIT(observer_.renegotiation_needed_, kTimeout);
observer_.renegotiation_needed_ = false;
}