diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index a1e7d7ee2f..eb6685a561 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -590,10 +590,24 @@ class PeerConnectionInterface : public rtc::RefCountInterface { virtual void RemoveStream(MediaStreamInterface* stream) = 0; // Add a new MediaStreamTrack to be sent on this PeerConnection, and return - // the newly created RtpSender. + // the newly created RtpSender. The RtpSender will be associated with the + // streams specified in the |stream_labels| list. // + // Errors: + // - INVALID_PARAMETER: |track| is null, has a kind other than audio or video, + // or a sender already exists for the track. + // - INVALID_STATE: The PeerConnection is closed. + // TODO(steveanton): Remove default implementation once downstream + // implementations have been updated. + virtual RTCErrorOr> + AddTrackWithStreamLabels(rtc::scoped_refptr track, + const std::vector& stream_labels) { + return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, "Not implemented"); + } // |streams| indicates which stream labels the track should be associated // with. + // TODO(steveanton): Remove this overload once callers have moved to the + // signature with stream labels. virtual rtc::scoped_refptr AddTrack( MediaStreamTrackInterface* track, std::vector streams) = 0; diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h index 1d8cf5faaa..c39c9333aa 100644 --- a/api/peerconnectionproxy.h +++ b/api/peerconnectionproxy.h @@ -28,6 +28,10 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection) PROXY_METHOD0(rtc::scoped_refptr, remote_streams) PROXY_METHOD1(bool, AddStream, MediaStreamInterface*) PROXY_METHOD1(void, RemoveStream, MediaStreamInterface*) + PROXY_METHOD2(RTCErrorOr>, + AddTrackWithStreamLabels, + rtc::scoped_refptr, + const std::vector&); PROXY_METHOD2(rtc::scoped_refptr, AddTrack, MediaStreamTrackInterface*, diff --git a/api/rtptransceiverinterface.h b/api/rtptransceiverinterface.h index 88607b2ed4..1d2988d4cb 100644 --- a/api/rtptransceiverinterface.h +++ b/api/rtptransceiverinterface.h @@ -38,7 +38,7 @@ struct RtpTransceiverInit final { // The added RtpTransceiver will be added to these streams. // TODO(bugs.webrtc.org/7600): Not implemented. - std::vector> streams; + std::vector stream_labels; // TODO(bugs.webrtc.org/7600): Not implemented. std::vector send_encodings; diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index da190ada39..49ab979e40 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1005,83 +1005,195 @@ rtc::scoped_refptr PeerConnection::AddTrack( MediaStreamTrackInterface* track, std::vector streams) { TRACE_EVENT0("webrtc", "PeerConnection::AddTrack"); - if (IsClosed()) { + std::vector stream_labels; + for (auto* stream : streams) { + if (!stream) { + RTC_LOG(LS_ERROR) << "Stream list has null element."; + return nullptr; + } + stream_labels.push_back(stream->label()); + } + auto sender_or_error = AddTrackWithStreamLabels(track, stream_labels); + if (!sender_or_error.ok()) { return nullptr; } - if (streams.size() >= 2) { - RTC_LOG(LS_ERROR) - << "Adding a track with two streams is not currently supported."; - return nullptr; + return sender_or_error.MoveValue(); +} + +RTCErrorOr> +PeerConnection::AddTrackWithStreamLabels( + rtc::scoped_refptr track, + const std::vector& stream_labels) { + TRACE_EVENT0("webrtc", "PeerConnection::AddTrackWithStreamLabels"); + if (!track) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Track is null."); + } + if (!(track->kind() == MediaStreamTrackInterface::kAudioKind || + track->kind() == MediaStreamTrackInterface::kVideoKind)) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "Track has invalid kind: " + track->kind()); + } + // TODO(bugs.webrtc.org/7932): Support adding a track to multiple streams. + if (stream_labels.size() > 1u) { + LOG_AND_RETURN_ERROR( + RTCErrorType::UNSUPPORTED_OPERATION, + "AddTrack with more than one stream is not currently supported."); + } + if (IsClosed()) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE, + "PeerConnection is closed."); } if (FindSenderForTrack(track)) { - RTC_LOG(LS_ERROR) << "Sender for track " << track->id() - << " already exists."; - return nullptr; + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_PARAMETER, + "Sender already exists for track " + track->id() + "."); } + // TODO(bugs.webrtc.org/7933): MediaSession expects the sender to have exactly + // one stream. AddTrackInternal will return an error if there is more than one + // stream, but if the caller specifies none then we need to generate a random + // stream label. + std::vector adjusted_stream_labels = stream_labels; + if (stream_labels.empty()) { + adjusted_stream_labels.push_back(rtc::CreateRandomUuid()); + } + RTC_DCHECK_EQ(1, adjusted_stream_labels.size()); + auto sender_or_error = + (IsUnifiedPlan() ? AddTrackUnifiedPlan(track, adjusted_stream_labels) + : AddTrackPlanB(track, adjusted_stream_labels)); + if (sender_or_error.ok()) { + observer_->OnRenegotiationNeeded(); + } + return sender_or_error; +} - // TODO(deadbeef): Support adding a track to multiple streams. - rtc::scoped_refptr> new_sender; +RTCErrorOr> +PeerConnection::AddTrackPlanB( + rtc::scoped_refptr track, + const std::vector& stream_labels) { if (track->kind() == MediaStreamTrackInterface::kAudioKind) { - new_sender = RtpSenderProxyWithInternal::Create( + auto new_sender = RtpSenderProxyWithInternal::Create( signaling_thread(), - new AudioRtpSender(static_cast(track), + new AudioRtpSender(static_cast(track.get()), voice_channel(), stats_.get())); GetAudioTransceiver()->internal()->AddSender(new_sender); - if (!streams.empty()) { - new_sender->internal()->set_stream_id(streams[0]->label()); - } + 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); } - } else if (track->kind() == MediaStreamTrackInterface::kVideoKind) { - new_sender = RtpSenderProxyWithInternal::Create( + 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), + new VideoRtpSender(static_cast(track.get()), video_channel())); GetVideoTransceiver()->internal()->AddSender(new_sender); - if (!streams.empty()) { - new_sender->internal()->set_stream_id(streams[0]->label()); - } + 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); } - } else { - RTC_LOG(LS_ERROR) << "CreateSender called with invalid kind: " - << track->kind(); - return rtc::scoped_refptr(); + return rtc::scoped_refptr(new_sender); } +} - observer_->OnRenegotiationNeeded(); - return new_sender; +RTCErrorOr> +PeerConnection::AddTrackUnifiedPlan( + rtc::scoped_refptr track, + const std::vector& stream_labels) { + auto transceiver = FindFirstTransceiverForAddedTrack(track); + if (transceiver) { + if (transceiver->direction() == RtpTransceiverDirection::kRecvOnly) { + transceiver->SetDirection(RtpTransceiverDirection::kSendRecv); + } else if (transceiver->direction() == RtpTransceiverDirection::kInactive) { + transceiver->SetDirection(RtpTransceiverDirection::kSendOnly); + } + } else { + cricket::MediaType media_type = + (track->kind() == MediaStreamTrackInterface::kAudioKind + ? cricket::MEDIA_TYPE_AUDIO + : cricket::MEDIA_TYPE_VIDEO); + transceiver = CreateTransceiver(media_type); + 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(); +} + +rtc::scoped_refptr> +PeerConnection::FindFirstTransceiverForAddedTrack( + rtc::scoped_refptr track) { + RTC_DCHECK(track); + for (auto transceiver : transceivers_) { + if (!transceiver->sender()->track() && + cricket::MediaTypeToString(transceiver->internal()->media_type()) == + track->kind() && + !transceiver->internal()->has_ever_been_used_to_send()) { + return transceiver; + } + } + return nullptr; } bool PeerConnection::RemoveTrack(RtpSenderInterface* sender) { TRACE_EVENT0("webrtc", "PeerConnection::RemoveTrack"); + return RemoveTrackInternal(sender).ok(); +} + +RTCError PeerConnection::RemoveTrackInternal( + rtc::scoped_refptr sender) { + if (!sender) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Sender is null."); + } if (IsClosed()) { - return false; + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE, + "PeerConnection is closed."); } - - bool removed; - if (sender->media_type() == cricket::MEDIA_TYPE_AUDIO) { - removed = GetAudioTransceiver()->internal()->RemoveSender(sender); + if (IsUnifiedPlan()) { + auto transceiver = FindTransceiverBySender(sender); + if (!transceiver || !sender->track()) { + return RTCError::OK(); + } + sender->SetTrack(nullptr); + if (transceiver->direction() == RtpTransceiverDirection::kSendRecv) { + transceiver->internal()->SetDirection(RtpTransceiverDirection::kRecvOnly); + } else if (transceiver->direction() == RtpTransceiverDirection::kSendOnly) { + transceiver->internal()->SetDirection(RtpTransceiverDirection::kInactive); + } } else { - RTC_DCHECK_EQ(cricket::MEDIA_TYPE_VIDEO, sender->media_type()); - removed = GetVideoTransceiver()->internal()->RemoveSender(sender); + bool removed; + if (sender->media_type() == cricket::MEDIA_TYPE_AUDIO) { + removed = GetAudioTransceiver()->internal()->RemoveSender(sender); + } else { + RTC_DCHECK_EQ(cricket::MEDIA_TYPE_VIDEO, sender->media_type()); + removed = GetVideoTransceiver()->internal()->RemoveSender(sender); + } + if (!removed) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_PARAMETER, + "Couldn't find sender " + sender->id() + " to remove."); + } } - if (!removed) { - RTC_LOG(LS_ERROR) << "Couldn't find sender " << sender->id() - << " to remove."; - return false; - } - observer_->OnRenegotiationNeeded(); - return true; + return RTCError::OK(); +} + +rtc::scoped_refptr> +PeerConnection::FindTransceiverBySender( + rtc::scoped_refptr sender) { + for (auto transceiver : transceivers_) { + if (transceiver->sender() == sender) { + return transceiver; + } + } + return nullptr; } RTCErrorOr> @@ -1151,10 +1263,26 @@ PeerConnection::AddTransceiver( // TODO(bugs.webrtc.org/7600): Verify init. + auto transceiver = CreateTransceiver(media_type); + transceiver->SetDirection(init.direction); + if (track) { + transceiver->sender()->SetTrack(track); + } + + observer_->OnRenegotiationNeeded(); + + return rtc::scoped_refptr(transceiver); +} + +rtc::scoped_refptr> +PeerConnection::CreateTransceiver(cricket::MediaType media_type) { rtc::scoped_refptr> sender; 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())); @@ -1168,24 +1296,11 @@ PeerConnection::AddTransceiver( signaling_thread(), new VideoRtpReceiver(receiver_id, {}, worker_thread(), 0, nullptr)); } - // 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 (track) { - sender->SetTrack(track); - } - rtc::scoped_refptr> transceiver = RtpTransceiverProxyWithInternal::Create( signaling_thread(), new RtpTransceiver(sender, receiver)); - transceiver->SetDirection(init.direction); - transceivers_.push_back(transceiver); - - observer_->OnRenegotiationNeeded(); - - return rtc::scoped_refptr(transceiver); + return transceiver; } rtc::scoped_refptr PeerConnection::CreateDtmfSender( diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 16185de206..50a9da15af 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -89,6 +89,9 @@ class PeerConnection : public PeerConnectionInterface, bool AddStream(MediaStreamInterface* local_stream) override; void RemoveStream(MediaStreamInterface* local_stream) override; + RTCErrorOr> AddTrackWithStreamLabels( + rtc::scoped_refptr track, + const std::vector& stream_labels) override; rtc::scoped_refptr AddTrack( MediaStreamTrackInterface* track, std::vector streams) override; @@ -342,11 +345,37 @@ class PeerConnection : public PeerConnectionInterface, void RemoveVideoTrack(VideoTrackInterface* track, MediaStreamInterface* stream); + // AddTrack implementation when Unified Plan is specified. + RTCErrorOr> AddTrackUnifiedPlan( + rtc::scoped_refptr track, + const std::vector& stream_labels); + // AddTrack implementation when Plan B is specified. + RTCErrorOr> AddTrackPlanB( + rtc::scoped_refptr track, + const std::vector& stream_labels); + + // Returns the first RtpTransceiver suitable for a newly added track, if such + // transceiver is available. + rtc::scoped_refptr> + FindFirstTransceiverForAddedTrack( + rtc::scoped_refptr track); + + // RemoveTrack that returns an RTCError. + RTCError RemoveTrackInternal(rtc::scoped_refptr sender); + + rtc::scoped_refptr> + FindTransceiverBySender(rtc::scoped_refptr sender); + RTCErrorOr> AddTransceiver( cricket::MediaType media_type, rtc::scoped_refptr track, const RtpTransceiverInit& init); + // Create a new RtpTransceiver of the given type and add it to the list of + // transceivers. + rtc::scoped_refptr> + CreateTransceiver(cricket::MediaType media_type); + void SetIceConnectionState(IceConnectionState new_state); // Called any time the IceGatheringState changes void OnIceGatheringChange(IceGatheringState new_state); diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index 4989726020..1e16728271 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -452,7 +452,7 @@ TEST_F(PeerConnectionRtpLegacyObserverTest, EXPECT_FALSE(observer->called()); } -// RtpTransceiver Tests +// RtpTransceiver Tests. // Test that by default there are no transceivers with Unified Plan. TEST_F(PeerConnectionRtpTest, PeerConnectionHasNoTransceivers) { @@ -602,4 +602,277 @@ TEST_F(PeerConnectionRtpTest, UnifiedPlanCanClosePeerConnection) { caller->pc()->Close(); } +// Unified Plan AddTrack tests. + +class PeerConnectionRtpUnifiedPlanTest : public PeerConnectionRtpTest {}; + +// Test that adding an audio track creates a new audio RtpSender with the given +// track. +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddAudioTrackCreatesAudioSender) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto audio_track = caller->CreateAudioTrack("a"); + auto sender = caller->pc()->AddTrack(audio_track, {}); + ASSERT_TRUE(sender); + + EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, sender->media_type()); + EXPECT_EQ(audio_track, sender->track()); +} + +// Test that adding a video track creates a new video RtpSender with the given +// track. +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddVideoTrackCreatesVideoSender) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto video_track = caller->CreateVideoTrack("a"); + auto sender = caller->pc()->AddTrack(video_track, {}); + ASSERT_TRUE(sender); + + EXPECT_EQ(cricket::MEDIA_TYPE_VIDEO, sender->media_type()); + EXPECT_EQ(video_track, sender->track()); +} + +// Test that adding a track to a new PeerConnection creates an RtpTransceiver +// with the sender that AddTrack returns and in the sendrecv direction. +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddFirstTrackCreatesTransceiver) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto sender = caller->AddAudioTrack("a"); + ASSERT_TRUE(sender); + + auto transceivers = caller->pc()->GetTransceivers(); + ASSERT_EQ(1u, transceivers.size()); + EXPECT_EQ(sender, transceivers[0]->sender()); + EXPECT_EQ(RtpTransceiverDirection::kSendRecv, transceivers[0]->direction()); +} + +// Test that if a transceiver of the same type but no track had been added to +// the PeerConnection and later a call to AddTrack is made, the resulting sender +// is the transceiver's sender and the sender's track is the newly-added track. +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackReusesTransceiver) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + auto audio_track = caller->CreateAudioTrack("a"); + auto sender = caller->pc()->AddTrack(audio_track, {}); + ASSERT_TRUE(sender); + + auto transceivers = caller->pc()->GetTransceivers(); + ASSERT_EQ(1u, transceivers.size()); + EXPECT_EQ(transceiver, transceivers[0]); + EXPECT_EQ(sender, transceiver->sender()); + EXPECT_EQ(audio_track, sender->track()); +} + +// Test that adding two tracks to a new PeerConnection creates two +// RtpTransceivers in the same order. +TEST_F(PeerConnectionRtpUnifiedPlanTest, TwoAddTrackCreatesTwoTransceivers) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto sender1 = caller->AddAudioTrack("a"); + auto sender2 = caller->AddVideoTrack("v"); + ASSERT_TRUE(sender2); + + auto transceivers = caller->pc()->GetTransceivers(); + ASSERT_EQ(2u, transceivers.size()); + EXPECT_EQ(sender1, transceivers[0]->sender()); + EXPECT_EQ(sender2, transceivers[1]->sender()); +} + +// Test that if there are multiple transceivers with no sending track then a +// later call to AddTrack will use the one of the same type as the newly-added +// track. +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackReusesTransceiverOfType) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto audio_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + auto video_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); + auto sender = caller->AddVideoTrack("v"); + + ASSERT_EQ(2u, caller->pc()->GetTransceivers().size()); + EXPECT_NE(sender, audio_transceiver->sender()); + EXPECT_EQ(sender, video_transceiver->sender()); +} + +// Test that if the only transceivers that do not have a sending track have a +// different type from the added track, then AddTrack will create a new +// transceiver for the track. +TEST_F(PeerConnectionRtpUnifiedPlanTest, + AddTrackDoesNotReuseTransceiverOfWrongType) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + auto sender = caller->AddVideoTrack("v"); + + auto transceivers = caller->pc()->GetTransceivers(); + ASSERT_EQ(2u, transceivers.size()); + EXPECT_NE(sender, transceivers[0]->sender()); + EXPECT_EQ(sender, transceivers[1]->sender()); +} + +// Test that the first available transceiver is reused by AddTrack when multiple +// are available. +TEST_F(PeerConnectionRtpUnifiedPlanTest, + AddTrackReusesFirstMatchingTransceiver) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + auto sender = caller->AddAudioTrack("a"); + + auto transceivers = caller->pc()->GetTransceivers(); + ASSERT_EQ(2u, transceivers.size()); + EXPECT_EQ(sender, transceivers[0]->sender()); + EXPECT_NE(sender, transceivers[1]->sender()); +} + +// Test that a call to AddTrack that reuses a transceiver will change the +// direction from inactive to sendonly. +TEST_F(PeerConnectionRtpUnifiedPlanTest, + AddTrackChangesDirectionFromInactiveToSendOnly) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + RtpTransceiverInit init; + init.direction = RtpTransceiverDirection::kInactive; + auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init); + + caller->observer()->clear_negotiation_needed(); + ASSERT_TRUE(caller->AddAudioTrack("a")); + EXPECT_TRUE(caller->observer()->negotiation_needed()); + + EXPECT_EQ(RtpTransceiverDirection::kSendOnly, transceiver->direction()); +} + +// Test that a call to AddTrack that reuses a transceiver will change the +// direction from recvonly to sendrecv. +TEST_F(PeerConnectionRtpUnifiedPlanTest, + AddTrackChangesDirectionFromRecvOnlyToSendRecv) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + RtpTransceiverInit init; + init.direction = RtpTransceiverDirection::kRecvOnly; + auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init); + + caller->observer()->clear_negotiation_needed(); + ASSERT_TRUE(caller->AddAudioTrack("a")); + EXPECT_TRUE(caller->observer()->negotiation_needed()); + + EXPECT_EQ(RtpTransceiverDirection::kSendRecv, transceiver->direction()); +} + +// Unified Plan AddTrack error handling. + +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackErrorIfClosed) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto audio_track = caller->CreateAudioTrack("a"); + caller->pc()->Close(); + + caller->observer()->clear_negotiation_needed(); + EXPECT_FALSE(caller->pc()->AddTrack(audio_track, {})); + EXPECT_FALSE(caller->observer()->negotiation_needed()); +} + +TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackErrorIfTrackAlreadyHasSender) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto audio_track = caller->CreateAudioTrack("a"); + ASSERT_TRUE(caller->pc()->AddTrack(audio_track, {})); + + caller->observer()->clear_negotiation_needed(); + EXPECT_FALSE(caller->pc()->AddTrack(audio_track, {})); + EXPECT_FALSE(caller->observer()->negotiation_needed()); +} + +// Unified Plan RemoveTrack tests. + +// Test that calling RemoveTrack on a sender with a previously-added track +// clears the sender's track. +TEST_F(PeerConnectionRtpUnifiedPlanTest, RemoveTrackClearsSenderTrack) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto sender = caller->AddAudioTrack("a"); + ASSERT_TRUE(caller->pc()->RemoveTrack(sender)); + + EXPECT_FALSE(sender->track()); +} + +// Test that calling RemoveTrack on a sender where the transceiver is configured +// in the sendrecv direction changes the transceiver's direction to recvonly. +TEST_F(PeerConnectionRtpUnifiedPlanTest, + RemoveTrackChangesDirectionFromSendRecvToRecvOnly) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + RtpTransceiverInit init; + init.direction = RtpTransceiverDirection::kSendRecv; + auto transceiver = + caller->AddTransceiver(caller->CreateAudioTrack("a"), init); + + caller->observer()->clear_negotiation_needed(); + ASSERT_TRUE(caller->pc()->RemoveTrack(transceiver->sender())); + EXPECT_TRUE(caller->observer()->negotiation_needed()); + + EXPECT_EQ(RtpTransceiverDirection::kRecvOnly, transceiver->direction()); + EXPECT_TRUE(caller->observer()->renegotiation_needed_); +} + +// Test that calling RemoveTrack on a sender where the transceiver is configured +// in the sendonly direction changes the transceiver's direction to inactive. +TEST_F(PeerConnectionRtpUnifiedPlanTest, + RemoveTrackChangesDirectionFromSendOnlyToInactive) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + RtpTransceiverInit init; + init.direction = RtpTransceiverDirection::kSendOnly; + auto transceiver = + caller->AddTransceiver(caller->CreateAudioTrack("a"), init); + + caller->observer()->clear_negotiation_needed(); + ASSERT_TRUE(caller->pc()->RemoveTrack(transceiver->sender())); + EXPECT_TRUE(caller->observer()->negotiation_needed()); + + EXPECT_EQ(RtpTransceiverDirection::kInactive, transceiver->direction()); +} + +// Test that calling RemoveTrack with a sender that has a null track results in +// no change in state. +TEST_F(PeerConnectionRtpUnifiedPlanTest, RemoveTrackWithNullSenderTrackIsNoOp) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto sender = caller->AddAudioTrack("a"); + auto transceiver = caller->pc()->GetTransceivers()[0]; + ASSERT_TRUE(sender->SetTrack(nullptr)); + + caller->observer()->clear_negotiation_needed(); + ASSERT_TRUE(caller->pc()->RemoveTrack(sender)); + EXPECT_FALSE(caller->observer()->negotiation_needed()); + + EXPECT_EQ(RtpTransceiverDirection::kSendRecv, transceiver->direction()); +} + +// Unified Plan RemoveTrack error handling. + +TEST_F(PeerConnectionRtpUnifiedPlanTest, RemoveTrackErrorIfClosed) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto sender = caller->AddAudioTrack("a"); + caller->pc()->Close(); + + caller->observer()->clear_negotiation_needed(); + EXPECT_FALSE(caller->pc()->RemoveTrack(sender)); + EXPECT_FALSE(caller->observer()->negotiation_needed()); +} + +TEST_F(PeerConnectionRtpUnifiedPlanTest, + RemoveTrackNoErrorIfTrackAlreadyRemoved) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + + auto sender = caller->AddAudioTrack("a"); + ASSERT_TRUE(caller->pc()->RemoveTrack(sender)); + + caller->observer()->clear_negotiation_needed(); + EXPECT_TRUE(caller->pc()->RemoveTrack(sender)); + EXPECT_FALSE(caller->observer()->negotiation_needed()); +} + } // namespace webrtc diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index f5f8a71bc9..9c1d3b6129 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -46,7 +46,7 @@ void LocalAudioSinkAdapter::SetSink(cricket::AudioSource::Sink* sink) { sink_ = sink; } -AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, +AudioRtpSender::AudioRtpSender(rtc::scoped_refptr track, const std::vector& stream_ids, cricket::VoiceChannel* channel, StatsCollector* stats) @@ -65,7 +65,7 @@ AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, CreateDtmfSender(); } -AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, +AudioRtpSender::AudioRtpSender(rtc::scoped_refptr track, cricket::VoiceChannel* channel, StatsCollector* stats) : id_(track->id()), @@ -303,7 +303,7 @@ void AudioRtpSender::CreateDtmfSender() { DtmfSenderProxy::Create(rtc::Thread::Current(), sender.get()); } -VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, +VideoRtpSender::VideoRtpSender(rtc::scoped_refptr track, const std::vector& stream_ids, cricket::VideoChannel* channel) : id_(track->id()), @@ -318,7 +318,7 @@ VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, track_->RegisterObserver(this); } -VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, +VideoRtpSender::VideoRtpSender(rtc::scoped_refptr track, cricket::VideoChannel* channel) : id_(track->id()), // TODO(steveanton): With Unified Plan this should be empty. diff --git a/pc/rtpsender.h b/pc/rtpsender.h index ce8c657199..4eeb4d5e9a 100644 --- a/pc/rtpsender.h +++ b/pc/rtpsender.h @@ -83,14 +83,14 @@ class AudioRtpSender : public DtmfProviderInterface, // StatsCollector provided so that Add/RemoveLocalAudioTrack can be called // at the appropriate times. // |channel| can be null if one does not exist yet. - AudioRtpSender(AudioTrackInterface* track, + 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(AudioTrackInterface* track, + AudioRtpSender(rtc::scoped_refptr track, cricket::VoiceChannel* channel, StatsCollector* stats); @@ -181,13 +181,14 @@ class VideoRtpSender : public ObserverInterface, public rtc::RefCountedObject { public: // |channel| can be null if one does not exist yet. - VideoRtpSender(VideoTrackInterface* track, + VideoRtpSender(rtc::scoped_refptr track, const std::vector& stream_id, cricket::VideoChannel* channel); // Randomly generates stream_id. // |channel| can be null if one does not exist yet. - VideoRtpSender(VideoTrackInterface* track, cricket::VideoChannel* channel); + VideoRtpSender(rtc::scoped_refptr track, + cricket::VideoChannel* channel); // Randomly generates id and stream_id. // |channel| can be null if one does not exist yet. diff --git a/pc/rtptransceiver.cc b/pc/rtptransceiver.cc index 8ca29fce53..2919ee779d 100644 --- a/pc/rtptransceiver.cc +++ b/pc/rtptransceiver.cc @@ -113,6 +113,19 @@ bool RtpTransceiver::RemoveReceiver(RtpReceiverInterface* receiver) { return true; } +rtc::scoped_refptr RtpTransceiver::sender_internal() const { + RTC_DCHECK(unified_plan_); + RTC_CHECK_EQ(1u, senders_.size()); + return senders_[0]->internal(); +} + +rtc::scoped_refptr RtpTransceiver::receiver_internal() + const { + RTC_DCHECK(unified_plan_); + RTC_CHECK_EQ(1u, receivers_.size()); + return receivers_[0]->internal(); +} + rtc::Optional RtpTransceiver::mid() const { return mid_; } diff --git a/pc/rtptransceiver.h b/pc/rtptransceiver.h index 7053fad5e7..19f393f3fe 100644 --- a/pc/rtptransceiver.h +++ b/pc/rtptransceiver.h @@ -109,6 +109,25 @@ class RtpTransceiver final return receivers_; } + // Returns the backing object for the transceiver's Unified Plan sender. + rtc::scoped_refptr sender_internal() const; + + // Returns the backing object for the transceiver's Unified Plan receiver. + rtc::scoped_refptr receiver_internal() const; + + // According to JSEP rules for SetRemoteDescription, RtpTransceivers can be + // reused only if they were added by AddTrack. + void set_created_by_addtrack(bool created_by_addtrack) { + created_by_addtrack_ = created_by_addtrack; + } + bool created_by_addtrack() const { return created_by_addtrack_; } + + // Returns true if this transceiver has ever had the current direction set to + // sendonly or sendrecv. + bool has_ever_been_used_to_send() const { + return has_ever_been_used_to_send_; + } + // RtpTransceiverInterface implementation. rtc::Optional mid() const override; rtc::scoped_refptr sender() const override; @@ -133,6 +152,10 @@ class RtpTransceiver final RtpTransceiverDirection direction_ = RtpTransceiverDirection::kInactive; rtc::Optional current_direction_; rtc::Optional mid_; + bool created_by_addtrack_ = false; + // TODO(steveanton): Implement this once there is a mechanism to set the + // current direction. + bool has_ever_been_used_to_send_ = false; cricket::BaseChannel* channel_ = nullptr; }; diff --git a/pc/test/mockpeerconnectionobservers.h b/pc/test/mockpeerconnectionobservers.h index b45c67d657..5eb29f4a55 100644 --- a/pc/test/mockpeerconnectionobservers.h +++ b/pc/test/mockpeerconnectionobservers.h @@ -158,6 +158,9 @@ class MockPeerConnectionObserver : public PeerConnectionObserver { return candidates; } + bool negotiation_needed() const { return renegotiation_needed_; } + void clear_negotiation_needed() { renegotiation_needed_ = false; } + rtc::scoped_refptr pc_; PeerConnectionInterface::SignalingState state_; std::vector> candidates_;