From ac9d92ccbe2b29590c53f702e11dc625820480d5 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Mon, 26 Oct 2015 11:48:22 -0700 Subject: [PATCH] Adding the ability to create an RtpSender without a track. This CL also changes AddStream to immediately create a sender, rather than waiting until the track is seen in SDP. And the PeerConnection now builds the list of "send streams" from the list of senders, rather than the collection of local media streams. Review URL: https://codereview.webrtc.org/1413713003 Cr-Commit-Position: refs/heads/master@{#10414} --- talk/app/webrtc/audiotrack.cc | 2 +- talk/app/webrtc/mediastreaminterface.h | 3 + talk/app/webrtc/mediastreamprovider.h | 9 +- talk/app/webrtc/peerconnection.cc | 242 ++++++++---------- talk/app/webrtc/peerconnection.h | 11 +- talk/app/webrtc/peerconnection_unittest.cc | 89 ++++--- talk/app/webrtc/peerconnectioninterface.h | 6 + .../peerconnectioninterface_unittest.cc | 2 + talk/app/webrtc/peerconnectionproxy.h | 3 + talk/app/webrtc/rtpsender.cc | 206 +++++++++++---- talk/app/webrtc/rtpsender.h | 69 ++++- talk/app/webrtc/rtpsenderinterface.h | 20 ++ talk/app/webrtc/rtpsenderreceiver_unittest.cc | 213 ++++++++++++++- .../webrtc/test/fakemediastreamsignaling.h | 140 ---------- talk/app/webrtc/videotrack.cc | 2 +- talk/app/webrtc/webrtcsession.cc | 3 - talk/libjingle_tests.gyp | 1 - webrtc/base/helpers.cc | 59 ++++- webrtc/base/helpers.h | 3 + webrtc/base/helpers_unittest.cc | 7 + 20 files changed, 701 insertions(+), 389 deletions(-) delete mode 100644 talk/app/webrtc/test/fakemediastreamsignaling.h diff --git a/talk/app/webrtc/audiotrack.cc b/talk/app/webrtc/audiotrack.cc index b0c91296f9..28e9e2798f 100644 --- a/talk/app/webrtc/audiotrack.cc +++ b/talk/app/webrtc/audiotrack.cc @@ -31,7 +31,7 @@ namespace webrtc { -static const char kAudioTrackKind[] = "audio"; +const char MediaStreamTrackInterface::kAudioTrackKind[] = "audio"; AudioTrack::AudioTrack(const std::string& label, AudioSourceInterface* audio_source) diff --git a/talk/app/webrtc/mediastreaminterface.h b/talk/app/webrtc/mediastreaminterface.h index 5911e85e8e..cac1735465 100644 --- a/talk/app/webrtc/mediastreaminterface.h +++ b/talk/app/webrtc/mediastreaminterface.h @@ -100,6 +100,9 @@ class MediaStreamTrackInterface : public rtc::RefCountInterface, kFailed = 3, // Track negotiation failed. }; + static const char kAudioTrackKind[]; + static const char kVideoTrackKind[]; + virtual std::string kind() const = 0; virtual std::string id() const = 0; virtual bool enabled() const = 0; diff --git a/talk/app/webrtc/mediastreamprovider.h b/talk/app/webrtc/mediastreamprovider.h index 1c62daf9f1..a78b55a68c 100644 --- a/talk/app/webrtc/mediastreamprovider.h +++ b/talk/app/webrtc/mediastreamprovider.h @@ -50,8 +50,8 @@ namespace webrtc { // RtpSenders/Receivers to get to the BaseChannels. These interfaces should be // refactored away eventually, as the classes converge. -// This interface is called by AudioTrackHandler classes in mediastreamhandler.h -// to change the settings of an audio track connected to certain PeerConnection. +// This interface is called by AudioRtpSender/Receivers to change the settings +// of an audio track connected to certain PeerConnection. class AudioProviderInterface { public: // Enable/disable the audio playout of a remote audio track with |ssrc|. @@ -71,9 +71,8 @@ class AudioProviderInterface { virtual ~AudioProviderInterface() {} }; -// This interface is called by VideoTrackHandler classes in mediastreamhandler.h -// to change the settings of a video track connected to a certain -// PeerConnection. +// This interface is called by VideoRtpSender/Receivers to change the settings +// of a video track connected to a certain PeerConnection. class VideoProviderInterface { public: virtual bool SetCaptureDevice(uint32_t ssrc, diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc index fecc19ba0e..138da5b33a 100644 --- a/talk/app/webrtc/peerconnection.cc +++ b/talk/app/webrtc/peerconnection.cc @@ -59,6 +59,7 @@ using webrtc::DataChannel; using webrtc::MediaConstraintsInterface; using webrtc::MediaStreamInterface; using webrtc::PeerConnectionInterface; +using webrtc::RtpSenderInterface; using webrtc::StreamCollection; using webrtc::StunConfigurations; using webrtc::TurnConfigurations; @@ -365,25 +366,15 @@ bool IsValidOfferToReceiveMedia(int value) { } // Add the stream and RTP data channel info to |session_options|. -void SetStreams(cricket::MediaSessionOptions* session_options, - rtc::scoped_refptr streams, - const std::map>& - rtp_data_channels) { +void AddSendStreams( + cricket::MediaSessionOptions* session_options, + const std::vector>& senders, + const std::map>& + rtp_data_channels) { session_options->streams.clear(); - if (streams != nullptr) { - for (size_t i = 0; i < streams->count(); ++i) { - MediaStreamInterface* stream = streams->at(i); - // For each audio track in the stream, add it to the MediaSessionOptions. - for (const auto& track : stream->GetAudioTracks()) { - session_options->AddSendStream(cricket::MEDIA_TYPE_AUDIO, track->id(), - stream->label()); - } - // For each video track in the stream, add it to the MediaSessionOptions. - for (const auto& track : stream->GetVideoTracks()) { - session_options->AddSendStream(cricket::MEDIA_TYPE_VIDEO, track->id(), - stream->label()); - } - } + for (const auto& sender : senders) { + session_options->AddSendStream(sender->media_type(), sender->id(), + sender->stream_id()); } // Check for data channels. @@ -668,8 +659,6 @@ PeerConnection::remote_streams() { return remote_streams_; } -// TODO(deadbeef): Create RtpSenders immediately here, even if local -// description hasn't yet been set. bool PeerConnection::AddStream(MediaStreamInterface* local_stream) { if (IsClosed()) { return false; @@ -680,23 +669,46 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) { local_streams_->AddStream(local_stream); - // Find tracks that have already been configured in SDP. This can occur if a - // local session description that contains the MSID of these tracks is set - // before AddLocalStream is called. It can also occur if the local session - // description is not changed and RemoveLocalStream is called and later - // AddLocalStream is called again with the same stream. for (const auto& track : local_stream->GetAudioTracks()) { - const TrackInfo* track_info = - FindTrackInfo(local_audio_tracks_, local_stream->label(), track->id()); - if (track_info) { - CreateAudioSender(local_stream, track.get(), track_info->ssrc); + auto sender = FindSenderForTrack(track.get()); + if (sender == senders_.end()) { + // Normal case; we've never seen this track before. + AudioRtpSender* new_sender = new AudioRtpSender( + track.get(), local_stream->label(), session_.get(), 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_, local_stream->label(), track->id()); + if (track_info) { + new_sender->SetSsrc(track_info->ssrc); + } + } else { + // 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)->set_stream_id(local_stream->label()); } } for (const auto& track : local_stream->GetVideoTracks()) { - const TrackInfo* track_info = - FindTrackInfo(local_video_tracks_, local_stream->label(), track->id()); - if (track_info) { - CreateVideoSender(local_stream, track.get(), track_info->ssrc); + auto sender = FindSenderForTrack(track.get()); + if (sender == senders_.end()) { + // Normal case; we've never seen this track before. + VideoRtpSender* new_sender = new VideoRtpSender( + track.get(), local_stream->label(), session_.get()); + senders_.push_back(new_sender); + const TrackInfo* track_info = FindTrackInfo( + local_video_tracks_, local_stream->label(), track->id()); + if (track_info) { + new_sender->SetSsrc(track_info->ssrc); + } + } else { + // 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)->set_stream_id(local_stream->label()); } } @@ -706,21 +718,27 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) { } // TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around -// indefinitely. +// indefinitely, when we have unified plan SDP. void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) { for (const auto& track : local_stream->GetAudioTracks()) { - const TrackInfo* track_info = - FindTrackInfo(local_audio_tracks_, local_stream->label(), track->id()); - if (track_info) { - DestroyAudioSender(local_stream, track.get(), track_info->ssrc); + auto sender = FindSenderForTrack(track.get()); + if (sender == senders_.end()) { + LOG(LS_WARNING) << "RtpSender for track with id " << track->id() + << " doesn't exist."; + continue; } + (*sender)->Stop(); + senders_.erase(sender); } for (const auto& track : local_stream->GetVideoTracks()) { - const TrackInfo* track_info = - FindTrackInfo(local_video_tracks_, local_stream->label(), track->id()); - if (track_info) { - DestroyVideoSender(local_stream, track.get()); + auto sender = FindSenderForTrack(track.get()); + if (sender == senders_.end()) { + LOG(LS_WARNING) << "RtpSender for track with id " << track->id() + << " doesn't exist."; + continue; } + (*sender)->Stop(); + senders_.erase(sender); } local_streams_->RemoveStream(local_stream); @@ -751,6 +769,21 @@ rtc::scoped_refptr PeerConnection::CreateDtmfSender( return DtmfSenderProxy::Create(signaling_thread(), sender.get()); } +rtc::scoped_refptr PeerConnection::CreateSender( + const std::string& kind) { + RtpSenderInterface* new_sender; + if (kind == MediaStreamTrackInterface::kAudioTrackKind) { + new_sender = new AudioRtpSender(session_.get(), stats_.get()); + } else if (kind == MediaStreamTrackInterface::kVideoTrackKind) { + new_sender = new VideoRtpSender(session_.get()); + } else { + LOG(LS_ERROR) << "CreateSender called with invalid kind: " << kind; + return rtc::scoped_refptr(); + } + senders_.push_back(new_sender); + return RtpSenderProxy::Create(signaling_thread(), new_sender); +} + std::vector> PeerConnection::GetSenders() const { std::vector> senders; @@ -1267,49 +1300,6 @@ void PeerConnection::DestroyVideoReceiver(MediaStreamInterface* stream, } } -void PeerConnection::CreateAudioSender(MediaStreamInterface* stream, - AudioTrackInterface* audio_track, - uint32_t ssrc) { - senders_.push_back(new AudioRtpSender(audio_track, ssrc, session_.get())); - stats_->AddLocalAudioTrack(audio_track, ssrc); -} - -void PeerConnection::CreateVideoSender(MediaStreamInterface* stream, - VideoTrackInterface* video_track, - uint32_t ssrc) { - senders_.push_back(new VideoRtpSender(video_track, ssrc, session_.get())); -} - -// TODO(deadbeef): Keep RtpSenders around even if track goes away in local -// description. -void PeerConnection::DestroyAudioSender(MediaStreamInterface* stream, - AudioTrackInterface* audio_track, - uint32_t ssrc) { - auto it = FindSenderForTrack(audio_track); - if (it == senders_.end()) { - LOG(LS_WARNING) << "RtpSender for track with id " << audio_track->id() - << " doesn't exist."; - return; - } else { - (*it)->Stop(); - senders_.erase(it); - } - stats_->RemoveLocalAudioTrack(audio_track, ssrc); -} - -void PeerConnection::DestroyVideoSender(MediaStreamInterface* stream, - VideoTrackInterface* video_track) { - auto it = FindSenderForTrack(video_track); - if (it == senders_.end()) { - LOG(LS_WARNING) << "RtpSender for track with id " << video_track->id() - << " doesn't exist."; - return; - } else { - (*it)->Stop(); - senders_.erase(it); - } -} - void PeerConnection::OnIceConnectionChange( PeerConnectionInterface::IceConnectionState new_state) { RTC_DCHECK(signaling_thread()->IsCurrent()); @@ -1385,7 +1375,7 @@ bool PeerConnection::GetOptionsForOffer( return false; } - SetStreams(session_options, local_streams_, rtp_data_channels_); + AddSendStreams(session_options, senders_, rtp_data_channels_); // Offer to receive audio/video if the constraint is not set and there are // send streams, or we're currently receiving. if (rtc_options.offer_to_receive_audio == RTCOfferAnswerOptions::kUndefined) { @@ -1418,7 +1408,7 @@ bool PeerConnection::GetOptionsForAnswer( return false; } - SetStreams(session_options, local_streams_, rtp_data_channels_); + AddSendStreams(session_options, senders_, rtp_data_channels_); session_options->bundle_enabled = session_options->bundle_enabled && (session_options->has_audio() || session_options->has_video() || @@ -1440,8 +1430,7 @@ void PeerConnection::UpdateRemoteStreamsList( TrackInfos* current_tracks = GetRemoteTracks(media_type); // Find removed tracks. I.e., tracks where the track id or ssrc don't match - // the - // new StreamParam. + // the new StreamParam. auto track_it = current_tracks->begin(); while (track_it != current_tracks->end()) { const TrackInfo& info = *track_it; @@ -1643,62 +1632,44 @@ void PeerConnection::OnLocalTrackSeen(const std::string& stream_label, const std::string& track_id, uint32_t ssrc, cricket::MediaType media_type) { - MediaStreamInterface* stream = local_streams_->find(stream_label); - if (!stream) { - LOG(LS_WARNING) << "An unknown local MediaStream with label " - << stream_label << " has been configured."; + RtpSenderInterface* sender = FindSenderById(track_id); + if (!sender) { + LOG(LS_WARNING) << "An unknown RtpSender with id " << track_id + << " has been configured in the local description."; return; } - if (media_type == cricket::MEDIA_TYPE_AUDIO) { - AudioTrackInterface* audio_track = stream->FindAudioTrack(track_id); - if (!audio_track) { - LOG(LS_WARNING) << "An unknown local AudioTrack with id , " << track_id - << " has been configured."; - return; - } - CreateAudioSender(stream, audio_track, ssrc); - } else if (media_type == cricket::MEDIA_TYPE_VIDEO) { - VideoTrackInterface* video_track = stream->FindVideoTrack(track_id); - if (!video_track) { - LOG(LS_WARNING) << "An unknown local VideoTrack with id , " << track_id - << " has been configured."; - return; - } - CreateVideoSender(stream, video_track, ssrc); - } else { - RTC_DCHECK(false && "Invalid media type"); + if (sender->media_type() != media_type) { + LOG(LS_WARNING) << "An RtpSender has been configured in the local" + << " description with an unexpected media type."; + return; } + + sender->set_stream_id(stream_label); + sender->SetSsrc(ssrc); } void PeerConnection::OnLocalTrackRemoved(const std::string& stream_label, const std::string& track_id, uint32_t ssrc, cricket::MediaType media_type) { - MediaStreamInterface* stream = local_streams_->find(stream_label); - if (!stream) { - // This is the normal case. I.e., RemoveLocalStream has been called and the + RtpSenderInterface* sender = FindSenderById(track_id); + if (!sender) { + // This is the normal case. I.e., RemoveStream has been called and the // SessionDescriptions has been renegotiated. return; } - // A track has been removed from the SessionDescription but the MediaStream - // is still associated with PeerConnection. This only occurs if the SDP - // doesn't match with the calls to AddLocalStream and RemoveLocalStream. - if (media_type == cricket::MEDIA_TYPE_AUDIO) { - AudioTrackInterface* audio_track = stream->FindAudioTrack(track_id); - if (!audio_track) { - return; - } - DestroyAudioSender(stream, audio_track, ssrc); - } else if (media_type == cricket::MEDIA_TYPE_VIDEO) { - VideoTrackInterface* video_track = stream->FindVideoTrack(track_id); - if (!video_track) { - return; - } - DestroyVideoSender(stream, video_track); - } else { - RTC_DCHECK(false && "Invalid media type."); + + // A sender has been removed from the SessionDescription but it's still + // associated with the PeerConnection. This only occurs if the SDP doesn't + // match with the calls to CreateSender, AddStream and RemoveStream. + if (sender->media_type() != media_type) { + LOG(LS_WARNING) << "An RtpSender has been configured in the local" + << " description with an unexpected media type."; + return; } + + sender->SetSsrc(0); } void PeerConnection::UpdateLocalRtpDataChannels( @@ -1916,6 +1887,15 @@ void PeerConnection::OnDataChannelOpenMessage( DataChannelProxy::Create(signaling_thread(), channel)); } +RtpSenderInterface* PeerConnection::FindSenderById(const std::string& id) { + auto it = + std::find_if(senders_.begin(), senders_.end(), + [id](const rtc::scoped_refptr& sender) { + return sender->id() == id; + }); + return it != senders_.end() ? it->get() : nullptr; +} + std::vector>::iterator PeerConnection::FindSenderForTrack(MediaStreamTrackInterface* track) { return std::find_if( diff --git a/talk/app/webrtc/peerconnection.h b/talk/app/webrtc/peerconnection.h index 2d388ae9f9..c233388343 100644 --- a/talk/app/webrtc/peerconnection.h +++ b/talk/app/webrtc/peerconnection.h @@ -91,6 +91,9 @@ class PeerConnection : public PeerConnectionInterface, rtc::scoped_refptr CreateDtmfSender( AudioTrackInterface* track) override; + rtc::scoped_refptr CreateSender( + const std::string& kind) override; + std::vector> GetSenders() const override; std::vector> GetReceivers() @@ -187,12 +190,6 @@ class PeerConnection : public PeerConnectionInterface, AudioTrackInterface* audio_track); void DestroyVideoReceiver(MediaStreamInterface* stream, VideoTrackInterface* video_track); - void CreateAudioSender(MediaStreamInterface* stream, - AudioTrackInterface* audio_track, - uint32_t ssrc); - void CreateVideoSender(MediaStreamInterface* stream, - VideoTrackInterface* video_track, - uint32_t ssrc); void DestroyAudioSender(MediaStreamInterface* stream, AudioTrackInterface* audio_track, uint32_t ssrc); @@ -328,6 +325,8 @@ class PeerConnection : public PeerConnectionInterface, void OnDataChannelOpenMessage(const std::string& label, const InternalDataChannelInit& config); + RtpSenderInterface* FindSenderById(const std::string& id); + std::vector>::iterator FindSenderForTrack(MediaStreamTrackInterface* track); std::vector>::iterator diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc index 3cf66d64d8..abe5e1ceb6 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -96,6 +96,7 @@ static const int kMaxWaitMs = 10000; #if !defined(THREAD_SANITIZER) static const int kMaxWaitForStatsMs = 3000; #endif +static const int kMaxWaitForActivationMs = 5000; static const int kMaxWaitForFramesMs = 10000; static const int kEndAudioFrameCount = 3; static const int kEndVideoFrameCount = 3; @@ -250,18 +251,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, peer_connection_factory_->CreateLocalMediaStream(stream_label); if (audio && can_receive_audio()) { - FakeConstraints constraints; - // Disable highpass filter so that we can get all the test audio frames. - constraints.AddMandatory( - MediaConstraintsInterface::kHighpassFilter, false); - rtc::scoped_refptr source = - peer_connection_factory_->CreateAudioSource(&constraints); - // TODO(perkj): Test audio source when it is implemented. Currently audio - // always use the default input. - std::string label = stream_label + kAudioTrackLabelBase; - rtc::scoped_refptr audio_track( - peer_connection_factory_->CreateAudioTrack(label, source)); - stream->AddTrack(audio_track); + stream->AddTrack(CreateLocalAudioTrack(stream_label)); } if (video && can_receive_video()) { stream->AddTrack(CreateLocalVideoTrack(stream_label)); @@ -357,6 +347,35 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, data_observer_.reset(new MockDataChannelObserver(data_channel_)); } + rtc::scoped_refptr CreateLocalAudioTrack( + const std::string& stream_label) { + FakeConstraints constraints; + // Disable highpass filter so that we can get all the test audio frames. + constraints.AddMandatory(MediaConstraintsInterface::kHighpassFilter, false); + rtc::scoped_refptr source = + peer_connection_factory_->CreateAudioSource(&constraints); + // TODO(perkj): Test audio source when it is implemented. Currently audio + // always use the default input. + std::string label = stream_label + kAudioTrackLabelBase; + return peer_connection_factory_->CreateAudioTrack(label, source); + } + + rtc::scoped_refptr CreateLocalVideoTrack( + const std::string& stream_label) { + // Set max frame rate to 10fps to reduce the risk of the tests to be flaky. + FakeConstraints source_constraints = video_constraints_; + source_constraints.SetMandatoryMaxFrameRate(10); + + cricket::FakeVideoCapturer* fake_capturer = + new webrtc::FakePeriodicVideoCapturer(); + video_capturers_.push_back(fake_capturer); + rtc::scoped_refptr source = + peer_connection_factory_->CreateVideoSource(fake_capturer, + &source_constraints); + std::string label = stream_label + kVideoTrackLabelBase; + return peer_connection_factory_->CreateVideoTrack(label, source); + } + DataChannelInterface* data_channel() { return data_channel_; } const MockDataChannelObserver* data_observer() const { return data_observer_.get(); @@ -671,22 +690,6 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, return peer_connection_.get() != nullptr; } - rtc::scoped_refptr - CreateLocalVideoTrack(const std::string stream_label) { - // Set max frame rate to 10fps to reduce the risk of the tests to be flaky. - FakeConstraints source_constraints = video_constraints_; - source_constraints.SetMandatoryMaxFrameRate(10); - - cricket::FakeVideoCapturer* fake_capturer = - new webrtc::FakePeriodicVideoCapturer(); - video_capturers_.push_back(fake_capturer); - rtc::scoped_refptr source = - peer_connection_factory_->CreateVideoSource( - fake_capturer, &source_constraints); - std::string label = stream_label + kVideoTrackLabelBase; - return peer_connection_factory_->CreateVideoTrack(label, source); - } - rtc::scoped_refptr CreatePeerConnection( webrtc::PortAllocatorFactoryInterface* factory, const MediaConstraintsInterface* constraints) { @@ -957,13 +960,11 @@ class JsepPeerConnectionP2PTestClient : public testing::Test { initiating_client_->AddMediaStream(true, true); } initiating_client_->Negotiate(); - const int kMaxWaitForActivationMs = 5000; // Assert true is used here since next tests are guaranteed to fail and // would eat up 5 seconds. ASSERT_TRUE_WAIT(SessionActive(), kMaxWaitForActivationMs); VerifySessionDescriptions(); - int audio_frame_count = kEndAudioFrameCount; // TODO(ronghuawu): Add test to cover the case of sendonly and recvonly. if (!initiating_client_->can_receive_audio() || @@ -1573,6 +1574,34 @@ TEST_F(JsepPeerConnectionP2PTestClient, LocalP2PTest(); } +// This tests that if we negotiate after calling CreateSender but before we +// have a track, then set a track later, frames from the newly-set track are +// received end-to-end. +TEST_F(JsepPeerConnectionP2PTestClient, EarlyWarmupTest) { + ASSERT_TRUE(CreateTestClients()); + auto audio_sender = initializing_client()->pc()->CreateSender("audio"); + auto video_sender = initializing_client()->pc()->CreateSender("video"); + initializing_client()->Negotiate(); + // Wait for ICE connection to complete, without any tracks. + // Note that the receiving client WILL (in HandleIncomingOffer) create + // tracks, so it's only the initiator here that's doing early warmup. + ASSERT_TRUE_WAIT(SessionActive(), kMaxWaitForActivationMs); + VerifySessionDescriptions(); + EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted, + initializing_client()->ice_connection_state(), + kMaxWaitForFramesMs); + EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected, + receiving_client()->ice_connection_state(), + kMaxWaitForFramesMs); + // Now set the tracks, and expect frames to immediately start flowing. + EXPECT_TRUE( + audio_sender->SetTrack(initializing_client()->CreateLocalAudioTrack(""))); + EXPECT_TRUE( + video_sender->SetTrack(initializing_client()->CreateLocalVideoTrack(""))); + EXPECT_TRUE_WAIT(FramesNotPending(kEndAudioFrameCount, kEndVideoFrameCount), + kMaxWaitForFramesMs); +} + class IceServerParsingTest : public testing::Test { public: // Convenience for parsing a single URL. diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index 77caa9d78b..317e663052 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -337,6 +337,12 @@ class PeerConnectionInterface : public rtc::RefCountInterface { AudioTrackInterface* track) = 0; // TODO(deadbeef): Make these pure virtual once all subclasses implement them. + // |kind| must be "audio" or "video". + virtual rtc::scoped_refptr CreateSender( + const std::string& kind) { + return rtc::scoped_refptr(); + } + virtual std::vector> GetSenders() const { return std::vector>(); diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc index 63163fd651..df5c38b860 100644 --- a/talk/app/webrtc/peerconnectioninterface_unittest.cc +++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc @@ -2020,8 +2020,10 @@ TEST_F(PeerConnectionInterfaceTest, LocalDescriptionChanged) { EXPECT_TRUE(ContainsSender(senders, kVideoTracks[1])); // Remove an audio and video track. + pc_->RemoveStream(reference_collection_->at(0)); rtc::scoped_ptr desc_2; CreateSessionDescriptionAndReference(1, 1, desc_2.accept()); + pc_->AddStream(reference_collection_->at(0)); EXPECT_TRUE(DoSetLocalDescription(desc_2.release())); senders = pc_->GetSenders(); EXPECT_EQ(2u, senders.size()); diff --git a/talk/app/webrtc/peerconnectionproxy.h b/talk/app/webrtc/peerconnectionproxy.h index d207fbbdd8..9b446c057b 100644 --- a/talk/app/webrtc/peerconnectionproxy.h +++ b/talk/app/webrtc/peerconnectionproxy.h @@ -43,6 +43,9 @@ BEGIN_PROXY_MAP(PeerConnection) PROXY_METHOD1(void, RemoveStream, MediaStreamInterface*) PROXY_METHOD1(rtc::scoped_refptr, CreateDtmfSender, AudioTrackInterface*) + PROXY_METHOD1(rtc::scoped_refptr, + CreateSender, + const std::string&) PROXY_CONSTMETHOD0(std::vector>, GetSenders) PROXY_CONSTMETHOD0(std::vector>, diff --git a/talk/app/webrtc/rtpsender.cc b/talk/app/webrtc/rtpsender.cc index 3a78f4598a..5b3ad49398 100644 --- a/talk/app/webrtc/rtpsender.cc +++ b/talk/app/webrtc/rtpsender.cc @@ -29,6 +29,7 @@ #include "talk/app/webrtc/localaudiosource.h" #include "talk/app/webrtc/videosourceinterface.h" +#include "webrtc/base/helpers.h" namespace webrtc { @@ -59,34 +60,49 @@ void LocalAudioSinkAdapter::SetSink(cricket::AudioRenderer::Sink* sink) { } AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, - uint32_t ssrc, - AudioProviderInterface* provider) + const std::string& stream_id, + AudioProviderInterface* provider, + StatsCollector* stats) : id_(track->id()), - track_(track), - ssrc_(ssrc), + stream_id_(stream_id), provider_(provider), + stats_(stats), + track_(track), cached_track_enabled_(track->enabled()), sink_adapter_(new LocalAudioSinkAdapter()) { + RTC_DCHECK(provider != nullptr); track_->RegisterObserver(this); track_->AddSink(sink_adapter_.get()); - Reconfigure(); } +AudioRtpSender::AudioRtpSender(AudioProviderInterface* provider, + StatsCollector* stats) + : id_(rtc::CreateRandomUuid()), + stream_id_(rtc::CreateRandomUuid()), + provider_(provider), + stats_(stats), + sink_adapter_(new LocalAudioSinkAdapter()) {} + AudioRtpSender::~AudioRtpSender() { - track_->RemoveSink(sink_adapter_.get()); - track_->UnregisterObserver(this); Stop(); } void AudioRtpSender::OnChanged() { + RTC_DCHECK(!stopped_); if (cached_track_enabled_ != track_->enabled()) { cached_track_enabled_ = track_->enabled(); - Reconfigure(); + if (can_send_track()) { + SetAudioSend(); + } } } bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) { - if (track->kind() != "audio") { + if (stopped_) { + LOG(LS_ERROR) << "SetTrack can't be called on a stopped RtpSender."; + return false; + } + if (track && track->kind() != MediaStreamTrackInterface::kAudioTrackKind) { LOG(LS_ERROR) << "SetTrack called on audio RtpSender with " << track->kind() << " track."; return false; @@ -94,36 +110,83 @@ bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) { AudioTrackInterface* audio_track = static_cast(track); // Detach from old track. - track_->RemoveSink(sink_adapter_.get()); - track_->UnregisterObserver(this); + if (track_) { + track_->RemoveSink(sink_adapter_.get()); + track_->UnregisterObserver(this); + } + + if (can_send_track() && stats_) { + stats_->RemoveLocalAudioTrack(track_.get(), ssrc_); + } // Attach to new track. + bool prev_can_send_track = can_send_track(); track_ = audio_track; - cached_track_enabled_ = track_->enabled(); - track_->RegisterObserver(this); - track_->AddSink(sink_adapter_.get()); - Reconfigure(); + if (track_) { + cached_track_enabled_ = track_->enabled(); + track_->RegisterObserver(this); + track_->AddSink(sink_adapter_.get()); + } + + // Update audio provider. + if (can_send_track()) { + SetAudioSend(); + if (stats_) { + stats_->AddLocalAudioTrack(track_.get(), ssrc_); + } + } else if (prev_can_send_track) { + cricket::AudioOptions options; + provider_->SetAudioSend(ssrc_, false, options, nullptr); + } return true; } +void AudioRtpSender::SetSsrc(uint32_t ssrc) { + if (stopped_ || ssrc == ssrc_) { + return; + } + // If we are already sending with a particular SSRC, stop sending. + if (can_send_track()) { + cricket::AudioOptions options; + provider_->SetAudioSend(ssrc_, false, options, nullptr); + if (stats_) { + stats_->RemoveLocalAudioTrack(track_.get(), ssrc_); + } + } + ssrc_ = ssrc; + if (can_send_track()) { + SetAudioSend(); + if (stats_) { + stats_->AddLocalAudioTrack(track_.get(), ssrc_); + } + } +} + void AudioRtpSender::Stop() { // TODO(deadbeef): Need to do more here to fully stop sending packets. - if (!provider_) { + if (stopped_) { return; } - cricket::AudioOptions options; - provider_->SetAudioSend(ssrc_, false, options, nullptr); - provider_ = nullptr; + if (track_) { + track_->RemoveSink(sink_adapter_.get()); + track_->UnregisterObserver(this); + } + if (can_send_track()) { + cricket::AudioOptions options; + provider_->SetAudioSend(ssrc_, false, options, nullptr); + if (stats_) { + stats_->RemoveLocalAudioTrack(track_.get(), ssrc_); + } + } + stopped_ = true; } -void AudioRtpSender::Reconfigure() { - if (!provider_) { - return; - } +void AudioRtpSender::SetAudioSend() { + RTC_DCHECK(!stopped_ && can_send_track()); cricket::AudioOptions options; if (track_->enabled() && track_->GetSource()) { // TODO(xians): Remove this static_cast since we should be able to connect - // a remote audio track to peer connection. + // a remote audio track to a peer connection. options = static_cast(track_->GetSource())->options(); } @@ -136,35 +199,42 @@ void AudioRtpSender::Reconfigure() { } VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, - uint32_t ssrc, + const std::string& stream_id, VideoProviderInterface* provider) : id_(track->id()), - track_(track), - ssrc_(ssrc), + stream_id_(stream_id), provider_(provider), + track_(track), cached_track_enabled_(track->enabled()) { + RTC_DCHECK(provider != nullptr); track_->RegisterObserver(this); - VideoSourceInterface* source = track_->GetSource(); - if (source) { - provider_->SetCaptureDevice(ssrc_, source->GetVideoCapturer()); - } - Reconfigure(); } +VideoRtpSender::VideoRtpSender(VideoProviderInterface* provider) + : id_(rtc::CreateRandomUuid()), + stream_id_(rtc::CreateRandomUuid()), + provider_(provider) {} + VideoRtpSender::~VideoRtpSender() { - track_->UnregisterObserver(this); Stop(); } void VideoRtpSender::OnChanged() { + RTC_DCHECK(!stopped_); if (cached_track_enabled_ != track_->enabled()) { cached_track_enabled_ = track_->enabled(); - Reconfigure(); + if (can_send_track()) { + SetVideoSend(); + } } } bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { - if (track->kind() != "video") { + if (stopped_) { + LOG(LS_ERROR) << "SetTrack can't be called on a stopped RtpSender."; + return false; + } + if (track && track->kind() != MediaStreamTrackInterface::kVideoTrackKind) { LOG(LS_ERROR) << "SetTrack called on video RtpSender with " << track->kind() << " track."; return false; @@ -172,30 +242,72 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { VideoTrackInterface* video_track = static_cast(track); // Detach from old track. - track_->UnregisterObserver(this); + if (track_) { + track_->UnregisterObserver(this); + } // Attach to new track. + bool prev_can_send_track = can_send_track(); track_ = video_track; - cached_track_enabled_ = track_->enabled(); - track_->RegisterObserver(this); - Reconfigure(); + if (track_) { + cached_track_enabled_ = track_->enabled(); + track_->RegisterObserver(this); + } + + // Update video provider. + if (can_send_track()) { + VideoSourceInterface* source = track_->GetSource(); + // TODO(deadbeef): If SetTrack is called with a disabled track, and the + // previous track was enabled, this could cause a frame from the new track + // to slip out. Really, what we need is for SetCaptureDevice and + // SetVideoSend + // to be combined into one atomic operation, all the way down to + // WebRtcVideoSendStream. + provider_->SetCaptureDevice(ssrc_, + source ? source->GetVideoCapturer() : nullptr); + SetVideoSend(); + } else if (prev_can_send_track) { + provider_->SetCaptureDevice(ssrc_, nullptr); + provider_->SetVideoSend(ssrc_, false, nullptr); + } return true; } +void VideoRtpSender::SetSsrc(uint32_t ssrc) { + if (stopped_ || ssrc == ssrc_) { + return; + } + // If we are already sending with a particular SSRC, stop sending. + if (can_send_track()) { + provider_->SetCaptureDevice(ssrc_, nullptr); + provider_->SetVideoSend(ssrc_, false, nullptr); + } + ssrc_ = ssrc; + if (can_send_track()) { + VideoSourceInterface* source = track_->GetSource(); + provider_->SetCaptureDevice(ssrc_, + source ? source->GetVideoCapturer() : nullptr); + SetVideoSend(); + } +} + void VideoRtpSender::Stop() { // TODO(deadbeef): Need to do more here to fully stop sending packets. - if (!provider_) { + if (stopped_) { return; } - provider_->SetCaptureDevice(ssrc_, nullptr); - provider_->SetVideoSend(ssrc_, false, nullptr); - provider_ = nullptr; + if (track_) { + track_->UnregisterObserver(this); + } + if (can_send_track()) { + provider_->SetCaptureDevice(ssrc_, nullptr); + provider_->SetVideoSend(ssrc_, false, nullptr); + } + stopped_ = true; } -void VideoRtpSender::Reconfigure() { - if (!provider_) { - return; - } +void VideoRtpSender::SetVideoSend() { + RTC_DCHECK(!stopped_ && can_send_track()); const cricket::VideoOptions* options = nullptr; VideoSourceInterface* source = track_->GetSource(); if (track_->enabled() && source) { diff --git a/talk/app/webrtc/rtpsender.h b/talk/app/webrtc/rtpsender.h index 3741909323..d5f88a941a 100644 --- a/talk/app/webrtc/rtpsender.h +++ b/talk/app/webrtc/rtpsender.h @@ -36,6 +36,7 @@ #include "talk/app/webrtc/mediastreamprovider.h" #include "talk/app/webrtc/rtpsenderinterface.h" +#include "talk/app/webrtc/statscollector.h" #include "talk/media/base/audiorenderer.h" #include "webrtc/base/basictypes.h" #include "webrtc/base/criticalsection.h" @@ -70,9 +71,15 @@ class LocalAudioSinkAdapter : public AudioTrackSinkInterface, class AudioRtpSender : public ObserverInterface, public rtc::RefCountedObject { public: + // StatsCollector provided so that Add/RemoveLocalAudioTrack can be called + // at the appropriate times. AudioRtpSender(AudioTrackInterface* track, - uint32_t ssrc, - AudioProviderInterface* provider); + const std::string& stream_id, + AudioProviderInterface* provider, + StatsCollector* stats); + + // Randomly generates id and stream_id. + AudioRtpSender(AudioProviderInterface* provider, StatsCollector* stats); virtual ~AudioRtpSender(); @@ -85,18 +92,37 @@ class AudioRtpSender : public ObserverInterface, return track_.get(); } + void SetSsrc(uint32_t ssrc) override; + + uint32_t ssrc() const override { return ssrc_; } + + cricket::MediaType media_type() const override { + return cricket::MEDIA_TYPE_AUDIO; + } + std::string id() const override { return id_; } + void set_stream_id(const std::string& stream_id) override { + stream_id_ = stream_id; + } + std::string stream_id() const override { return stream_id_; } + void Stop() override; private: - void Reconfigure(); + bool can_send_track() const { return track_ && ssrc_; } + // Helper function to construct options for + // AudioProviderInterface::SetAudioSend. + void SetAudioSend(); std::string id_; - rtc::scoped_refptr track_; - uint32_t ssrc_; + std::string stream_id_; AudioProviderInterface* provider_; - bool cached_track_enabled_; + StatsCollector* stats_; + rtc::scoped_refptr track_; + uint32_t ssrc_ = 0; + bool cached_track_enabled_ = false; + bool stopped_ = false; // Used to pass the data callback from the |track_| to the other end of // cricket::AudioRenderer. @@ -107,9 +133,12 @@ class VideoRtpSender : public ObserverInterface, public rtc::RefCountedObject { public: VideoRtpSender(VideoTrackInterface* track, - uint32_t ssrc, + const std::string& stream_id, VideoProviderInterface* provider); + // Randomly generates id and stream_id. + explicit VideoRtpSender(VideoProviderInterface* provider); + virtual ~VideoRtpSender(); // ObserverInterface implementation @@ -121,18 +150,36 @@ class VideoRtpSender : public ObserverInterface, return track_.get(); } + void SetSsrc(uint32_t ssrc) override; + + uint32_t ssrc() const override { return ssrc_; } + + cricket::MediaType media_type() const override { + return cricket::MEDIA_TYPE_VIDEO; + } + std::string id() const override { return id_; } + void set_stream_id(const std::string& stream_id) override { + stream_id_ = stream_id; + } + std::string stream_id() const override { return stream_id_; } + void Stop() override; private: - void Reconfigure(); + bool can_send_track() const { return track_ && ssrc_; } + // Helper function to construct options for + // VideoProviderInterface::SetVideoSend. + void SetVideoSend(); std::string id_; - rtc::scoped_refptr track_; - uint32_t ssrc_; + std::string stream_id_; VideoProviderInterface* provider_; - bool cached_track_enabled_; + rtc::scoped_refptr track_; + uint32_t ssrc_ = 0; + bool cached_track_enabled_ = false; + bool stopped_ = false; }; } // namespace webrtc diff --git a/talk/app/webrtc/rtpsenderinterface.h b/talk/app/webrtc/rtpsenderinterface.h index fca98f21db..f54e8ca090 100644 --- a/talk/app/webrtc/rtpsenderinterface.h +++ b/talk/app/webrtc/rtpsenderinterface.h @@ -35,6 +35,7 @@ #include "talk/app/webrtc/proxy.h" #include "talk/app/webrtc/mediastreaminterface.h" +#include "talk/session/media/mediasession.h" #include "webrtc/base/refcount.h" #include "webrtc/base/scoped_ref_ptr.h" @@ -47,10 +48,24 @@ class RtpSenderInterface : public rtc::RefCountInterface { virtual bool SetTrack(MediaStreamTrackInterface* track) = 0; virtual rtc::scoped_refptr track() const = 0; + // Used to set the SSRC of the sender, once a local description has been set. + // If |ssrc| is 0, this indiates that the sender should disconnect from the + // underlying transport (this occurs if the sender isn't seen in a local + // description). + virtual void SetSsrc(uint32_t ssrc) = 0; + virtual uint32_t ssrc() const = 0; + + // Audio or video sender? + virtual cricket::MediaType media_type() const = 0; + // Not to be confused with "mid", this is a field we can temporarily use // to uniquely identify a receiver until we implement Unified Plan SDP. virtual std::string id() const = 0; + // TODO(deadbeef): Support one sender having multiple stream ids. + virtual void set_stream_id(const std::string& stream_id) = 0; + virtual std::string stream_id() const = 0; + virtual void Stop() = 0; protected: @@ -61,7 +76,12 @@ class RtpSenderInterface : public rtc::RefCountInterface { BEGIN_PROXY_MAP(RtpSender) PROXY_METHOD1(bool, SetTrack, MediaStreamTrackInterface*) PROXY_CONSTMETHOD0(rtc::scoped_refptr, track) +PROXY_METHOD1(void, SetSsrc, uint32_t) +PROXY_CONSTMETHOD0(uint32_t, ssrc) +PROXY_CONSTMETHOD0(cricket::MediaType, media_type) PROXY_CONSTMETHOD0(std::string, id) +PROXY_METHOD1(void, set_stream_id, const std::string&) +PROXY_CONSTMETHOD0(std::string, stream_id) PROXY_METHOD0(void, Stop) END_PROXY() diff --git a/talk/app/webrtc/rtpsenderreceiver_unittest.cc b/talk/app/webrtc/rtpsenderreceiver_unittest.cc index c9d7e008c3..c9871864bd 100644 --- a/talk/app/webrtc/rtpsenderreceiver_unittest.cc +++ b/talk/app/webrtc/rtpsenderreceiver_unittest.cc @@ -48,7 +48,9 @@ static const char kStreamLabel1[] = "local_stream_1"; static const char kVideoTrackId[] = "video_1"; static const char kAudioTrackId[] = "audio_1"; static const uint32_t kVideoSsrc = 98; +static const uint32_t kVideoSsrc2 = 100; static const uint32_t kAudioSsrc = 99; +static const uint32_t kAudioSsrc2 = 101; namespace webrtc { @@ -120,8 +122,10 @@ class RtpSenderReceiverTest : public testing::Test { audio_track_ = AudioTrack::Create(kAudioTrackId, NULL); EXPECT_TRUE(stream_->AddTrack(audio_track_)); EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, true, _, _)); - audio_rtp_sender_ = new AudioRtpSender(stream_->GetAudioTracks()[0], - kAudioSsrc, &audio_provider_); + audio_rtp_sender_ = + new AudioRtpSender(stream_->GetAudioTracks()[0], stream_->label(), + &audio_provider_, nullptr); + audio_rtp_sender_->SetSsrc(kAudioSsrc); } void CreateVideoRtpSender() { @@ -130,7 +134,8 @@ class RtpSenderReceiverTest : public testing::Test { kVideoSsrc, video_track_->GetSource()->GetVideoCapturer())); EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); video_rtp_sender_ = new VideoRtpSender(stream_->GetVideoTracks()[0], - kVideoSsrc, &video_provider_); + stream_->label(), &video_provider_); + video_rtp_sender_->SetSsrc(kVideoSsrc); } void DestroyAudioRtpSender() { @@ -280,4 +285,206 @@ TEST_F(RtpSenderReceiverTest, RemoteAudioTrackSetVolume) { DestroyAudioRtpReceiver(); } +// Test that provider methods aren't called without both a track and an SSRC. +TEST_F(RtpSenderReceiverTest, AudioSenderWithoutTrackAndSsrc) { + rtc::scoped_refptr sender = + new AudioRtpSender(&audio_provider_, nullptr); + rtc::scoped_refptr track = + AudioTrack::Create(kAudioTrackId, nullptr); + EXPECT_TRUE(sender->SetTrack(track)); + EXPECT_TRUE(sender->SetTrack(nullptr)); + sender->SetSsrc(kAudioSsrc); + sender->SetSsrc(0); + // Just let it get destroyed and make sure it doesn't call any methods on the + // provider interface. +} + +// Test that provider methods aren't called without both a track and an SSRC. +TEST_F(RtpSenderReceiverTest, VideoSenderWithoutTrackAndSsrc) { + rtc::scoped_refptr sender = + new VideoRtpSender(&video_provider_); + EXPECT_TRUE(sender->SetTrack(video_track_)); + EXPECT_TRUE(sender->SetTrack(nullptr)); + sender->SetSsrc(kVideoSsrc); + sender->SetSsrc(0); + // Just let it get destroyed and make sure it doesn't call any methods on the + // provider interface. +} + +// Test that an audio sender calls the expected methods on the provider once +// it has a track and SSRC, when the SSRC is set first. +TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupSsrcThenTrack) { + rtc::scoped_refptr sender = + new AudioRtpSender(&audio_provider_, nullptr); + rtc::scoped_refptr track = + AudioTrack::Create(kAudioTrackId, nullptr); + sender->SetSsrc(kAudioSsrc); + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, true, _, _)); + sender->SetTrack(track); + + // Calls expected from destructor. + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, false, _, _)).Times(1); +} + +// Test that an audio sender calls the expected methods on the provider once +// it has a track and SSRC, when the SSRC is set last. +TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupTrackThenSsrc) { + rtc::scoped_refptr sender = + new AudioRtpSender(&audio_provider_, nullptr); + rtc::scoped_refptr track = + AudioTrack::Create(kAudioTrackId, nullptr); + sender->SetTrack(track); + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, true, _, _)); + sender->SetSsrc(kAudioSsrc); + + // Calls expected from destructor. + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, false, _, _)).Times(1); +} + +// Test that a video sender calls the expected methods on the provider once +// it has a track and SSRC, when the SSRC is set first. +TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupSsrcThenTrack) { + rtc::scoped_refptr sender = + new VideoRtpSender(&video_provider_); + sender->SetSsrc(kVideoSsrc); + EXPECT_CALL(video_provider_, + SetCaptureDevice(kVideoSsrc, + video_track_->GetSource()->GetVideoCapturer())); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + sender->SetTrack(video_track_); + + // Calls expected from destructor. + EXPECT_CALL(video_provider_, SetCaptureDevice(kVideoSsrc, nullptr)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); +} + +// Test that a video sender calls the expected methods on the provider once +// it has a track and SSRC, when the SSRC is set last. +TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupTrackThenSsrc) { + rtc::scoped_refptr sender = + new VideoRtpSender(&video_provider_); + sender->SetTrack(video_track_); + EXPECT_CALL(video_provider_, + SetCaptureDevice(kVideoSsrc, + video_track_->GetSource()->GetVideoCapturer())); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + sender->SetSsrc(kVideoSsrc); + + // Calls expected from destructor. + EXPECT_CALL(video_provider_, SetCaptureDevice(kVideoSsrc, nullptr)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); +} + +// Test that the sender is disconnected from the provider when its SSRC is +// set to 0. +TEST_F(RtpSenderReceiverTest, AudioSenderSsrcSetToZero) { + rtc::scoped_refptr track = + AudioTrack::Create(kAudioTrackId, nullptr); + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, true, _, _)); + rtc::scoped_refptr sender = + new AudioRtpSender(track, kStreamLabel1, &audio_provider_, nullptr); + sender->SetSsrc(kAudioSsrc); + + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, false, _, _)).Times(1); + sender->SetSsrc(0); + + // Make sure it's SetSsrc that called methods on the provider, and not the + // destructor. + EXPECT_CALL(audio_provider_, SetAudioSend(_, _, _, _)).Times(0); +} + +// Test that the sender is disconnected from the provider when its SSRC is +// set to 0. +TEST_F(RtpSenderReceiverTest, VideoSenderSsrcSetToZero) { + EXPECT_CALL(video_provider_, + SetCaptureDevice(kVideoSsrc, + video_track_->GetSource()->GetVideoCapturer())); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + rtc::scoped_refptr sender = + new VideoRtpSender(video_track_, kStreamLabel1, &video_provider_); + sender->SetSsrc(kVideoSsrc); + + EXPECT_CALL(video_provider_, SetCaptureDevice(kVideoSsrc, nullptr)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); + sender->SetSsrc(0); + + // Make sure it's SetSsrc that called methods on the provider, and not the + // destructor. + EXPECT_CALL(video_provider_, SetCaptureDevice(_, _)).Times(0); + EXPECT_CALL(video_provider_, SetVideoSend(_, _, _)).Times(0); +} + +TEST_F(RtpSenderReceiverTest, AudioSenderTrackSetToNull) { + rtc::scoped_refptr track = + AudioTrack::Create(kAudioTrackId, nullptr); + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, true, _, _)); + rtc::scoped_refptr sender = + new AudioRtpSender(track, kStreamLabel1, &audio_provider_, nullptr); + sender->SetSsrc(kAudioSsrc); + + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, false, _, _)).Times(1); + EXPECT_TRUE(sender->SetTrack(nullptr)); + + // Make sure it's SetTrack that called methods on the provider, and not the + // destructor. + EXPECT_CALL(audio_provider_, SetAudioSend(_, _, _, _)).Times(0); +} + +TEST_F(RtpSenderReceiverTest, VideoSenderTrackSetToNull) { + EXPECT_CALL(video_provider_, + SetCaptureDevice(kVideoSsrc, + video_track_->GetSource()->GetVideoCapturer())); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + rtc::scoped_refptr sender = + new VideoRtpSender(video_track_, kStreamLabel1, &video_provider_); + sender->SetSsrc(kVideoSsrc); + + EXPECT_CALL(video_provider_, SetCaptureDevice(kVideoSsrc, nullptr)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); + EXPECT_TRUE(sender->SetTrack(nullptr)); + + // Make sure it's SetTrack that called methods on the provider, and not the + // destructor. + EXPECT_CALL(video_provider_, SetCaptureDevice(_, _)).Times(0); + EXPECT_CALL(video_provider_, SetVideoSend(_, _, _)).Times(0); +} + +TEST_F(RtpSenderReceiverTest, AudioSenderSsrcChanged) { + rtc::scoped_refptr track = + AudioTrack::Create(kAudioTrackId, nullptr); + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, true, _, _)); + rtc::scoped_refptr sender = + new AudioRtpSender(track, kStreamLabel1, &audio_provider_, nullptr); + sender->SetSsrc(kAudioSsrc); + + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, false, _, _)).Times(1); + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc2, true, _, _)).Times(1); + sender->SetSsrc(kAudioSsrc2); + + // Calls expected from destructor. + EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc2, false, _, _)).Times(1); +} + +TEST_F(RtpSenderReceiverTest, VideoSenderSsrcChanged) { + EXPECT_CALL(video_provider_, + SetCaptureDevice(kVideoSsrc, + video_track_->GetSource()->GetVideoCapturer())); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + rtc::scoped_refptr sender = + new VideoRtpSender(video_track_, kStreamLabel1, &video_provider_); + sender->SetSsrc(kVideoSsrc); + + EXPECT_CALL(video_provider_, SetCaptureDevice(kVideoSsrc, nullptr)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); + EXPECT_CALL(video_provider_, + SetCaptureDevice(kVideoSsrc2, + video_track_->GetSource()->GetVideoCapturer())); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc2, true, _)); + sender->SetSsrc(kVideoSsrc2); + + // Calls expected from destructor. + EXPECT_CALL(video_provider_, SetCaptureDevice(kVideoSsrc2, nullptr)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc2, false, _)).Times(1); +} + } // namespace webrtc diff --git a/talk/app/webrtc/test/fakemediastreamsignaling.h b/talk/app/webrtc/test/fakemediastreamsignaling.h deleted file mode 100644 index 562c4ad306..0000000000 --- a/talk/app/webrtc/test/fakemediastreamsignaling.h +++ /dev/null @@ -1,140 +0,0 @@ -/* - * libjingle - * Copyright 2013 Google Inc. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. The name of the author may not be used to endorse or promote products - * derived from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED - * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO - * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, - * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; - * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, - * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR - * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF - * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#ifndef TALK_APP_WEBRTC_TEST_FAKEMEDIASTREAMSIGNALING_H_ -#define TALK_APP_WEBRTC_TEST_FAKEMEDIASTREAMSIGNALING_H_ - -#include "talk/app/webrtc/audiotrack.h" -#include "talk/app/webrtc/mediastreamsignaling.h" -#include "talk/app/webrtc/videotrack.h" - -static const char kStream1[] = "stream1"; -static const char kVideoTrack1[] = "video1"; -static const char kAudioTrack1[] = "audio1"; - -static const char kStream2[] = "stream2"; -static const char kVideoTrack2[] = "video2"; -static const char kAudioTrack2[] = "audio2"; - -class FakeMediaStreamSignaling : public webrtc::MediaStreamSignaling, - public webrtc::MediaStreamSignalingObserver { - public: - explicit FakeMediaStreamSignaling(cricket::ChannelManager* channel_manager) : - webrtc::MediaStreamSignaling(rtc::Thread::Current(), this, - channel_manager) { - } - - void SendAudioVideoStream1() { - ClearLocalStreams(); - AddLocalStream(CreateStream(kStream1, kAudioTrack1, kVideoTrack1)); - } - - void SendAudioVideoStream2() { - ClearLocalStreams(); - AddLocalStream(CreateStream(kStream2, kAudioTrack2, kVideoTrack2)); - } - - void SendAudioVideoStream1And2() { - ClearLocalStreams(); - AddLocalStream(CreateStream(kStream1, kAudioTrack1, kVideoTrack1)); - AddLocalStream(CreateStream(kStream2, kAudioTrack2, kVideoTrack2)); - } - - void SendNothing() { - ClearLocalStreams(); - } - - void UseOptionsAudioOnly() { - ClearLocalStreams(); - AddLocalStream(CreateStream(kStream2, kAudioTrack2, "")); - } - - void UseOptionsVideoOnly() { - ClearLocalStreams(); - AddLocalStream(CreateStream(kStream2, "", kVideoTrack2)); - } - - void ClearLocalStreams() { - while (local_streams()->count() != 0) { - RemoveLocalStream(local_streams()->at(0)); - } - } - - // Implements MediaStreamSignalingObserver. - virtual void OnAddRemoteStream(webrtc::MediaStreamInterface* stream) {} - virtual void OnRemoveRemoteStream(webrtc::MediaStreamInterface* stream) {} - virtual void OnAddDataChannel(webrtc::DataChannelInterface* data_channel) {} - virtual void OnAddLocalAudioTrack(webrtc::MediaStreamInterface* stream, - webrtc::AudioTrackInterface* audio_track, - uint32_t ssrc) {} - virtual void OnAddLocalVideoTrack(webrtc::MediaStreamInterface* stream, - webrtc::VideoTrackInterface* video_track, - uint32_t ssrc) {} - virtual void OnAddRemoteAudioTrack(webrtc::MediaStreamInterface* stream, - webrtc::AudioTrackInterface* audio_track, - uint32_t ssrc) {} - virtual void OnAddRemoteVideoTrack(webrtc::MediaStreamInterface* stream, - webrtc::VideoTrackInterface* video_track, - uint32_t ssrc) {} - virtual void OnRemoveRemoteAudioTrack( - webrtc::MediaStreamInterface* stream, - webrtc::AudioTrackInterface* audio_track) {} - virtual void OnRemoveRemoteVideoTrack( - webrtc::MediaStreamInterface* stream, - webrtc::VideoTrackInterface* video_track) {} - virtual void OnRemoveLocalAudioTrack(webrtc::MediaStreamInterface* stream, - webrtc::AudioTrackInterface* audio_track, - uint32_t ssrc) {} - virtual void OnRemoveLocalVideoTrack( - webrtc::MediaStreamInterface* stream, - webrtc::VideoTrackInterface* video_track) {} - virtual void OnRemoveLocalStream(webrtc::MediaStreamInterface* stream) {} - - private: - rtc::scoped_refptr CreateStream( - const std::string& stream_label, - const std::string& audio_track_id, - const std::string& video_track_id) { - rtc::scoped_refptr stream( - webrtc::MediaStream::Create(stream_label)); - - if (!audio_track_id.empty()) { - rtc::scoped_refptr audio_track( - webrtc::AudioTrack::Create(audio_track_id, NULL)); - stream->AddTrack(audio_track); - } - - if (!video_track_id.empty()) { - rtc::scoped_refptr video_track( - webrtc::VideoTrack::Create(video_track_id, NULL)); - stream->AddTrack(video_track); - } - return stream; - } -}; - -#endif // TALK_APP_WEBRTC_TEST_FAKEMEDIASTREAMSIGNALING_H_ diff --git a/talk/app/webrtc/videotrack.cc b/talk/app/webrtc/videotrack.cc index 7c78aea91f..322273579b 100644 --- a/talk/app/webrtc/videotrack.cc +++ b/talk/app/webrtc/videotrack.cc @@ -31,7 +31,7 @@ namespace webrtc { -static const char kVideoTrackKind[] = "video"; +const char MediaStreamTrackInterface::kVideoTrackKind[] = "video"; VideoTrack::VideoTrack(const std::string& label, VideoSourceInterface* video_source) diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 95abeab77a..24454dc188 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -1747,7 +1747,6 @@ void WebRtcSession::RemoveUnusedChannels(const SessionDescription* desc) { cricket::GetFirstVideoContent(desc); if ((!video_info || video_info->rejected) && video_channel_) { SignalVideoChannelDestroyed(); - const std::string content_name = video_channel_->content_name(); channel_manager_->DestroyVideoChannel(video_channel_.release()); } @@ -1755,7 +1754,6 @@ void WebRtcSession::RemoveUnusedChannels(const SessionDescription* desc) { cricket::GetFirstAudioContent(desc); if ((!voice_info || voice_info->rejected) && voice_channel_) { SignalVoiceChannelDestroyed(); - const std::string content_name = voice_channel_->content_name(); channel_manager_->DestroyVoiceChannel(voice_channel_.release()); } @@ -1763,7 +1761,6 @@ void WebRtcSession::RemoveUnusedChannels(const SessionDescription* desc) { cricket::GetFirstDataContent(desc); if ((!data_info || data_info->rejected) && data_channel_) { SignalDataChannelDestroyed(); - const std::string content_name = data_channel_->content_name(); channel_manager_->DestroyDataChannel(data_channel_.release()); } } diff --git a/talk/libjingle_tests.gyp b/talk/libjingle_tests.gyp index 41b38b345d..a8bf57dd08 100755 --- a/talk/libjingle_tests.gyp +++ b/talk/libjingle_tests.gyp @@ -215,7 +215,6 @@ 'app/webrtc/test/fakeconstraints.h', 'app/webrtc/test/fakedatachannelprovider.h', 'app/webrtc/test/fakedtlsidentitystore.h', - 'app/webrtc/test/fakemediastreamsignaling.h', 'app/webrtc/test/fakeperiodicvideocapturer.h', 'app/webrtc/test/fakevideotrackrenderer.h', 'app/webrtc/test/mockpeerconnectionobservers.h', diff --git a/webrtc/base/helpers.cc b/webrtc/base/helpers.cc index 8e59b6410c..1ad5d0e12b 100644 --- a/webrtc/base/helpers.cc +++ b/webrtc/base/helpers.cc @@ -164,17 +164,21 @@ class TestRandomGenerator : public RandomGenerator { int seed_; }; -// TODO: Use Base64::Base64Table instead. -static const char BASE64[64] = { - 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', - 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', - 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', - 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', - '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '+', '/' -}; - namespace { +// TODO: Use Base64::Base64Table instead. +static const char kBase64[64] = { + 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', + 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', + 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '+', '/'}; + +static const char kHex[16] = {'0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'}; + +static const char kUuidDigit17[4] = {'8', '9', 'a', 'b'}; + // This round about way of creating a global RNG is to safe-guard against // indeterminant static initialization order. scoped_ptr& GetGlobalRng() { @@ -232,7 +236,7 @@ bool CreateRandomString(size_t len, } bool CreateRandomString(size_t len, std::string* str) { - return CreateRandomString(len, BASE64, 64, str); + return CreateRandomString(len, kBase64, 64, str); } bool CreateRandomString(size_t len, const std::string& table, @@ -241,6 +245,41 @@ bool CreateRandomString(size_t len, const std::string& table, static_cast(table.size()), str); } +// Version 4 UUID is of the form: +// xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx +// Where 'x' is a hex digit, and 'y' is 8, 9, a or b. +std::string CreateRandomUuid() { + std::string str; + scoped_ptr bytes(new uint8_t[31]); + if (!Rng().Generate(bytes.get(), 31)) { + LOG(LS_ERROR) << "Failed to generate random string!"; + return str; + } + str.reserve(36); + for (size_t i = 0; i < 8; ++i) { + str.push_back(kHex[bytes[i] % 16]); + } + str.push_back('-'); + for (size_t i = 8; i < 12; ++i) { + str.push_back(kHex[bytes[i] % 16]); + } + str.push_back('-'); + str.push_back('4'); + for (size_t i = 12; i < 15; ++i) { + str.push_back(kHex[bytes[i] % 16]); + } + str.push_back('-'); + str.push_back(kUuidDigit17[bytes[15] % 4]); + for (size_t i = 16; i < 19; ++i) { + str.push_back(kHex[bytes[i] % 16]); + } + str.push_back('-'); + for (size_t i = 19; i < 31; ++i) { + str.push_back(kHex[bytes[i] % 16]); + } + return str; +} + uint32_t CreateRandomId() { uint32_t id; if (!Rng().Generate(&id, sizeof(id))) { diff --git a/webrtc/base/helpers.h b/webrtc/base/helpers.h index 102c08bd0d..0e7937362a 100644 --- a/webrtc/base/helpers.h +++ b/webrtc/base/helpers.h @@ -39,6 +39,9 @@ bool CreateRandomString(size_t length, std::string* str); bool CreateRandomString(size_t length, const std::string& table, std::string* str); +// Generates a (cryptographically) random UUID version 4 string. +std::string CreateRandomUuid(); + // Generates a random id. uint32_t CreateRandomId(); diff --git a/webrtc/base/helpers_unittest.cc b/webrtc/base/helpers_unittest.cc index 6ea0167e98..83cc685919 100644 --- a/webrtc/base/helpers_unittest.cc +++ b/webrtc/base/helpers_unittest.cc @@ -43,16 +43,23 @@ TEST_F(RandomTest, TestCreateRandomString) { EXPECT_EQ(256U, random2.size()); } +TEST_F(RandomTest, TestCreateRandomUuid) { + std::string random = CreateRandomUuid(); + EXPECT_EQ(36U, random.size()); +} + TEST_F(RandomTest, TestCreateRandomForTest) { // Make sure we get the output we expect. SetRandomTestMode(true); EXPECT_EQ(2154761789U, CreateRandomId()); EXPECT_EQ("h0ISP4S5SJKH/9EY", CreateRandomString(16)); + EXPECT_EQ("41706e92-cdd3-46d9-a22d-8ff1737ffb11", CreateRandomUuid()); // Reset and make sure we get the same output. SetRandomTestMode(true); EXPECT_EQ(2154761789U, CreateRandomId()); EXPECT_EQ("h0ISP4S5SJKH/9EY", CreateRandomString(16)); + EXPECT_EQ("41706e92-cdd3-46d9-a22d-8ff1737ffb11", CreateRandomUuid()); // Test different character sets. SetRandomTestMode(true);