diff --git a/talk/app/webrtc/audiotrack.cc b/talk/app/webrtc/audiotrack.cc index 28e9e2798f..b0c91296f9 100644 --- a/talk/app/webrtc/audiotrack.cc +++ b/talk/app/webrtc/audiotrack.cc @@ -31,7 +31,7 @@ namespace webrtc { -const char MediaStreamTrackInterface::kAudioTrackKind[] = "audio"; +static const char 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 cac1735465..5911e85e8e 100644 --- a/talk/app/webrtc/mediastreaminterface.h +++ b/talk/app/webrtc/mediastreaminterface.h @@ -100,9 +100,6 @@ 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 a78b55a68c..1c62daf9f1 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 AudioRtpSender/Receivers to change the settings -// of an audio track connected to certain PeerConnection. +// This interface is called by AudioTrackHandler classes in mediastreamhandler.h +// 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,8 +71,9 @@ class AudioProviderInterface { virtual ~AudioProviderInterface() {} }; -// This interface is called by VideoRtpSender/Receivers to change the settings -// of a video track connected to a certain PeerConnection. +// This interface is called by VideoTrackHandler classes in mediastreamhandler.h +// 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 138da5b33a..fecc19ba0e 100644 --- a/talk/app/webrtc/peerconnection.cc +++ b/talk/app/webrtc/peerconnection.cc @@ -59,7 +59,6 @@ using webrtc::DataChannel; using webrtc::MediaConstraintsInterface; using webrtc::MediaStreamInterface; using webrtc::PeerConnectionInterface; -using webrtc::RtpSenderInterface; using webrtc::StreamCollection; using webrtc::StunConfigurations; using webrtc::TurnConfigurations; @@ -366,15 +365,25 @@ bool IsValidOfferToReceiveMedia(int value) { } // Add the stream and RTP data channel info to |session_options|. -void AddSendStreams( - cricket::MediaSessionOptions* session_options, - const std::vector>& senders, - const std::map>& - rtp_data_channels) { +void SetStreams(cricket::MediaSessionOptions* session_options, + rtc::scoped_refptr streams, + const std::map>& + rtp_data_channels) { session_options->streams.clear(); - for (const auto& sender : senders) { - session_options->AddSendStream(sender->media_type(), sender->id(), - sender->stream_id()); + 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()); + } + } } // Check for data channels. @@ -659,6 +668,8 @@ 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; @@ -669,46 +680,23 @@ 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()) { - 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()); + const TrackInfo* track_info = + FindTrackInfo(local_audio_tracks_, local_stream->label(), track->id()); + if (track_info) { + CreateAudioSender(local_stream, track.get(), track_info->ssrc); } } for (const auto& track : local_stream->GetVideoTracks()) { - 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()); + const TrackInfo* track_info = + FindTrackInfo(local_video_tracks_, local_stream->label(), track->id()); + if (track_info) { + CreateVideoSender(local_stream, track.get(), track_info->ssrc); } } @@ -718,27 +706,21 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) { } // TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around -// indefinitely, when we have unified plan SDP. +// indefinitely. void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) { for (const auto& track : local_stream->GetAudioTracks()) { - auto sender = FindSenderForTrack(track.get()); - if (sender == senders_.end()) { - LOG(LS_WARNING) << "RtpSender for track with id " << track->id() - << " doesn't exist."; - continue; + const TrackInfo* track_info = + FindTrackInfo(local_audio_tracks_, local_stream->label(), track->id()); + if (track_info) { + DestroyAudioSender(local_stream, track.get(), track_info->ssrc); } - (*sender)->Stop(); - senders_.erase(sender); } for (const auto& track : local_stream->GetVideoTracks()) { - auto sender = FindSenderForTrack(track.get()); - if (sender == senders_.end()) { - LOG(LS_WARNING) << "RtpSender for track with id " << track->id() - << " doesn't exist."; - continue; + const TrackInfo* track_info = + FindTrackInfo(local_video_tracks_, local_stream->label(), track->id()); + if (track_info) { + DestroyVideoSender(local_stream, track.get()); } - (*sender)->Stop(); - senders_.erase(sender); } local_streams_->RemoveStream(local_stream); @@ -769,21 +751,6 @@ 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; @@ -1300,6 +1267,49 @@ 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()); @@ -1375,7 +1385,7 @@ bool PeerConnection::GetOptionsForOffer( return false; } - AddSendStreams(session_options, senders_, rtp_data_channels_); + SetStreams(session_options, local_streams_, 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) { @@ -1408,7 +1418,7 @@ bool PeerConnection::GetOptionsForAnswer( return false; } - AddSendStreams(session_options, senders_, rtp_data_channels_); + SetStreams(session_options, local_streams_, rtp_data_channels_); session_options->bundle_enabled = session_options->bundle_enabled && (session_options->has_audio() || session_options->has_video() || @@ -1430,7 +1440,8 @@ 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; @@ -1632,44 +1643,62 @@ void PeerConnection::OnLocalTrackSeen(const std::string& stream_label, const std::string& track_id, uint32_t ssrc, cricket::MediaType media_type) { - RtpSenderInterface* sender = FindSenderById(track_id); - if (!sender) { - LOG(LS_WARNING) << "An unknown RtpSender with id " << track_id - << " has been configured in the local description."; + MediaStreamInterface* stream = local_streams_->find(stream_label); + if (!stream) { + LOG(LS_WARNING) << "An unknown local MediaStream with label " + << stream_label << " has been configured."; return; } - if (sender->media_type() != media_type) { - LOG(LS_WARNING) << "An RtpSender has been configured in the local" - << " description with an unexpected media type."; - 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"); } - - 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) { - RtpSenderInterface* sender = FindSenderById(track_id); - if (!sender) { - // This is the normal case. I.e., RemoveStream has been called and the + MediaStreamInterface* stream = local_streams_->find(stream_label); + if (!stream) { + // This is the normal case. I.e., RemoveLocalStream has been called and the // SessionDescriptions has been renegotiated. return; } - - // 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; + // 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."); } - - sender->SetSsrc(0); } void PeerConnection::UpdateLocalRtpDataChannels( @@ -1887,15 +1916,6 @@ 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 c233388343..2d388ae9f9 100644 --- a/talk/app/webrtc/peerconnection.h +++ b/talk/app/webrtc/peerconnection.h @@ -91,9 +91,6 @@ 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() @@ -190,6 +187,12 @@ 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); @@ -325,8 +328,6 @@ 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 abe5e1ceb6..3cf66d64d8 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -96,7 +96,6 @@ 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; @@ -251,7 +250,18 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, peer_connection_factory_->CreateLocalMediaStream(stream_label); if (audio && can_receive_audio()) { - stream->AddTrack(CreateLocalAudioTrack(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; + rtc::scoped_refptr audio_track( + peer_connection_factory_->CreateAudioTrack(label, source)); + stream->AddTrack(audio_track); } if (video && can_receive_video()) { stream->AddTrack(CreateLocalVideoTrack(stream_label)); @@ -347,35 +357,6 @@ 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(); @@ -690,6 +671,22 @@ 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) { @@ -960,11 +957,13 @@ 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() || @@ -1574,34 +1573,6 @@ 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 317e663052..77caa9d78b 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -337,12 +337,6 @@ 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 df5c38b860..63163fd651 100644 --- a/talk/app/webrtc/peerconnectioninterface_unittest.cc +++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc @@ -2020,10 +2020,8 @@ 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 9b446c057b..d207fbbdd8 100644 --- a/talk/app/webrtc/peerconnectionproxy.h +++ b/talk/app/webrtc/peerconnectionproxy.h @@ -43,9 +43,6 @@ 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 5b3ad49398..3a78f4598a 100644 --- a/talk/app/webrtc/rtpsender.cc +++ b/talk/app/webrtc/rtpsender.cc @@ -29,7 +29,6 @@ #include "talk/app/webrtc/localaudiosource.h" #include "talk/app/webrtc/videosourceinterface.h" -#include "webrtc/base/helpers.h" namespace webrtc { @@ -60,49 +59,34 @@ void LocalAudioSinkAdapter::SetSink(cricket::AudioRenderer::Sink* sink) { } AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, - const std::string& stream_id, - AudioProviderInterface* provider, - StatsCollector* stats) + uint32_t ssrc, + AudioProviderInterface* provider) : id_(track->id()), - stream_id_(stream_id), - provider_(provider), - stats_(stats), track_(track), + ssrc_(ssrc), + provider_(provider), 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(); - if (can_send_track()) { - SetAudioSend(); - } + Reconfigure(); } } bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) { - if (stopped_) { - LOG(LS_ERROR) << "SetTrack can't be called on a stopped RtpSender."; - return false; - } - if (track && track->kind() != MediaStreamTrackInterface::kAudioTrackKind) { + if (track->kind() != "audio") { LOG(LS_ERROR) << "SetTrack called on audio RtpSender with " << track->kind() << " track."; return false; @@ -110,83 +94,36 @@ bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) { AudioTrackInterface* audio_track = static_cast(track); // Detach from old track. - if (track_) { - track_->RemoveSink(sink_adapter_.get()); - track_->UnregisterObserver(this); - } - - if (can_send_track() && stats_) { - stats_->RemoveLocalAudioTrack(track_.get(), ssrc_); - } + track_->RemoveSink(sink_adapter_.get()); + track_->UnregisterObserver(this); // Attach to new track. - bool prev_can_send_track = can_send_track(); track_ = audio_track; - 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); - } + cached_track_enabled_ = track_->enabled(); + track_->RegisterObserver(this); + track_->AddSink(sink_adapter_.get()); + Reconfigure(); 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 (stopped_) { + if (!provider_) { return; } - 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; + cricket::AudioOptions options; + provider_->SetAudioSend(ssrc_, false, options, nullptr); + provider_ = nullptr; } -void AudioRtpSender::SetAudioSend() { - RTC_DCHECK(!stopped_ && can_send_track()); +void AudioRtpSender::Reconfigure() { + if (!provider_) { + return; + } 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 a peer connection. + // a remote audio track to peer connection. options = static_cast(track_->GetSource())->options(); } @@ -199,42 +136,35 @@ void AudioRtpSender::SetAudioSend() { } VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, - const std::string& stream_id, + uint32_t ssrc, VideoProviderInterface* provider) : id_(track->id()), - stream_id_(stream_id), - provider_(provider), track_(track), + ssrc_(ssrc), + provider_(provider), 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(); - if (can_send_track()) { - SetVideoSend(); - } + Reconfigure(); } } bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { - if (stopped_) { - LOG(LS_ERROR) << "SetTrack can't be called on a stopped RtpSender."; - return false; - } - if (track && track->kind() != MediaStreamTrackInterface::kVideoTrackKind) { + if (track->kind() != "video") { LOG(LS_ERROR) << "SetTrack called on video RtpSender with " << track->kind() << " track."; return false; @@ -242,72 +172,30 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { VideoTrackInterface* video_track = static_cast(track); // Detach from old track. - if (track_) { - track_->UnregisterObserver(this); - } + track_->UnregisterObserver(this); // Attach to new track. - bool prev_can_send_track = can_send_track(); track_ = video_track; - 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); - } + cached_track_enabled_ = track_->enabled(); + track_->RegisterObserver(this); + Reconfigure(); 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 (stopped_) { + if (!provider_) { return; } - if (track_) { - track_->UnregisterObserver(this); - } - if (can_send_track()) { - provider_->SetCaptureDevice(ssrc_, nullptr); - provider_->SetVideoSend(ssrc_, false, nullptr); - } - stopped_ = true; + provider_->SetCaptureDevice(ssrc_, nullptr); + provider_->SetVideoSend(ssrc_, false, nullptr); + provider_ = nullptr; } -void VideoRtpSender::SetVideoSend() { - RTC_DCHECK(!stopped_ && can_send_track()); +void VideoRtpSender::Reconfigure() { + if (!provider_) { + return; + } 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 d5f88a941a..3741909323 100644 --- a/talk/app/webrtc/rtpsender.h +++ b/talk/app/webrtc/rtpsender.h @@ -36,7 +36,6 @@ #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" @@ -71,15 +70,9 @@ 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, - const std::string& stream_id, - AudioProviderInterface* provider, - StatsCollector* stats); - - // Randomly generates id and stream_id. - AudioRtpSender(AudioProviderInterface* provider, StatsCollector* stats); + uint32_t ssrc, + AudioProviderInterface* provider); virtual ~AudioRtpSender(); @@ -92,37 +85,18 @@ 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: - bool can_send_track() const { return track_ && ssrc_; } - // Helper function to construct options for - // AudioProviderInterface::SetAudioSend. - void SetAudioSend(); + void Reconfigure(); std::string id_; - std::string stream_id_; - AudioProviderInterface* provider_; - StatsCollector* stats_; rtc::scoped_refptr track_; - uint32_t ssrc_ = 0; - bool cached_track_enabled_ = false; - bool stopped_ = false; + uint32_t ssrc_; + AudioProviderInterface* provider_; + bool cached_track_enabled_; // Used to pass the data callback from the |track_| to the other end of // cricket::AudioRenderer. @@ -133,12 +107,9 @@ class VideoRtpSender : public ObserverInterface, public rtc::RefCountedObject { public: VideoRtpSender(VideoTrackInterface* track, - const std::string& stream_id, + uint32_t ssrc, VideoProviderInterface* provider); - // Randomly generates id and stream_id. - explicit VideoRtpSender(VideoProviderInterface* provider); - virtual ~VideoRtpSender(); // ObserverInterface implementation @@ -150,36 +121,18 @@ 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: - bool can_send_track() const { return track_ && ssrc_; } - // Helper function to construct options for - // VideoProviderInterface::SetVideoSend. - void SetVideoSend(); + void Reconfigure(); std::string id_; - std::string stream_id_; - VideoProviderInterface* provider_; rtc::scoped_refptr track_; - uint32_t ssrc_ = 0; - bool cached_track_enabled_ = false; - bool stopped_ = false; + uint32_t ssrc_; + VideoProviderInterface* provider_; + bool cached_track_enabled_; }; } // namespace webrtc diff --git a/talk/app/webrtc/rtpsenderinterface.h b/talk/app/webrtc/rtpsenderinterface.h index f54e8ca090..fca98f21db 100644 --- a/talk/app/webrtc/rtpsenderinterface.h +++ b/talk/app/webrtc/rtpsenderinterface.h @@ -35,7 +35,6 @@ #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" @@ -48,24 +47,10 @@ 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: @@ -76,12 +61,7 @@ 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 c9871864bd..c9d7e008c3 100644 --- a/talk/app/webrtc/rtpsenderreceiver_unittest.cc +++ b/talk/app/webrtc/rtpsenderreceiver_unittest.cc @@ -48,9 +48,7 @@ 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 { @@ -122,10 +120,8 @@ 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], stream_->label(), - &audio_provider_, nullptr); - audio_rtp_sender_->SetSsrc(kAudioSsrc); + audio_rtp_sender_ = new AudioRtpSender(stream_->GetAudioTracks()[0], + kAudioSsrc, &audio_provider_); } void CreateVideoRtpSender() { @@ -134,8 +130,7 @@ 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], - stream_->label(), &video_provider_); - video_rtp_sender_->SetSsrc(kVideoSsrc); + kVideoSsrc, &video_provider_); } void DestroyAudioRtpSender() { @@ -285,206 +280,4 @@ 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 new file mode 100644 index 0000000000..562c4ad306 --- /dev/null +++ b/talk/app/webrtc/test/fakemediastreamsignaling.h @@ -0,0 +1,140 @@ +/* + * 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 322273579b..7c78aea91f 100644 --- a/talk/app/webrtc/videotrack.cc +++ b/talk/app/webrtc/videotrack.cc @@ -31,7 +31,7 @@ namespace webrtc { -const char MediaStreamTrackInterface::kVideoTrackKind[] = "video"; +static const char 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 24454dc188..95abeab77a 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -1747,6 +1747,7 @@ 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()); } @@ -1754,6 +1755,7 @@ 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()); } @@ -1761,6 +1763,7 @@ 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 a8bf57dd08..41b38b345d 100755 --- a/talk/libjingle_tests.gyp +++ b/talk/libjingle_tests.gyp @@ -215,6 +215,7 @@ '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 1ad5d0e12b..8e59b6410c 100644 --- a/webrtc/base/helpers.cc +++ b/webrtc/base/helpers.cc @@ -164,20 +164,16 @@ class TestRandomGenerator : public RandomGenerator { int seed_; }; -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 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', '+', '/' +}; -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'}; +namespace { // This round about way of creating a global RNG is to safe-guard against // indeterminant static initialization order. @@ -236,7 +232,7 @@ bool CreateRandomString(size_t len, } bool CreateRandomString(size_t len, std::string* str) { - return CreateRandomString(len, kBase64, 64, str); + return CreateRandomString(len, BASE64, 64, str); } bool CreateRandomString(size_t len, const std::string& table, @@ -245,41 +241,6 @@ 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 0e7937362a..102c08bd0d 100644 --- a/webrtc/base/helpers.h +++ b/webrtc/base/helpers.h @@ -39,9 +39,6 @@ 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 83cc685919..6ea0167e98 100644 --- a/webrtc/base/helpers_unittest.cc +++ b/webrtc/base/helpers_unittest.cc @@ -43,23 +43,16 @@ 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);