From 09a0d0171c190ee0de341b955be5d80480171f79 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 4 Jan 2022 19:42:07 +0000 Subject: [PATCH] Deprecate RemoveTrack (old signature) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This also removes all internal usage of RemoveTrack, and changes the replacement function to RemoveTrackOrError rather than RemoveTrackNew. Bug: webrtc:9534 Change-Id: Idf7bb17495686de77c70428dcbfb12278328ce59 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244094 Reviewed-by: Niels Moller Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#35624} --- api/peer_connection_interface.cc | 6 ---- api/peer_connection_interface.h | 36 ++++++++++++++----- api/test/dummy_peer_connection.h | 2 +- api/test/mock_peerconnectioninterface.h | 2 +- pc/peer_connection.cc | 7 +--- pc/peer_connection.h | 3 +- pc/peer_connection_proxy.h | 5 +-- pc/scenario_tests/goog_cc_test.cc | 2 +- pc/test/fake_peer_connection_base.h | 2 +- sdk/android/src/jni/pc/peer_connection.cc | 6 ++-- .../api/peerconnection/RTCPeerConnection.mm | 2 +- 11 files changed, 41 insertions(+), 32 deletions(-) diff --git a/api/peer_connection_interface.cc b/api/peer_connection_interface.cc index 230731c42d..9f159ea731 100644 --- a/api/peer_connection_interface.cc +++ b/api/peer_connection_interface.cc @@ -41,12 +41,6 @@ PeerConnectionInterface::RTCConfiguration::RTCConfiguration( PeerConnectionInterface::RTCConfiguration::~RTCConfiguration() = default; -RTCError PeerConnectionInterface::RemoveTrackNew( - rtc::scoped_refptr sender) { - return RTCError(RemoveTrack(sender) ? RTCErrorType::NONE - : RTCErrorType::INTERNAL_ERROR); -} - RTCError PeerConnectionInterface::SetConfiguration( const PeerConnectionInterface::RTCConfiguration& config) { return RTCError(); diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 7a2ac12e41..c367fae36d 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -805,23 +805,41 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { rtc::scoped_refptr track, const std::vector& stream_ids) = 0; - // Remove an RtpSender from this PeerConnection. - // Returns true on success. - // TODO(steveanton): Replace with signature that returns RTCError. - virtual bool RemoveTrack(RtpSenderInterface* sender) = 0; - - // Plan B semantics: Removes the RtpSender from this PeerConnection. - // Unified Plan semantics: Stop sending on the RtpSender and mark the + // Removes the connection between a MediaStreamTrack and the PeerConnection. + // Stops sending on the RtpSender and marks the // corresponding RtpTransceiver direction as no longer sending. + // https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-removetrack // // Errors: // - INVALID_PARAMETER: `sender` is null or (Plan B only) the sender is not // associated with this PeerConnection. // - INVALID_STATE: PeerConnection is closed. + // + // Plan B semantics: Removes the RtpSender from this PeerConnection. + // // TODO(bugs.webrtc.org/9534): Rename to RemoveTrack once the other signature - // is removed. + // is removed; remove default implementation once upstream is updated. + virtual RTCError RemoveTrackOrError( + rtc::scoped_refptr sender) { + RTC_CHECK_NOTREACHED(); + return RTCError(); + } + + // Legacy API for removing a track from the PeerConnection. + // Returns true on success. + // TODO(bugs.webrtc.org/9534): Replace with signature that returns RTCError. + ABSL_DEPRECATED("Use RemoveTrackOrError") + virtual bool RemoveTrack(RtpSenderInterface* sender) { + return RemoveTrackOrError(rtc::scoped_refptr(sender)) + .ok(); + } + + // Old name for the new API. Will be removed when clients are updated. + ABSL_DEPRECATED("Use RemoveTrackOrError") virtual RTCError RemoveTrackNew( - rtc::scoped_refptr sender); + rtc::scoped_refptr sender) { + return RemoveTrackOrError(sender); + } // AddTransceiver creates a new RtpTransceiver and adds it to the set of // transceivers. Adding a transceiver will cause future calls to CreateOffer diff --git a/api/test/dummy_peer_connection.h b/api/test/dummy_peer_connection.h index 80ae20c3c7..a477d56082 100644 --- a/api/test/dummy_peer_connection.h +++ b/api/test/dummy_peer_connection.h @@ -47,7 +47,7 @@ class DummyPeerConnection : public PeerConnectionInterface { bool RemoveTrack(RtpSenderInterface* sender) override { return false; } - RTCError RemoveTrackNew( + RTCError RemoveTrackOrError( rtc::scoped_refptr sender) override { return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, "Not implemented"); } diff --git a/api/test/mock_peerconnectioninterface.h b/api/test/mock_peerconnectioninterface.h index cd67d32a10..fc740b0718 100644 --- a/api/test/mock_peerconnectioninterface.h +++ b/api/test/mock_peerconnectioninterface.h @@ -50,7 +50,7 @@ class MockPeerConnectionInterface (override)); MOCK_METHOD(bool, RemoveTrack, (RtpSenderInterface*), (override)); MOCK_METHOD(RTCError, - RemoveTrackNew, + RemoveTrackOrError, (rtc::scoped_refptr), (override)); MOCK_METHOD(RTCErrorOr>, diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index a00e400a69..0bd3f27e8b 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -847,12 +847,7 @@ RTCErrorOr> PeerConnection::AddTrack( return sender_or_error; } -bool PeerConnection::RemoveTrack(RtpSenderInterface* sender) { - TRACE_EVENT0("webrtc", "PeerConnection::RemoveTrack"); - return RemoveTrackNew(rtc::scoped_refptr(sender)).ok(); -} - -RTCError PeerConnection::RemoveTrackNew( +RTCError PeerConnection::RemoveTrackOrError( rtc::scoped_refptr sender) { RTC_DCHECK_RUN_ON(signaling_thread()); if (!sender) { diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 7326bccd10..59f43c7d9a 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -141,8 +141,7 @@ class PeerConnection : public PeerConnectionInternal, RTCErrorOr> AddTrack( rtc::scoped_refptr track, const std::vector& stream_ids) override; - bool RemoveTrack(RtpSenderInterface* sender) override; - RTCError RemoveTrackNew( + RTCError RemoveTrackOrError( rtc::scoped_refptr sender) override; RTCErrorOr> AddTransceiver( diff --git a/pc/peer_connection_proxy.h b/pc/peer_connection_proxy.h index 7601c9d053..643602971a 100644 --- a/pc/peer_connection_proxy.h +++ b/pc/peer_connection_proxy.h @@ -35,8 +35,9 @@ PROXY_METHOD2(RTCErrorOr>, AddTrack, rtc::scoped_refptr, const std::vector&) -PROXY_METHOD1(bool, RemoveTrack, RtpSenderInterface*) -PROXY_METHOD1(RTCError, RemoveTrackNew, rtc::scoped_refptr) +PROXY_METHOD1(RTCError, + RemoveTrackOrError, + rtc::scoped_refptr) PROXY_METHOD1(RTCErrorOr>, AddTransceiver, rtc::scoped_refptr) diff --git a/pc/scenario_tests/goog_cc_test.cc b/pc/scenario_tests/goog_cc_test.cc index d9e27e2edf..82ae47b0c7 100644 --- a/pc/scenario_tests/goog_cc_test.cc +++ b/pc/scenario_tests/goog_cc_test.cc @@ -93,7 +93,7 @@ TEST(GoogCcPeerScenarioTest, MAYBE_NoBweChangeFromVideoUnmute) { // Resume video but stop audio. Bandwidth should not drop. video.capturer->Start(); - RTCError status = caller->pc()->RemoveTrackNew(audio.sender); + RTCError status = caller->pc()->RemoveTrackOrError(audio.sender); ASSERT_TRUE(status.ok()); audio.track->set_enabled(false); for (int i = 0; i < 10; i++) { diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h index 7970dd0f0f..d392862192 100644 --- a/pc/test/fake_peer_connection_base.h +++ b/pc/test/fake_peer_connection_base.h @@ -52,7 +52,7 @@ class FakePeerConnectionBase : public PeerConnectionInternal { bool RemoveTrack(RtpSenderInterface* sender) override { return false; } - RTCError RemoveTrackNew( + RTCError RemoveTrackOrError( rtc::scoped_refptr sender) override { return RTCError(RTCErrorType::UNSUPPORTED_OPERATION); } diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc index 143ad2bbc1..ee3c35cb3a 100644 --- a/sdk/android/src/jni/pc/peer_connection.cc +++ b/sdk/android/src/jni/pc/peer_connection.cc @@ -776,8 +776,10 @@ static jboolean JNI_PeerConnection_RemoveTrack( JNIEnv* jni, const JavaParamRef& j_pc, jlong native_sender) { - return ExtractNativePC(jni, j_pc)->RemoveTrack( - reinterpret_cast(native_sender)); + return ExtractNativePC(jni, j_pc) + ->RemoveTrackOrError(rtc::scoped_refptr( + reinterpret_cast(native_sender))) + .ok(); } static ScopedJavaLocalRef JNI_PeerConnection_AddTransceiverWithTrack( diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm index 71ca10051e..4a31a5460c 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm +++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm @@ -515,7 +515,7 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack( } - (BOOL)removeTrack:(RTC_OBJC_TYPE(RTCRtpSender) *)sender { - bool result = _peerConnection->RemoveTrack(sender.nativeRtpSender); + bool result = _peerConnection->RemoveTrackOrError(sender.nativeRtpSender).ok(); if (!result) { RTCLogError(@"Failed to remote track %@", sender); }