diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index d3209f4d88..6ea0cb033c 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -599,9 +599,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // - 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) { + virtual RTCErrorOr> AddTrack( + 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 diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h index c39c9333aa..7235f5b4b2 100644 --- a/api/peerconnectionproxy.h +++ b/api/peerconnectionproxy.h @@ -29,7 +29,7 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection) PROXY_METHOD1(bool, AddStream, MediaStreamInterface*) PROXY_METHOD1(void, RemoveStream, MediaStreamInterface*) PROXY_METHOD2(RTCErrorOr>, - AddTrackWithStreamLabels, + AddTrack, rtc::scoped_refptr, const std::vector&); PROXY_METHOD2(rtc::scoped_refptr, diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 78fcf1d666..020f3c3633 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1009,18 +1009,17 @@ rtc::scoped_refptr PeerConnection::AddTrack( } stream_labels.push_back(stream->label()); } - auto sender_or_error = AddTrackWithStreamLabels(track, stream_labels); + auto sender_or_error = AddTrack(track, stream_labels); if (!sender_or_error.ok()) { return nullptr; } return sender_or_error.MoveValue(); } -RTCErrorOr> -PeerConnection::AddTrackWithStreamLabels( +RTCErrorOr> PeerConnection::AddTrack( rtc::scoped_refptr track, const std::vector& stream_labels) { - TRACE_EVENT0("webrtc", "PeerConnection::AddTrackWithStreamLabels"); + TRACE_EVENT0("webrtc", "PeerConnection::AddTrack"); if (!track) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Track is null."); } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index a3797f4c7b..3fd944cc14 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -88,7 +88,7 @@ class PeerConnection : public PeerConnectionInterface, bool AddStream(MediaStreamInterface* local_stream) override; void RemoveStream(MediaStreamInterface* local_stream) override; - RTCErrorOr> AddTrackWithStreamLabels( + RTCErrorOr> AddTrack( rtc::scoped_refptr track, const std::vector& stream_labels) override; rtc::scoped_refptr AddTrack( diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index 1e16728271..78e18c3518 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -101,9 +101,7 @@ TEST_F(PeerConnectionRtpCallbacksTest, AddTrackWithoutStreamFiresOnAddTrack) { auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {})); + ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"))); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -121,10 +119,8 @@ TEST_F(PeerConnectionRtpCallbacksTest, AddTrackWithStreamFiresOnAddTrack) { auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - auto stream = MediaStream::Create("audio_stream"); - EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {stream.get()})); + ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"), + {"audio_stream"})); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -142,9 +138,7 @@ TEST_F(PeerConnectionRtpCallbacksTest, auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - auto sender = caller->pc()->AddTrack(audio_track.get(), {}); + auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {}); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -164,10 +158,8 @@ TEST_F(PeerConnectionRtpCallbacksTest, auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - auto stream = MediaStream::Create("audio_stream"); - auto sender = caller->pc()->AddTrack(audio_track.get(), {stream.get()}); + auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), + {"audio_stream"}); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -187,14 +179,11 @@ TEST_F(PeerConnectionRtpCallbacksTest, auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track1( - pc_factory_->CreateAudioTrack("audio_track1", nullptr)); - rtc::scoped_refptr audio_track2( - pc_factory_->CreateAudioTrack("audio_track2", nullptr)); - auto stream = MediaStream::Create("shared_audio_stream"); - std::vector streams{stream.get()}; - auto sender1 = caller->pc()->AddTrack(audio_track1.get(), streams); - auto sender2 = caller->pc()->AddTrack(audio_track2.get(), streams); + const char kSharedStreamLabel[] = "shared_audio_stream"; + auto sender1 = caller->AddTrack(caller->CreateAudioTrack("audio_track1"), + {kSharedStreamLabel}); + auto sender2 = caller->AddTrack(caller->CreateAudioTrack("audio_track2"), + {kSharedStreamLabel}); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -230,9 +219,7 @@ TEST_F(PeerConnectionRtpObserverTest, AddSenderWithoutStreamAddsReceiver) { auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {})); + ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"), {})); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -250,10 +237,8 @@ TEST_F(PeerConnectionRtpObserverTest, AddSenderWithStreamAddsReceiver) { auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - auto stream = webrtc::MediaStream::Create("audio_stream"); - EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {stream})); + ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"), + {"audio_stream"})); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -271,9 +256,7 @@ TEST_F(PeerConnectionRtpObserverTest, auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - auto sender = caller->pc()->AddTrack(audio_track.get(), {}); + auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {}); ASSERT_TRUE(sender); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), @@ -294,10 +277,8 @@ TEST_F(PeerConnectionRtpObserverTest, RemoveSenderWithStreamRemovesReceiver) { auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); - auto stream = webrtc::MediaStream::Create("audio_stream"); - auto sender = caller->pc()->AddTrack(audio_track.get(), {stream}); + auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), + {"audio_stream"}); ASSERT_TRUE(sender); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), @@ -319,14 +300,11 @@ TEST_F(PeerConnectionRtpObserverTest, auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track1( - pc_factory_->CreateAudioTrack("audio_track1", nullptr)); - rtc::scoped_refptr audio_track2( - pc_factory_->CreateAudioTrack("audio_track2", nullptr)); - auto stream = webrtc::MediaStream::Create("shared_audio_stream"); - std::vector streams{stream.get()}; - auto sender1 = caller->pc()->AddTrack(audio_track1.get(), streams); - auto sender2 = caller->pc()->AddTrack(audio_track2.get(), streams); + const char kSharedStreamLabel[] = "shared_audio_stream"; + auto sender1 = caller->AddTrack(caller->CreateAudioTrack("audio_track1"), + {kSharedStreamLabel}); + auto sender2 = caller->AddTrack(caller->CreateAudioTrack("audio_track2"), + {kSharedStreamLabel}); ASSERT_TRUE( callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(), static_cast(nullptr))); @@ -375,11 +353,9 @@ TEST_F(PeerConnectionRtpObserverTest, auto caller = CreatePeerConnection(); auto callee = CreatePeerConnection(); - rtc::scoped_refptr audio_track( - pc_factory_->CreateAudioTrack("audio_track", nullptr)); // Create SDP for adding a track and for removing it. This will be used in the // first and second SetRemoteDescription() calls. - auto sender = caller->pc()->AddTrack(audio_track.get(), {}); + auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {}); auto srd1_sdp = caller->CreateOfferAndSetAsLocal(); EXPECT_TRUE(caller->pc()->RemoveTrack(sender)); auto srd2_sdp = caller->CreateOfferAndSetAsLocal(); @@ -612,7 +588,7 @@ TEST_F(PeerConnectionRtpUnifiedPlanTest, AddAudioTrackCreatesAudioSender) { auto caller = CreatePeerConnectionWithUnifiedPlan(); auto audio_track = caller->CreateAudioTrack("a"); - auto sender = caller->pc()->AddTrack(audio_track, {}); + auto sender = caller->AddTrack(audio_track); ASSERT_TRUE(sender); EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, sender->media_type()); @@ -625,7 +601,7 @@ TEST_F(PeerConnectionRtpUnifiedPlanTest, AddVideoTrackCreatesVideoSender) { auto caller = CreatePeerConnectionWithUnifiedPlan(); auto video_track = caller->CreateVideoTrack("a"); - auto sender = caller->pc()->AddTrack(video_track, {}); + auto sender = caller->AddTrack(video_track); ASSERT_TRUE(sender); EXPECT_EQ(cricket::MEDIA_TYPE_VIDEO, sender->media_type()); @@ -654,7 +630,7 @@ TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackReusesTransceiver) { auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); auto audio_track = caller->CreateAudioTrack("a"); - auto sender = caller->pc()->AddTrack(audio_track, {}); + auto sender = caller->AddTrack(audio_track); ASSERT_TRUE(sender); auto transceivers = caller->pc()->GetTransceivers(); @@ -769,7 +745,9 @@ TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackErrorIfClosed) { caller->pc()->Close(); caller->observer()->clear_negotiation_needed(); - EXPECT_FALSE(caller->pc()->AddTrack(audio_track, {})); + auto result = caller->pc() + ->AddTrack(audio_track, std::vector()); + EXPECT_EQ(RTCErrorType::INVALID_STATE, result.error().type()); EXPECT_FALSE(caller->observer()->negotiation_needed()); } @@ -777,10 +755,12 @@ TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackErrorIfTrackAlreadyHasSender) { auto caller = CreatePeerConnectionWithUnifiedPlan(); auto audio_track = caller->CreateAudioTrack("a"); - ASSERT_TRUE(caller->pc()->AddTrack(audio_track, {})); + ASSERT_TRUE(caller->AddTrack(audio_track)); caller->observer()->clear_negotiation_needed(); - EXPECT_FALSE(caller->pc()->AddTrack(audio_track, {})); + auto result = caller->pc() + ->AddTrack(audio_track, std::vector()); + EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type()); EXPECT_FALSE(caller->observer()->negotiation_needed()); } diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index 56c12e9d79..d8e1761b98 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -1409,11 +1409,6 @@ TEST_F(PeerConnectionInterfaceTest, RemoveStream) { // Also tests that RemoveTrack removes the tracks from subsequent offers. TEST_F(PeerConnectionInterfaceTest, AddTrackRemoveTrack) { CreatePeerConnectionWithoutDtls(); - // Create a dummy stream, so tracks share a stream label. - rtc::scoped_refptr stream( - pc_factory_->CreateLocalMediaStream(kStreamLabel1)); - std::vector stream_list; - stream_list.push_back(stream.get()); rtc::scoped_refptr audio_track( pc_factory_->CreateAudioTrack("audio_track", nullptr)); rtc::scoped_refptr video_track( @@ -1421,8 +1416,8 @@ TEST_F(PeerConnectionInterfaceTest, AddTrackRemoveTrack) { "video_track", pc_factory_->CreateVideoSource( std::unique_ptr( new cricket::FakeVideoCapturer())))); - auto audio_sender = pc_->AddTrack(audio_track, stream_list); - auto video_sender = pc_->AddTrack(video_track, stream_list); + auto audio_sender = pc_->AddTrack(audio_track, {kStreamLabel1}).MoveValue(); + auto video_sender = pc_->AddTrack(video_track, {kStreamLabel1}).MoveValue(); EXPECT_EQ(1UL, audio_sender->stream_ids().size()); EXPECT_EQ(kStreamLabel1, audio_sender->stream_ids()[0]); EXPECT_EQ("audio_track", audio_sender->id()); @@ -1483,9 +1478,9 @@ TEST_F(PeerConnectionInterfaceTest, AddTrackWithoutStream) { std::unique_ptr( new cricket::FakeVideoCapturer())))); auto audio_sender = - pc_->AddTrack(audio_track, std::vector()); + pc_->AddTrack(audio_track, std::vector()).MoveValue(); auto video_sender = - pc_->AddTrack(video_track, std::vector()); + pc_->AddTrack(video_track, std::vector()).MoveValue(); EXPECT_EQ("audio_track", audio_sender->id()); EXPECT_EQ(audio_track, audio_sender->track()); EXPECT_EQ("video_track", video_sender->id()); @@ -1506,10 +1501,8 @@ TEST_F(PeerConnectionInterfaceTest, AddTrackBeforeConnecting) { "video_track", pc_factory_->CreateVideoSource( std::unique_ptr( new cricket::FakeVideoCapturer())))); - auto audio_sender = - pc_->AddTrack(audio_track, std::vector()); - auto video_sender = - pc_->AddTrack(video_track, std::vector()); + auto audio_sender = pc_->AddTrack(audio_track, std::vector()); + auto video_sender = pc_->AddTrack(video_track, std::vector()); EXPECT_TRUE(DoGetStats(nullptr)); } diff --git a/pc/peerconnectionwrapper.cc b/pc/peerconnectionwrapper.cc index 5d804a8ca7..35aebcce9b 100644 --- a/pc/peerconnectionwrapper.cc +++ b/pc/peerconnectionwrapper.cc @@ -266,6 +266,15 @@ rtc::scoped_refptr PeerConnectionWrapper::CreateVideoTrack( return pc_factory()->CreateVideoTrack(label, video_source); } +rtc::scoped_refptr PeerConnectionWrapper::AddTrack( + rtc::scoped_refptr track, + const std::vector& stream_labels) { + RTCErrorOr> result = + pc()->AddTrack(track, stream_labels); + EXPECT_EQ(RTCErrorType::NONE, result.error().type()); + return result.MoveValue(); +} + rtc::scoped_refptr PeerConnectionWrapper::AddAudioTrack( const std::string& track_label, std::vector streams) { diff --git a/pc/peerconnectionwrapper.h b/pc/peerconnectionwrapper.h index 0830b5502d..c20c1f259f 100644 --- a/pc/peerconnectionwrapper.h +++ b/pc/peerconnectionwrapper.h @@ -130,6 +130,12 @@ class PeerConnectionWrapper { rtc::scoped_refptr CreateVideoTrack( const std::string& label); + // Wrapper for the underlying PeerConnection's AddTrack method. DCHECKs if + // AddTrack fails. + rtc::scoped_refptr AddTrack( + rtc::scoped_refptr track, + const std::vector& stream_labels = {}); + // Calls the underlying PeerConnection's AddTrack method with an audio media // stream track not bound to any source. rtc::scoped_refptr AddAudioTrack(