diff --git a/ortc/ortcrtpsenderadapter.cc b/ortc/ortcrtpsenderadapter.cc index 869a7cd314..306db64a57 100644 --- a/ortc/ortcrtpsenderadapter.cc +++ b/ortc/ortcrtpsenderadapter.cc @@ -155,14 +155,18 @@ OrtcRtpSenderAdapter::OrtcRtpSenderAdapter( void OrtcRtpSenderAdapter::CreateInternalSender() { switch (kind_) { - case cricket::MEDIA_TYPE_AUDIO: - internal_sender_ = new AudioRtpSender( - rtp_transport_controller_->voice_channel(), nullptr); + case cricket::MEDIA_TYPE_AUDIO: { + auto* audio_sender = new AudioRtpSender(nullptr); + audio_sender->SetChannel(rtp_transport_controller_->voice_channel()); + internal_sender_ = audio_sender; break; - case cricket::MEDIA_TYPE_VIDEO: - internal_sender_ = - new VideoRtpSender(rtp_transport_controller_->video_channel()); + } + case cricket::MEDIA_TYPE_VIDEO: { + auto* video_sender = new VideoRtpSender(); + video_sender->SetChannel(rtp_transport_controller_->video_channel()); + internal_sender_ = video_sender; break; + } case cricket::MEDIA_TYPE_DATA: RTC_NOTREACHED(); } diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 3bab2e1d49..d6b441426e 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1066,36 +1066,34 @@ RTCErrorOr> PeerConnection::AddTrackPlanB( rtc::scoped_refptr track, const std::vector& stream_labels) { + cricket::MediaType media_type = + (track->kind() == MediaStreamTrackInterface::kAudioKind + ? cricket::MEDIA_TYPE_AUDIO + : cricket::MEDIA_TYPE_VIDEO); + auto new_sender = CreateSender(media_type, track, stream_labels); if (track->kind() == MediaStreamTrackInterface::kAudioKind) { - auto new_sender = RtpSenderProxyWithInternal::Create( - signaling_thread(), - new AudioRtpSender(static_cast(track.get()), - voice_channel(), stats_.get())); + static_cast(new_sender->internal()) + ->SetChannel(voice_channel()); GetAudioTransceiver()->internal()->AddSender(new_sender); - new_sender->internal()->set_stream_ids(stream_labels); const RtpSenderInfo* sender_info = FindSenderInfo(local_audio_sender_infos_, new_sender->internal()->stream_id(), track->id()); if (sender_info) { new_sender->internal()->SetSsrc(sender_info->first_ssrc); } - return rtc::scoped_refptr(new_sender); } else { RTC_DCHECK_EQ(MediaStreamTrackInterface::kVideoKind, track->kind()); - auto new_sender = RtpSenderProxyWithInternal::Create( - signaling_thread(), - new VideoRtpSender(static_cast(track.get()), - video_channel())); + static_cast(new_sender->internal()) + ->SetChannel(video_channel()); GetVideoTransceiver()->internal()->AddSender(new_sender); - new_sender->internal()->set_stream_ids(stream_labels); const RtpSenderInfo* sender_info = FindSenderInfo(local_video_sender_infos_, new_sender->internal()->stream_id(), track->id()); if (sender_info) { new_sender->internal()->SetSsrc(sender_info->first_ssrc); } - return rtc::scoped_refptr(new_sender); } + return rtc::scoped_refptr(new_sender); } RTCErrorOr> @@ -1109,17 +1107,19 @@ PeerConnection::AddTrackUnifiedPlan( } else if (transceiver->direction() == RtpTransceiverDirection::kInactive) { transceiver->SetDirection(RtpTransceiverDirection::kSendOnly); } + transceiver->sender()->SetTrack(track); + transceiver->internal()->sender_internal()->set_stream_ids(stream_labels); } else { cricket::MediaType media_type = (track->kind() == MediaStreamTrackInterface::kAudioKind ? cricket::MEDIA_TYPE_AUDIO : cricket::MEDIA_TYPE_VIDEO); - transceiver = CreateTransceiver(media_type); + auto sender = CreateSender(media_type, track, stream_labels); + auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid()); + transceiver = CreateAndAddTransceiver(sender, receiver); transceiver->internal()->set_created_by_addtrack(true); transceiver->SetDirection(RtpTransceiverDirection::kSendRecv); } - transceiver->sender()->SetTrack(track); - transceiver->internal()->sender_internal()->set_stream_ids(stream_labels); return transceiver->sender(); } @@ -1259,43 +1259,76 @@ PeerConnection::AddTransceiver( // TODO(bugs.webrtc.org/7600): Verify init. - auto transceiver = CreateTransceiver(media_type); - transceiver->SetDirection(init.direction); - if (track) { - transceiver->sender()->SetTrack(track); + if (init.stream_labels.size() > 1u) { + LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_PARAMETER, + "More than one stream is not yet supported."); } + std::vector stream_labels = {!init.stream_labels.empty() + ? init.stream_labels[0] + : rtc::CreateRandomUuid()}; + + auto sender = CreateSender(media_type, track, stream_labels); + auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid()); + auto transceiver = CreateAndAddTransceiver(sender, receiver); + transceiver->internal()->set_direction(init.direction); + observer_->OnRenegotiationNeeded(); return rtc::scoped_refptr(transceiver); } -rtc::scoped_refptr> -PeerConnection::CreateTransceiver(cricket::MediaType media_type) { +rtc::scoped_refptr> +PeerConnection::CreateSender( + cricket::MediaType media_type, + rtc::scoped_refptr track, + const std::vector& stream_labels) { rtc::scoped_refptr> sender; + if (media_type == cricket::MEDIA_TYPE_AUDIO) { + RTC_DCHECK(!track || + (track->kind() == MediaStreamTrackInterface::kAudioKind)); + sender = RtpSenderProxyWithInternal::Create( + signaling_thread(), + new AudioRtpSender(static_cast(track.get()), + stream_labels, stats_.get())); + } else { + RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO); + RTC_DCHECK(!track || + (track->kind() == MediaStreamTrackInterface::kVideoKind)); + sender = RtpSenderProxyWithInternal::Create( + signaling_thread(), + new VideoRtpSender(static_cast(track.get()), + stream_labels)); + } + sender->internal()->set_stream_ids(stream_labels); + return sender; +} + +rtc::scoped_refptr> +PeerConnection::CreateReceiver(cricket::MediaType media_type, + const std::string& receiver_id) { rtc::scoped_refptr> receiver; - std::string receiver_id = rtc::CreateRandomUuid(); - // TODO(bugs.webrtc.org/7600): Initializing the sender/receiver with a null - // channel prevents users from calling SetParameters on them, which is needed - // to be in compliance with the spec. if (media_type == cricket::MEDIA_TYPE_AUDIO) { - sender = RtpSenderProxyWithInternal::Create( - signaling_thread(), new AudioRtpSender(nullptr, stats_.get())); receiver = RtpReceiverProxyWithInternal::Create( signaling_thread(), new AudioRtpReceiver(worker_thread(), receiver_id, {}, 0, nullptr)); } else { - RTC_DCHECK_EQ(cricket::MEDIA_TYPE_VIDEO, media_type); - sender = RtpSenderProxyWithInternal::Create( - signaling_thread(), new VideoRtpSender(nullptr)); + RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO); receiver = RtpReceiverProxyWithInternal::Create( signaling_thread(), new VideoRtpReceiver(worker_thread(), receiver_id, {}, 0, nullptr)); } - rtc::scoped_refptr> - transceiver = RtpTransceiverProxyWithInternal::Create( - signaling_thread(), new RtpTransceiver(sender, receiver)); + return receiver; +} + +rtc::scoped_refptr> +PeerConnection::CreateAndAddTransceiver( + rtc::scoped_refptr> sender, + rtc::scoped_refptr> + receiver) { + auto transceiver = RtpTransceiverProxyWithInternal::Create( + signaling_thread(), new RtpTransceiver(sender, receiver)); transceivers_.push_back(transceiver); return transceiver; } @@ -1327,25 +1360,31 @@ rtc::scoped_refptr PeerConnection::CreateSender( return nullptr; } + std::vector stream_labels; + if (!stream_id.empty()) { + stream_labels.push_back(stream_id); + } + // TODO(steveanton): Move construction of the RtpSenders to RtpTransceiver. rtc::scoped_refptr> new_sender; if (kind == MediaStreamTrackInterface::kAudioKind) { + auto* audio_sender = + new AudioRtpSender(nullptr, stream_labels, stats_.get()); + audio_sender->SetChannel(voice_channel()); new_sender = RtpSenderProxyWithInternal::Create( - signaling_thread(), new AudioRtpSender(voice_channel(), stats_.get())); + signaling_thread(), audio_sender); GetAudioTransceiver()->internal()->AddSender(new_sender); } else if (kind == MediaStreamTrackInterface::kVideoKind) { + auto* video_sender = new VideoRtpSender(nullptr, stream_labels); + video_sender->SetChannel(video_channel()); new_sender = RtpSenderProxyWithInternal::Create( - signaling_thread(), new VideoRtpSender(video_channel())); + signaling_thread(), video_sender); GetVideoTransceiver()->internal()->AddSender(new_sender); } else { RTC_LOG(LS_ERROR) << "CreateSender called with invalid kind: " << kind; return nullptr; } - if (!stream_id.empty()) { - new_sender->internal()->set_stream_id(stream_id); - } - return new_sender; } @@ -2223,7 +2262,11 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, // If no RtpTransceiver was found in the previous step, create one with a // recvonly direction. if (!transceiver) { - transceiver = CreateTransceiver(media_desc->type()); + auto sender = + CreateSender(media_desc->type(), nullptr, {rtc::CreateRandomUuid()}); + auto receiver = + CreateReceiver(media_desc->type(), media_desc->streams()[0].id); + transceiver = CreateAndAddTransceiver(sender, receiver); transceiver->internal()->set_direction( RtpTransceiverDirection::kRecvOnly); } @@ -2770,11 +2813,10 @@ void PeerConnection::AddAudioTrack(AudioTrackInterface* track, } // Normal case; we've never seen this track before. - rtc::scoped_refptr> new_sender = - RtpSenderProxyWithInternal::Create( - signaling_thread(), - new AudioRtpSender(track, {stream->label()}, voice_channel(), - stats_.get())); + auto new_sender = + CreateSender(cricket::MEDIA_TYPE_AUDIO, track, {stream->label()}); + static_cast(new_sender->internal()) + ->SetChannel(voice_channel()); GetAudioTransceiver()->internal()->AddSender(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 @@ -2815,10 +2857,10 @@ void PeerConnection::AddVideoTrack(VideoTrackInterface* track, } // Normal case; we've never seen this track before. - rtc::scoped_refptr> new_sender = - RtpSenderProxyWithInternal::Create( - signaling_thread(), - new VideoRtpSender(track, {stream->label()}, video_channel())); + auto new_sender = + CreateSender(cricket::MEDIA_TYPE_VIDEO, track, {stream->label()}); + static_cast(new_sender->internal()) + ->SetChannel(video_channel()); GetVideoTransceiver()->internal()->AddSender(new_sender); const RtpSenderInfo* sender_info = FindSenderInfo(local_video_sender_infos_, stream->label(), track->id()); @@ -5277,6 +5319,30 @@ RTCError PeerConnection::ValidateSessionDescription( } } + // Unified Plan SDP should have exactly one stream per m= section for audio + // and video. + if (IsUnifiedPlan()) { + for (const ContentInfo& content : sdesc->description()->contents()) { + if (content.rejected) { + continue; + } + if (content.media_description()) { + cricket::MediaType media_type = content.media_description()->type(); + if (!(media_type == cricket::MEDIA_TYPE_AUDIO || + media_type == cricket::MEDIA_TYPE_VIDEO)) { + continue; + } + const cricket::StreamParamsVec& streams = + content.media_description()->streams(); + if (streams.size() != 1u) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "Unified Plan SDP should have exactly one " + "track per media section for audio and video."); + } + } + } + } + return RTCError::OK(); } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 4c7973bd1f..f2f833cc6b 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -379,10 +379,21 @@ class PeerConnection : public PeerConnectionInterface, rtc::scoped_refptr track, const RtpTransceiverInit& init); + rtc::scoped_refptr> + CreateSender(cricket::MediaType media_type, + rtc::scoped_refptr track, + const std::vector& stream_labels); + + rtc::scoped_refptr> + CreateReceiver(cricket::MediaType media_type, const std::string& receiver_id); + // Create a new RtpTransceiver of the given type and add it to the list of // transceivers. rtc::scoped_refptr> - CreateTransceiver(cricket::MediaType media_type); + CreateAndAddTransceiver( + rtc::scoped_refptr> sender, + rtc::scoped_refptr> + receiver); void SetIceConnectionState(IceConnectionState new_state); // Called any time the IceGatheringState changes diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc index 3ab8b5afcb..edab0820e4 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -242,6 +242,18 @@ TEST_F(PeerConnectionJsepTest, SetLocalOfferSetsTransceiverMid) { EXPECT_EQ(video_mid, video_transceiver->mid()); } +TEST_F(PeerConnectionJsepTest, SetLocalOfferFailsWithNoTrackInMediaSection) { + auto caller = CreatePeerConnection(); + caller->AddAudioTrack("audio"); + + auto offer = caller->CreateOffer(); + auto& contents = offer->description()->contents(); + ASSERT_EQ(1u, contents.size()); + contents[0].description->mutable_streams().clear(); + + ASSERT_FALSE(caller->SetLocalDescription(std::move(offer))); +} + // Tests for JSEP SetRemoteDescription with a remote offer. // Test that setting a remote offer with sendrecv audio and video creates two @@ -876,4 +888,77 @@ TEST_F(PeerConnectionJsepTest, CalleeDoesReoffer) { EXPECT_EQ(2u, callee->pc()->GetTransceivers().size()); } +// Tests for MSID properties. + +// Test that adding a track with AddTrack results in an offer that signals the +// track's ID. +TEST_F(PeerConnectionJsepTest, AddingTrackWithAddTrackSpecifiesTrackId) { + const std::string kTrackId = "audio_track"; + + auto caller = CreatePeerConnection(); + caller->AddAudioTrack(kTrackId); + + auto offer = caller->CreateOffer(); + auto contents = offer->description()->contents(); + ASSERT_EQ(1u, contents.size()); + auto streams = contents[0].media_description()->streams(); + ASSERT_EQ(1u, streams.size()); + EXPECT_EQ(kTrackId, streams[0].id); +} + +// Test that adding a track by calling AddTransceiver then SetTrack results in +// an offer that does not signal the track's ID and signals a random ID. +TEST_F(PeerConnectionJsepTest, + AddingTrackWithAddTransceiverSpecifiesRandomTrackId) { + const std::string kTrackId = "audio_track"; + + auto caller = CreatePeerConnection(); + auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + transceiver->sender()->SetTrack(caller->CreateAudioTrack(kTrackId)); + + auto offer = caller->CreateOffer(); + auto contents = offer->description()->contents(); + ASSERT_EQ(1u, contents.size()); + auto streams = contents[0].media_description()->streams(); + ASSERT_EQ(1u, streams.size()); + EXPECT_NE(kTrackId, streams[0].id); +} + +// Test that the callee RtpReceiver created by a call to SetRemoteDescription +// has its ID set to the signaled track ID. +TEST_F(PeerConnectionJsepTest, + RtpReceiverCreatedBySetRemoteDescriptionHasSignaledTrackId) { + const std::string kTrackId = "audio_track"; + + auto caller = CreatePeerConnection(); + auto callee = CreatePeerConnection(); + caller->AddAudioTrack(kTrackId); + + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + + ASSERT_EQ(1u, callee->pc()->GetReceivers().size()); + auto receiver = callee->pc()->GetReceivers()[0]; + EXPECT_EQ(kTrackId, receiver->id()); +} + +// Test that if the callee RtpReceiver is reused by a call to +// SetRemoteDescription, its ID does not change. +TEST_F(PeerConnectionJsepTest, + RtpReceiverCreatedBeforeSetRemoteDescriptionKeepsId) { + const std::string kTrackId = "audio_track"; + + auto caller = CreatePeerConnection(); + auto callee = CreatePeerConnection(); + caller->AddAudioTrack(kTrackId); + callee->AddAudioTrack("dummy_track"); + + ASSERT_EQ(1u, callee->pc()->GetReceivers().size()); + auto receiver = callee->pc()->GetReceivers()[0]; + std::string receiver_id = receiver->id(); + + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + + EXPECT_EQ(receiver_id, receiver->id()); +} + } // namespace webrtc diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index 78e18c3518..f418f7ee9f 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -736,6 +736,17 @@ TEST_F(PeerConnectionRtpUnifiedPlanTest, EXPECT_EQ(RtpTransceiverDirection::kSendRecv, transceiver->direction()); } +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackCreatesSenderWithTrackId) { + const std::string kTrackId = "audio_track"; + + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto audio_track = caller->CreateAudioTrack(kTrackId); + auto sender = caller->AddTrack(audio_track); + + EXPECT_EQ(kTrackId, sender->id()); +} + // Unified Plan AddTrack error handling. TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackErrorIfClosed) { diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index 9c1d3b6129..91e84d73bd 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -46,50 +46,28 @@ void LocalAudioSinkAdapter::SetSink(cricket::AudioSource::Sink* sink) { sink_ = sink; } -AudioRtpSender::AudioRtpSender(rtc::scoped_refptr track, - const std::vector& stream_ids, - cricket::VoiceChannel* channel, - StatsCollector* stats) - : id_(track->id()), - stream_ids_(stream_ids), - channel_(channel), - stats_(stats), - track_(track), - cached_track_enabled_(track->enabled()), - sink_adapter_(new LocalAudioSinkAdapter()) { - // TODO(steveanton): Relax this constraint once more Unified Plan work is - // done. - RTC_CHECK(stream_ids_.size() == 1U); - track_->RegisterObserver(this); - track_->AddSink(sink_adapter_.get()); - CreateDtmfSender(); -} +AudioRtpSender::AudioRtpSender(StatsCollector* stats) + : AudioRtpSender(nullptr, {rtc::CreateRandomUuid()}, stats) {} AudioRtpSender::AudioRtpSender(rtc::scoped_refptr track, - cricket::VoiceChannel* channel, + const std::vector& stream_labels, StatsCollector* stats) - : id_(track->id()), - // TODO(steveanton): With Unified Plan this should be empty. - stream_ids_({rtc::CreateRandomUuid()}), - channel_(channel), + : id_(track ? track->id() : rtc::CreateRandomUuid()), + stream_ids_(stream_labels), stats_(stats), track_(track), - cached_track_enabled_(track->enabled()), + dtmf_sender_proxy_(DtmfSenderProxy::Create( + rtc::Thread::Current(), + DtmfSender::Create(track_, rtc::Thread::Current(), this))), + cached_track_enabled_(track ? track->enabled() : false), sink_adapter_(new LocalAudioSinkAdapter()) { - track_->RegisterObserver(this); - track_->AddSink(sink_adapter_.get()); - CreateDtmfSender(); -} - -AudioRtpSender::AudioRtpSender(cricket::VoiceChannel* channel, - StatsCollector* stats) - : id_(rtc::CreateRandomUuid()), - // TODO(steveanton): With Unified Plan this should be empty. - stream_ids_({rtc::CreateRandomUuid()}), - channel_(channel), - stats_(stats), - sink_adapter_(new LocalAudioSinkAdapter()) { - CreateDtmfSender(); + // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are + // supported. + RTC_DCHECK_EQ(stream_labels.size(), 1u); + if (track_) { + track_->RegisterObserver(this); + track_->AddSink(sink_adapter_.get()); + } } AudioRtpSender::~AudioRtpSender() { @@ -289,53 +267,26 @@ void AudioRtpSender::ClearAudioSend() { } } -void AudioRtpSender::CreateDtmfSender() { - // Should be on signaling thread. - // TODO(deadbeef): Add thread checking to RtpSender/RtpReceiver - // implementations. - rtc::scoped_refptr sender( - DtmfSender::Create(track_, rtc::Thread::Current(), this)); - if (!sender.get()) { - RTC_LOG(LS_ERROR) << "CreateDtmfSender failed on DtmfSender::Create."; - RTC_NOTREACHED(); +VideoRtpSender::VideoRtpSender() + : VideoRtpSender(nullptr, {rtc::CreateRandomUuid()}) {} + +VideoRtpSender::VideoRtpSender(rtc::scoped_refptr track, + const std::vector& stream_labels) + : id_(track ? track->id() : rtc::CreateRandomUuid()), + stream_ids_(stream_labels), + track_(track), + cached_track_enabled_(track ? track->enabled() : false), + cached_track_content_hint_( + track ? track->content_hint() + : VideoTrackInterface::ContentHint::kNone) { + // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are + // supported. + RTC_DCHECK_EQ(stream_labels.size(), 1u); + if (track_) { + track_->RegisterObserver(this); } - dtmf_sender_proxy_ = - DtmfSenderProxy::Create(rtc::Thread::Current(), sender.get()); } -VideoRtpSender::VideoRtpSender(rtc::scoped_refptr track, - const std::vector& stream_ids, - cricket::VideoChannel* channel) - : id_(track->id()), - stream_ids_({stream_ids}), - channel_(channel), - track_(track), - cached_track_enabled_(track->enabled()), - cached_track_content_hint_(track->content_hint()) { - // TODO(steveanton): Relax this constraint once more Unified Plan work is - // done. - RTC_CHECK(stream_ids_.size() == 1U); - track_->RegisterObserver(this); -} - -VideoRtpSender::VideoRtpSender(rtc::scoped_refptr track, - cricket::VideoChannel* channel) - : id_(track->id()), - // TODO(steveanton): With Unified Plan this should be empty. - stream_ids_({rtc::CreateRandomUuid()}), - channel_(channel), - track_(track), - cached_track_enabled_(track->enabled()), - cached_track_content_hint_(track->content_hint()) { - track_->RegisterObserver(this); -} - -VideoRtpSender::VideoRtpSender(cricket::VideoChannel* channel) - : id_(rtc::CreateRandomUuid()), - // TODO(steveanton): With Unified Plan this should be empty. - stream_ids_({rtc::CreateRandomUuid()}), - channel_(channel) {} - VideoRtpSender::~VideoRtpSender() { Stop(); } diff --git a/pc/rtpsender.h b/pc/rtpsender.h index 4eeb4d5e9a..a0c759ad1c 100644 --- a/pc/rtpsender.h +++ b/pc/rtpsender.h @@ -82,21 +82,16 @@ class AudioRtpSender : public DtmfProviderInterface, public: // StatsCollector provided so that Add/RemoveLocalAudioTrack can be called // at the appropriate times. - // |channel| can be null if one does not exist yet. - AudioRtpSender(rtc::scoped_refptr track, - const std::vector& stream_id, - cricket::VoiceChannel* channel, - StatsCollector* stats); - // Randomly generates stream_id. - // |channel| can be null if one does not exist yet. - AudioRtpSender(rtc::scoped_refptr track, - cricket::VoiceChannel* channel, - StatsCollector* stats); + // Construct an AudioRtpSender with a null track, a single, randomly generated + // stream label, and a randomly generated ID. + explicit AudioRtpSender(StatsCollector* stats); - // Randomly generates id and stream_id. - // |channel| can be null if one does not exist yet. - AudioRtpSender(cricket::VoiceChannel* channel, StatsCollector* stats); + // Construct an AudioRtpSender with the given track and stream labels. The + // sender ID will be set to the track's ID. + AudioRtpSender(rtc::scoped_refptr track, + const std::vector& stream_labels, + StatsCollector* stats); virtual ~AudioRtpSender(); @@ -156,11 +151,9 @@ class AudioRtpSender : public DtmfProviderInterface, // Helper function to call SetAudioSend with "stop sending" parameters. void ClearAudioSend(); - void CreateDtmfSender(); - sigslot::signal0<> SignalDestroyed; - std::string id_; + const std::string id_; // TODO(steveanton): Until more Unified Plan work is done, this can only have // exactly one element. std::vector stream_ids_; @@ -180,19 +173,14 @@ class AudioRtpSender : public DtmfProviderInterface, class VideoRtpSender : public ObserverInterface, public rtc::RefCountedObject { public: - // |channel| can be null if one does not exist yet. - VideoRtpSender(rtc::scoped_refptr track, - const std::vector& stream_id, - cricket::VideoChannel* channel); + // Construct a VideoRtpSender with a null track, a single, randomly generated + // stream label, and a randomly generated ID. + VideoRtpSender(); - // Randomly generates stream_id. - // |channel| can be null if one does not exist yet. + // Construct a VideoRtpSender with the given track and stream labels. The + // sender ID will be set to the track's ID. VideoRtpSender(rtc::scoped_refptr track, - cricket::VideoChannel* channel); - - // Randomly generates id and stream_id. - // |channel| can be null if one does not exist yet. - explicit VideoRtpSender(cricket::VideoChannel* channel); + const std::vector& stream_labels); virtual ~VideoRtpSender(); @@ -245,7 +233,7 @@ class VideoRtpSender : public ObserverInterface, // Helper function to call SetVideoSend with "stop sending" parameters. void ClearVideoSend(); - std::string id_; + const std::string id_; // TODO(steveanton): Until more Unified Plan work is done, this can only have // exactly one element. std::vector stream_ids_; diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index f1ba6d87f1..9ccbe95026 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -134,28 +134,38 @@ class RtpSenderReceiverTest : public testing::Test, const rtc::scoped_refptr& source) { audio_track_ = AudioTrack::Create(kAudioTrackId, source); EXPECT_TRUE(local_stream_->AddTrack(audio_track_)); - audio_rtp_sender_ = - new AudioRtpSender(local_stream_->GetAudioTracks()[0], - {local_stream_->label()}, voice_channel_, nullptr); + audio_rtp_sender_ = new AudioRtpSender(local_stream_->GetAudioTracks()[0], + {local_stream_->label()}, nullptr); + audio_rtp_sender_->SetChannel(voice_channel_); audio_rtp_sender_->SetSsrc(kAudioSsrc); audio_rtp_sender_->GetOnDestroyedSignal()->connect( this, &RtpSenderReceiverTest::OnAudioSenderDestroyed); VerifyVoiceChannelInput(); } + void CreateAudioRtpSenderWithNoTrack() { + audio_rtp_sender_ = new AudioRtpSender(nullptr); + audio_rtp_sender_->SetChannel(voice_channel_); + } + void OnAudioSenderDestroyed() { audio_sender_destroyed_signal_fired_ = true; } void CreateVideoRtpSender() { CreateVideoRtpSender(false); } void CreateVideoRtpSender(bool is_screencast) { AddVideoTrack(is_screencast); - video_rtp_sender_ = - new VideoRtpSender(local_stream_->GetVideoTracks()[0], - {local_stream_->label()}, video_channel_); + video_rtp_sender_ = new VideoRtpSender(local_stream_->GetVideoTracks()[0], + {local_stream_->label()}); + video_rtp_sender_->SetChannel(video_channel_); video_rtp_sender_->SetSsrc(kVideoSsrc); VerifyVideoChannelInput(); } + void CreateVideoRtpSenderWithNoTrack() { + video_rtp_sender_ = new VideoRtpSender(); + video_rtp_sender_->SetChannel(video_channel_); + } + void DestroyAudioRtpSender() { audio_rtp_sender_ = nullptr; VerifyVoiceChannelNoInput(); @@ -425,7 +435,7 @@ TEST_F(RtpSenderReceiverTest, RemoteAudioTrackSetVolume) { // Test that the media channel isn't enabled for sending if the audio sender // doesn't have both a track and SSRC. TEST_F(RtpSenderReceiverTest, AudioSenderWithoutTrackAndSsrc) { - audio_rtp_sender_ = new AudioRtpSender(voice_channel_, nullptr); + CreateAudioRtpSenderWithNoTrack(); rtc::scoped_refptr track = AudioTrack::Create(kAudioTrackId, nullptr); @@ -442,7 +452,7 @@ TEST_F(RtpSenderReceiverTest, AudioSenderWithoutTrackAndSsrc) { // Test that the media channel isn't enabled for sending if the video sender // doesn't have both a track and SSRC. TEST_F(RtpSenderReceiverTest, VideoSenderWithoutTrackAndSsrc) { - video_rtp_sender_ = new VideoRtpSender(video_channel_); + CreateVideoRtpSenderWithNoTrack(); // Track but no SSRC. EXPECT_TRUE(video_rtp_sender_->SetTrack(video_track_)); @@ -457,7 +467,7 @@ TEST_F(RtpSenderReceiverTest, VideoSenderWithoutTrackAndSsrc) { // Test that the media channel is enabled for sending when the audio sender // has a track and SSRC, when the SSRC is set first. TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupSsrcThenTrack) { - audio_rtp_sender_ = new AudioRtpSender(voice_channel_, nullptr); + CreateAudioRtpSenderWithNoTrack(); rtc::scoped_refptr track = AudioTrack::Create(kAudioTrackId, nullptr); audio_rtp_sender_->SetSsrc(kAudioSsrc); @@ -470,7 +480,7 @@ TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupSsrcThenTrack) { // Test that the media channel is enabled for sending when the audio sender // has a track and SSRC, when the SSRC is set last. TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupTrackThenSsrc) { - audio_rtp_sender_ = new AudioRtpSender(voice_channel_, nullptr); + CreateAudioRtpSenderWithNoTrack(); rtc::scoped_refptr track = AudioTrack::Create(kAudioTrackId, nullptr); audio_rtp_sender_->SetTrack(track); @@ -484,7 +494,7 @@ TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupTrackThenSsrc) { // has a track and SSRC, when the SSRC is set first. TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupSsrcThenTrack) { AddVideoTrack(); - video_rtp_sender_ = new VideoRtpSender(video_channel_); + CreateVideoRtpSenderWithNoTrack(); video_rtp_sender_->SetSsrc(kVideoSsrc); video_rtp_sender_->SetTrack(video_track_); VerifyVideoChannelInput(); @@ -496,7 +506,7 @@ TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupSsrcThenTrack) { // has a track and SSRC, when the SSRC is set last. TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupTrackThenSsrc) { AddVideoTrack(); - video_rtp_sender_ = new VideoRtpSender(video_channel_); + CreateVideoRtpSenderWithNoTrack(); video_rtp_sender_->SetTrack(video_track_); video_rtp_sender_->SetSsrc(kVideoSsrc); VerifyVideoChannelInput(); @@ -762,9 +772,9 @@ TEST_F(RtpSenderReceiverTest, // Setting detailed overrides the default non-screencast mode. This should be // applied even if the track is set on construction. video_track_->set_content_hint(VideoTrackInterface::ContentHint::kDetailed); - video_rtp_sender_ = - new VideoRtpSender(local_stream_->GetVideoTracks()[0], - {local_stream_->label()}, video_channel_); + video_rtp_sender_ = new VideoRtpSender(local_stream_->GetVideoTracks()[0], + {local_stream_->label()}); + video_rtp_sender_->SetChannel(video_channel_); video_track_->set_enabled(true); // Sender is not ready to send (no SSRC) so no option should have been set.