diff --git a/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java b/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java index 63ecf10ca2..aeb91e1750 100644 --- a/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java +++ b/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java @@ -10,10 +10,10 @@ package org.webrtc; -/** Factory for creating webrtc::PeerConnectionInterface instances. */ +/** Factory for creating webrtc::jni::OwnedPeerConnection instances. */ public interface NativePeerConnectionFactory { /** - * Create a new webrtc::PeerConnectionInterface instance and returns a pointer to it. + * Create a new webrtc::jni::OwnedPeerConnection instance and returns a pointer to it. * The caller takes ownership of the object. */ long createNativePeerConnection(); diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java index 238814d601..20b06d5c08 100644 --- a/sdk/android/api/org/webrtc/PeerConnection.java +++ b/sdk/android/api/org/webrtc/PeerConnection.java @@ -548,7 +548,6 @@ public class PeerConnection { private final List localStreams = new ArrayList<>(); private final long nativePeerConnection; - private final long nativeObserver; private List senders = new ArrayList<>(); private List receivers = new ArrayList<>(); @@ -557,12 +556,11 @@ public class PeerConnection { * their PeerConnection creation in JNI. */ public PeerConnection(NativePeerConnectionFactory factory) { - this(factory.createNativePeerConnection(), 0 /* nativeObserver */); + this(factory.createNativePeerConnection()); } - PeerConnection(long nativePeerConnection, long nativeObserver) { + PeerConnection(long nativePeerConnection) { this.nativePeerConnection = nativePeerConnection; - this.nativeObserver = nativeObserver; } // JsepInterface. @@ -611,7 +609,7 @@ public class PeerConnection { } public boolean setConfiguration(RTCConfiguration config) { - return nativeSetConfiguration(config, nativeObserver); + return nativeSetConfiguration(config); } public boolean addIceCandidate(IceCandidate candidate) { @@ -781,14 +779,16 @@ public class PeerConnection { receiver.dispose(); } receivers.clear(); - JniCommon.nativeReleaseRef(nativePeerConnection); - if (nativeObserver != 0) { - nativeFreePeerConnectionObserver(nativeObserver); - } + nativeFreeOwnedPeerConnection(nativePeerConnection); + } + + /** Returns a pointer to the native webrtc::PeerConnectionInterface. */ + public long getNativePeerConnection() { + return nativeGetNativePeerConnection(); } @CalledByNative - public long getNativePeerConnection() { + long getNativeOwnedPeerConnection() { return nativePeerConnection; } @@ -796,10 +796,7 @@ public class PeerConnection { return nativeCreatePeerConnectionObserver(observer); } - public static void freeNativePeerConnectionObserver(long observer) { - nativeFreePeerConnectionObserver(observer); - } - + private native long nativeGetNativePeerConnection(); private native SessionDescription nativeGetLocalDescription(); private native SessionDescription nativeGetRemoteDescription(); private native DataChannel nativeCreateDataChannel(String label, DataChannel.Init init); @@ -815,8 +812,8 @@ public class PeerConnection { private native IceGatheringState nativeIceGatheringState(); private native void nativeClose(); private static native long nativeCreatePeerConnectionObserver(Observer observer); - private static native void nativeFreePeerConnectionObserver(long observer); - private native boolean nativeSetConfiguration(RTCConfiguration config, long nativeObserver); + private static native void nativeFreeOwnedPeerConnection(long ownedPeerConnection); + private native boolean nativeSetConfiguration(RTCConfiguration config); private native boolean nativeAddIceCandidate( String sdpMid, int sdpMLineIndex, String iceCandidateSdp); private native boolean nativeRemoveIceCandidates(final IceCandidate[] candidates); diff --git a/sdk/android/api/org/webrtc/PeerConnectionFactory.java b/sdk/android/api/org/webrtc/PeerConnectionFactory.java index 5c9dc616c0..e17d6550f8 100644 --- a/sdk/android/api/org/webrtc/PeerConnectionFactory.java +++ b/sdk/android/api/org/webrtc/PeerConnectionFactory.java @@ -232,7 +232,7 @@ public class PeerConnectionFactory { if (nativePeerConnection == 0) { return null; } - return new PeerConnection(nativePeerConnection, nativeObserver); + return new PeerConnection(nativePeerConnection); } /** diff --git a/sdk/android/src/jni/pc/peerconnection.cc b/sdk/android/src/jni/pc/peerconnection.cc index d904f07d3c..a94d835ff4 100644 --- a/sdk/android/src/jni/pc/peerconnection.cc +++ b/sdk/android/src/jni/pc/peerconnection.cc @@ -58,8 +58,9 @@ namespace { PeerConnectionInterface* ExtractNativePC(JNIEnv* jni, const JavaRef& j_pc) { - return reinterpret_cast( - Java_PeerConnection_getNativePeerConnection(jni, j_pc)); + return reinterpret_cast( + Java_PeerConnection_getNativeOwnedPeerConnection(jni, j_pc)) + ->pc(); } PeerConnectionInterface::IceServers JavaToNativeIceServers( @@ -284,12 +285,6 @@ void PeerConnectionObserverJni::OnAddTrack( NativeToJavaMediaStreamArray(env, streams)); } -void PeerConnectionObserverJni::SetConstraints( - std::unique_ptr constraints) { - RTC_CHECK(!constraints_.get()) << "constraints already set!"; - constraints_ = std::move(constraints); -} - // If the NativeToJavaStreamsMap contains the stream, return it. // Otherwise, create a new Java MediaStream. JavaMediaStream& PeerConnectionObserverJni::GetOrCreateJavaStream( @@ -318,6 +313,19 @@ PeerConnectionObserverJni::NativeToJavaMediaStreamArray( }); } +OwnedPeerConnection::OwnedPeerConnection( + rtc::scoped_refptr peer_connection, + std::unique_ptr observer, + std::unique_ptr constraints) + : peer_connection_(peer_connection), + observer_(std::move(observer)), + constraints_(std::move(constraints)) {} + +OwnedPeerConnection::~OwnedPeerConnection() { + // Ensure that PeerConnection is destroyed before the observer. + peer_connection_ = nullptr; +} + static jlong JNI_PeerConnection_CreatePeerConnectionObserver( JNIEnv* jni, const JavaParamRef&, @@ -325,12 +333,17 @@ static jlong JNI_PeerConnection_CreatePeerConnectionObserver( return jlongFromPointer(new PeerConnectionObserverJni(jni, j_observer)); } -static void JNI_PeerConnection_FreePeerConnectionObserver( +static void JNI_PeerConnection_FreeOwnedPeerConnection( JNIEnv*, const JavaParamRef&, jlong j_p) { - PeerConnectionObserver* p = reinterpret_cast(j_p); - delete p; + delete reinterpret_cast(j_p); +} + +static jlong JNI_PeerConnection_GetNativePeerConnection( + JNIEnv* jni, + const JavaParamRef& j_pc) { + return jlongFromPointer(ExtractNativePC(jni, j_pc)); } static ScopedJavaLocalRef JNI_PeerConnection_GetLocalDescription( @@ -426,19 +439,18 @@ static void JNI_PeerConnection_SetAudioRecording( static jboolean JNI_PeerConnection_SetConfiguration( JNIEnv* jni, const JavaParamRef& j_pc, - const JavaParamRef& j_rtc_config, - jlong native_observer) { + const JavaParamRef& j_rtc_config) { // Need to merge constraints into RTCConfiguration again, which are stored - // in the observer object. - PeerConnectionObserverJni* observer = - reinterpret_cast(native_observer); + // in the OwnedPeerConnection object. + OwnedPeerConnection* owned_pc = reinterpret_cast( + Java_PeerConnection_getNativeOwnedPeerConnection(jni, j_pc)); PeerConnectionInterface::RTCConfiguration rtc_config( PeerConnectionInterface::RTCConfigurationType::kAggressive); JavaToNativeRTCConfiguration(jni, j_rtc_config, &rtc_config); - if (observer && observer->constraints()) { - CopyConstraintsIntoRtcConfiguration(observer->constraints(), &rtc_config); + if (owned_pc->constraints()) { + CopyConstraintsIntoRtcConfiguration(owned_pc->constraints(), &rtc_config); } - return ExtractNativePC(jni, j_pc)->SetConfiguration(rtc_config); + return owned_pc->pc()->SetConfiguration(rtc_config); } static jboolean JNI_PeerConnection_AddIceCandidate( diff --git a/sdk/android/src/jni/pc/peerconnection.h b/sdk/android/src/jni/pc/peerconnection.h index 7e14b51306..6b021a0c46 100644 --- a/sdk/android/src/jni/pc/peerconnection.h +++ b/sdk/android/src/jni/pc/peerconnection.h @@ -61,9 +61,6 @@ class PeerConnectionObserverJni : public PeerConnectionObserver { const std::vector>& streams) override; - void SetConstraints(std::unique_ptr constraints); - const MediaConstraintsInterface* constraints() { return constraints_.get(); } - private: typedef std::map NativeToJavaStreamsMap; @@ -86,6 +83,36 @@ class PeerConnectionObserverJni : public PeerConnectionObserver { // C++ -> Java remote streams. NativeToJavaStreamsMap remote_streams_; std::vector rtp_receivers_; +}; + +// PeerConnection doesn't take ownership of the observer. In Java API, we don't +// want the client to have to manually dispose the observer. To solve this, this +// wrapper class is used for object ownership. +// +// Also stores reference to the deprecated PeerConnection constraints for now. +class OwnedPeerConnection { + public: + OwnedPeerConnection( + rtc::scoped_refptr peer_connection, + std::unique_ptr observer) + : OwnedPeerConnection(peer_connection, + std::move(observer), + nullptr /* constraints */) {} + // Deprecated. PC constraints are deprecated. + OwnedPeerConnection( + rtc::scoped_refptr peer_connection, + std::unique_ptr observer, + std::unique_ptr constraints); + ~OwnedPeerConnection(); + + PeerConnectionInterface* pc() const { return peer_connection_.get(); } + const MediaConstraintsInterface* constraints() const { + return constraints_.get(); + } + + private: + rtc::scoped_refptr peer_connection_; + std::unique_ptr observer_; std::unique_ptr constraints_; }; diff --git a/sdk/android/src/jni/pc/peerconnectionfactory.cc b/sdk/android/src/jni/pc/peerconnectionfactory.cc index b625df8401..633a0fec9b 100644 --- a/sdk/android/src/jni/pc/peerconnectionfactory.cc +++ b/sdk/android/src/jni/pc/peerconnectionfactory.cc @@ -417,6 +417,8 @@ static jlong JNI_PeerConnectionFactory_CreatePeerConnection( rtc::scoped_refptr f( reinterpret_cast( factoryFromJava(factory))); + std::unique_ptr observer( + reinterpret_cast(observer_p)); PeerConnectionInterface::RTCConfiguration rtc_config( PeerConnectionInterface::RTCConfigurationType::kAggressive); @@ -436,15 +438,15 @@ static jlong JNI_PeerConnectionFactory_CreatePeerConnection( rtc_config.certificates.push_back(certificate); } - PeerConnectionObserverJni* observer = - reinterpret_cast(observer_p); + std::unique_ptr constraints; if (!j_constraints.is_null()) { - observer->SetConstraints(JavaToNativeMediaConstraints(jni, j_constraints)); - CopyConstraintsIntoRtcConfiguration(observer->constraints(), &rtc_config); + constraints = JavaToNativeMediaConstraints(jni, j_constraints); + CopyConstraintsIntoRtcConfiguration(constraints.get(), &rtc_config); } rtc::scoped_refptr pc( - f->CreatePeerConnection(rtc_config, nullptr, nullptr, observer)); - return jlongFromPointer(pc.release()); + f->CreatePeerConnection(rtc_config, nullptr, nullptr, observer.get())); + return jlongFromPointer( + new OwnedPeerConnection(pc, std::move(observer), std::move(constraints))); } static jlong JNI_PeerConnectionFactory_CreateVideoSource(