From dcccda7e7c5e0e68cc0e8b6b401d5b1a119893c2 Mon Sep 17 00:00:00 2001 From: zhihuang Date: Wed, 21 Dec 2016 14:08:03 -0800 Subject: [PATCH] Created a java wrapper for the callback OnAddTrack to PeerConnection.Observer Created a java wrapper for the callback OnAddTrack in this CL since it has been added to native C++ API The callback function is called when a track is signaled by remote side and a new RtpReceiver is created. The application can tell when tracks are added to the streams by listening to this callback. BUG=webrtc:6112 Review-Url: https://codereview.webrtc.org/2513723002 Cr-Commit-Position: refs/heads/master@{#15745} --- .../appspot/apprtc/PeerConnectionClient.java | 4 + .../api/org/webrtc/PeerConnection.java | 6 + .../src/org/webrtc/PeerConnectionTest.java | 171 +++++++++--------- .../sdk/android/src/jni/peerconnection_jni.cc | 106 +++++++++-- 4 files changed, 178 insertions(+), 109 deletions(-) diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java index 61784a0fd3..cc5e457cbf 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java @@ -41,6 +41,7 @@ import org.webrtc.PeerConnection; import org.webrtc.PeerConnection.IceConnectionState; import org.webrtc.PeerConnectionFactory; import org.webrtc.RtpParameters; +import org.webrtc.RtpReceiver; import org.webrtc.RtpSender; import org.webrtc.SdpObserver; import org.webrtc.SessionDescription; @@ -1175,6 +1176,9 @@ public class PeerConnectionClient { // No need to do anything; AppRTC follows a pre-agreed-upon // signaling/negotiation protocol. } + + @Override + public void onAddTrack(final RtpReceiver receiver, final MediaStream[] mediaStreams) {} } // Implementation detail: handle offer creation/signaling and answer setting, diff --git a/webrtc/sdk/android/api/org/webrtc/PeerConnection.java b/webrtc/sdk/android/api/org/webrtc/PeerConnection.java index ab08598519..b4ad5795fe 100644 --- a/webrtc/sdk/android/api/org/webrtc/PeerConnection.java +++ b/webrtc/sdk/android/api/org/webrtc/PeerConnection.java @@ -80,6 +80,12 @@ public class PeerConnection { /** Triggered when renegotiation is necessary. */ public void onRenegotiationNeeded(); + + /** + * Triggered when a new track is signaled by the remote peer, as a result of + * setRemoteDescription. + */ + public void onAddTrack(RtpReceiver receiver, MediaStream[] mediaStreams); } /** Java version of PeerConnectionInterface.IceServer. */ diff --git a/webrtc/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java b/webrtc/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java index 59809365d4..845f54580b 100644 --- a/webrtc/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java +++ b/webrtc/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java @@ -53,6 +53,7 @@ public class PeerConnectionTest extends ActivityTestCase { private int expectedWidth = 0; private int expectedHeight = 0; private int expectedFramesDelivered = 0; + private int expectedTracksAdded = 0; private LinkedList expectedSignalingChanges = new LinkedList(); private LinkedList expectedIceConnectionChanges = new LinkedList(); @@ -227,6 +228,15 @@ public class PeerConnectionTest extends ActivityTestCase { assertTrue(--expectedRenegotiations >= 0); } + public synchronized void expectAddTrack(int expectedTracksAdded) { + this.expectedTracksAdded = expectedTracksAdded; + } + + @Override + public synchronized void onAddTrack(RtpReceiver receiver, MediaStream[] mediaStreams) { + expectedTracksAdded--; + } + public synchronized void expectMessage(ByteBuffer expectedBuffer, boolean expectedBinary) { expectedBuffers.add(new DataChannel.Buffer(expectedBuffer, expectedBinary)); } @@ -339,6 +349,9 @@ public class PeerConnectionTest extends ActivityTestCase { if (expectedFirstVideoPacket > 0) { stillWaitingForExpectations.add("expectedFirstVideoPacket: " + expectedFirstVideoPacket); } + if (expectedTracksAdded != 0) { + stillWaitingForExpectations.add("expectedAddedTrack: " + expectedTracksAdded); + } return stillWaitingForExpectations; } @@ -571,6 +584,9 @@ public class PeerConnectionTest extends ActivityTestCase { addTracksToPC(factory, offeringPC, videoSource, "offeredMediaStream", "offeredVideoTrack", "offeredAudioTrack", new ExpectedResolutionSetter(answeringExpectations)); + offeringExpectations.expectAddTrack(2); + answeringExpectations.expectAddTrack(2); + offeringExpectations.expectRenegotiationNeeded(); DataChannel offeringDC = offeringPC.createDataChannel("offeringDC", new DataChannel.Init()); assertEquals("offeringDC", offeringDC.label()); @@ -751,7 +767,7 @@ public class PeerConnectionTest extends ActivityTestCase { } @MediumTest - public void testTrackRemoval() throws Exception { + public void testTrackRemovalAndAddition() throws Exception { // Allow loopback interfaces too since our Android devices often don't // have those. PeerConnectionFactory.Options options = new PeerConnectionFactory.Options(); @@ -788,6 +804,8 @@ public class PeerConnectionTest extends ActivityTestCase { addTracksToPC(factory, offeringPC, videoSource, "offeredMediaStream", "offeredVideoTrack", "offeredAudioTrack", new ExpectedResolutionSetter(answeringExpectations)); + offeringExpectations.expectAddTrack(2); + answeringExpectations.expectAddTrack(2); // Create offer. SdpObserverLatch sdpLatch = new SdpObserverLatch(); offeringPC.createOffer(sdpLatch, new MediaConstraints()); @@ -886,50 +904,7 @@ public class PeerConnectionTest extends ActivityTestCase { // Note that when we call removeTrack, we regain responsibility for // disposing of the track. oLMS.get().removeTrack(offererVideoTrack); - - // Create offer. - sdpLatch = new SdpObserverLatch(); - offeringPC.createOffer(sdpLatch, new MediaConstraints()); - assertTrue(sdpLatch.await()); - offerSdp = sdpLatch.getSdp(); - assertEquals(offerSdp.type, SessionDescription.Type.OFFER); - assertFalse(offerSdp.description.isEmpty()); - - // Set local description for offerer. - sdpLatch = new SdpObserverLatch(); - offeringExpectations.expectSignalingChange(SignalingState.HAVE_LOCAL_OFFER); - offeringPC.setLocalDescription(sdpLatch, offerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); - - // Set remote description for answerer. - sdpLatch = new SdpObserverLatch(); - answeringExpectations.expectSignalingChange(SignalingState.HAVE_REMOTE_OFFER); - answeringPC.setRemoteDescription(sdpLatch, offerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); - - // Create answer. - sdpLatch = new SdpObserverLatch(); - answeringPC.createAnswer(sdpLatch, new MediaConstraints()); - assertTrue(sdpLatch.await()); - answerSdp = sdpLatch.getSdp(); - assertEquals(answerSdp.type, SessionDescription.Type.ANSWER); - assertFalse(answerSdp.description.isEmpty()); - - // Set local description for answerer. - sdpLatch = new SdpObserverLatch(); - answeringExpectations.expectSignalingChange(SignalingState.STABLE); - answeringPC.setLocalDescription(sdpLatch, answerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); - - // Set remote description for offerer. - sdpLatch = new SdpObserverLatch(); - offeringExpectations.expectSignalingChange(SignalingState.STABLE); - offeringPC.setRemoteDescription(sdpLatch, answerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); + negotiate(offeringPC, offeringExpectations, answeringPC, answeringExpectations); // Make sure the track was really removed. // TODO(deadbeef): Currently the expectation is that the video track's @@ -939,56 +914,24 @@ public class PeerConnectionTest extends ActivityTestCase { MediaStream aRMS = answeringExpectations.gotRemoteStreams.iterator().next(); assertEquals(aRMS.videoTracks.get(0).state(), MediaStreamTrack.State.ENDED); - // Finally, remove the audio track as well, which should completely remove - // the remote stream. This used to trigger an assert. + // Add the video track to test if the answeringPC will create a new track + // for the updated remote description. + oLMS.get().addTrack(offererVideoTrack); + // The answeringPC sets the updated remote description with a track added. + // So the onAddTrack callback is expected to be called once. + answeringExpectations.expectAddTrack(1); + offeringExpectations.expectAddTrack(0); + negotiate(offeringPC, offeringExpectations, answeringPC, answeringExpectations); + + // Finally, remove both the audio and video tracks, which should completely + // remove the remote stream. This used to trigger an assert. // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=5128 + oLMS.get().removeTrack(offererVideoTrack); AudioTrack offererAudioTrack = oLMS.get().audioTracks.get(0); oLMS.get().removeTrack(offererAudioTrack); - // Create offer. - sdpLatch = new SdpObserverLatch(); - offeringPC.createOffer(sdpLatch, new MediaConstraints()); - assertTrue(sdpLatch.await()); - offerSdp = sdpLatch.getSdp(); - assertEquals(offerSdp.type, SessionDescription.Type.OFFER); - assertFalse(offerSdp.description.isEmpty()); - - // Set local description for offerer. - sdpLatch = new SdpObserverLatch(); - offeringExpectations.expectSignalingChange(SignalingState.HAVE_LOCAL_OFFER); - offeringPC.setLocalDescription(sdpLatch, offerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); - - // Set remote description for answerer. - sdpLatch = new SdpObserverLatch(); - answeringExpectations.expectSignalingChange(SignalingState.HAVE_REMOTE_OFFER); answeringExpectations.expectRemoveStream("offeredMediaStream"); - answeringPC.setRemoteDescription(sdpLatch, offerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); - - // Create answer. - sdpLatch = new SdpObserverLatch(); - answeringPC.createAnswer(sdpLatch, new MediaConstraints()); - assertTrue(sdpLatch.await()); - answerSdp = sdpLatch.getSdp(); - assertEquals(answerSdp.type, SessionDescription.Type.ANSWER); - assertFalse(answerSdp.description.isEmpty()); - - // Set local description for answerer. - sdpLatch = new SdpObserverLatch(); - answeringExpectations.expectSignalingChange(SignalingState.STABLE); - answeringPC.setLocalDescription(sdpLatch, answerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); - - // Set remote description for offerer. - sdpLatch = new SdpObserverLatch(); - offeringExpectations.expectSignalingChange(SignalingState.STABLE); - offeringPC.setRemoteDescription(sdpLatch, answerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); + negotiate(offeringPC, offeringExpectations, answeringPC, answeringExpectations); // Make sure the stream was really removed. assertTrue(answeringExpectations.gotRemoteStreams.isEmpty()); @@ -1007,6 +950,54 @@ public class PeerConnectionTest extends ActivityTestCase { System.gc(); } + private static void negotiate(PeerConnection offeringPC, + ObserverExpectations offeringExpectations, PeerConnection answeringPC, + ObserverExpectations answeringExpectations) { + // Create offer. + SdpObserverLatch sdpLatch = new SdpObserverLatch(); + offeringPC.createOffer(sdpLatch, new MediaConstraints()); + assertTrue(sdpLatch.await()); + SessionDescription offerSdp = sdpLatch.getSdp(); + assertEquals(offerSdp.type, SessionDescription.Type.OFFER); + assertFalse(offerSdp.description.isEmpty()); + + // Set local description for offerer. + sdpLatch = new SdpObserverLatch(); + offeringExpectations.expectSignalingChange(SignalingState.HAVE_LOCAL_OFFER); + offeringPC.setLocalDescription(sdpLatch, offerSdp); + assertTrue(sdpLatch.await()); + assertNull(sdpLatch.getSdp()); + + // Set remote description for answerer. + sdpLatch = new SdpObserverLatch(); + answeringExpectations.expectSignalingChange(SignalingState.HAVE_REMOTE_OFFER); + answeringPC.setRemoteDescription(sdpLatch, offerSdp); + assertTrue(sdpLatch.await()); + assertNull(sdpLatch.getSdp()); + + // Create answer. + sdpLatch = new SdpObserverLatch(); + answeringPC.createAnswer(sdpLatch, new MediaConstraints()); + assertTrue(sdpLatch.await()); + SessionDescription answerSdp = sdpLatch.getSdp(); + assertEquals(answerSdp.type, SessionDescription.Type.ANSWER); + assertFalse(answerSdp.description.isEmpty()); + + // Set local description for answerer. + sdpLatch = new SdpObserverLatch(); + answeringExpectations.expectSignalingChange(SignalingState.STABLE); + answeringPC.setLocalDescription(sdpLatch, answerSdp); + assertTrue(sdpLatch.await()); + assertNull(sdpLatch.getSdp()); + + // Set remote description for offerer. + sdpLatch = new SdpObserverLatch(); + offeringExpectations.expectSignalingChange(SignalingState.STABLE); + offeringPC.setRemoteDescription(sdpLatch, answerSdp); + assertTrue(sdpLatch.await()); + assertNull(sdpLatch.getSdp()); + } + private static void getMetrics() { Metrics metrics = Metrics.getAndReset(); assertTrue(metrics.map.size() > 0); diff --git a/webrtc/sdk/android/src/jni/peerconnection_jni.cc b/webrtc/sdk/android/src/jni/peerconnection_jni.cc index c9c8ed35af..88c4f3ff46 100644 --- a/webrtc/sdk/android/src/jni/peerconnection_jni.cc +++ b/webrtc/sdk/android/src/jni/peerconnection_jni.cc @@ -174,23 +174,27 @@ class PCOJava : public PeerConnectionObserver { : j_observer_global_(jni, j_observer), j_observer_class_(jni, GetObjectClass(jni, *j_observer_global_)), j_media_stream_class_(jni, FindClass(jni, "org/webrtc/MediaStream")), - j_media_stream_ctor_(GetMethodID( - jni, *j_media_stream_class_, "", "(J)V")), + j_media_stream_ctor_( + GetMethodID(jni, *j_media_stream_class_, "", "(J)V")), j_audio_track_class_(jni, FindClass(jni, "org/webrtc/AudioTrack")), - j_audio_track_ctor_(GetMethodID( - jni, *j_audio_track_class_, "", "(J)V")), + j_audio_track_ctor_( + GetMethodID(jni, *j_audio_track_class_, "", "(J)V")), j_video_track_class_(jni, FindClass(jni, "org/webrtc/VideoTrack")), - j_video_track_ctor_(GetMethodID( - jni, *j_video_track_class_, "", "(J)V")), + j_video_track_ctor_( + GetMethodID(jni, *j_video_track_class_, "", "(J)V")), j_data_channel_class_(jni, FindClass(jni, "org/webrtc/DataChannel")), - j_data_channel_ctor_(GetMethodID( - jni, *j_data_channel_class_, "", "(J)V")) { - } + j_data_channel_ctor_( + GetMethodID(jni, *j_data_channel_class_, "", "(J)V")), + j_rtp_receiver_class_(jni, FindClass(jni, "org/webrtc/RtpReceiver")), + j_rtp_receiver_ctor_( + GetMethodID(jni, *j_rtp_receiver_class_, "", "(J)V")) {} virtual ~PCOJava() { ScopedLocalRefFrame local_ref_frame(jni()); while (!remote_streams_.empty()) DisposeRemoteStream(remote_streams_.begin()); + while (!rtp_receivers_.empty()) + DisposeRtpReceiver(rtp_receivers_.begin()); } void OnIceCandidate(const IceCandidateInterface* candidate) override { @@ -268,13 +272,9 @@ class PCOJava : public PeerConnectionObserver { void OnAddStream(rtc::scoped_refptr stream) override { ScopedLocalRefFrame local_ref_frame(jni()); - // Java MediaStream holds one reference. Corresponding Release() is in - // MediaStream_free, triggered by MediaStream.dispose(). - stream->AddRef(); - jobject j_stream = - jni()->NewObject(*j_media_stream_class_, j_media_stream_ctor_, - reinterpret_cast(stream.get())); - CHECK_EXCEPTION(jni()) << "error during NewObject"; + // The stream could be added into the remote_streams_ map when calling + // OnAddTrack. + jobject j_stream = GetOrCreateJavaStream(stream); for (const auto& track : stream->GetAudioTracks()) { jstring id = JavaStringFromStdString(jni(), track->id()); @@ -321,7 +321,6 @@ class PCOJava : public PeerConnectionObserver { CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod"; RTC_CHECK(added); } - remote_streams_[stream] = NewGlobalRef(jni(), j_stream); jmethodID m = GetMethodID(jni(), *j_observer_class_, "onAddStream", "(Lorg/webrtc/MediaStream;)V"); @@ -349,8 +348,9 @@ class PCOJava : public PeerConnectionObserver { void OnDataChannel( rtc::scoped_refptr channel) override { ScopedLocalRefFrame local_ref_frame(jni()); - jobject j_channel = jni()->NewObject( - *j_data_channel_class_, j_data_channel_ctor_, (jlong)channel.get()); + jobject j_channel = + jni()->NewObject(*j_data_channel_class_, j_data_channel_ctor_, + jlongFromPointer(channel.get())); CHECK_EXCEPTION(jni()) << "error during NewObject"; jmethodID m = GetMethodID(jni(), *j_observer_class_, "onDataChannel", @@ -375,6 +375,26 @@ class PCOJava : public PeerConnectionObserver { CHECK_EXCEPTION(jni()) << "error during CallVoidMethod"; } + void OnAddTrack(rtc::scoped_refptr receiver, + const std::vector>& + streams) override { + ScopedLocalRefFrame local_ref_frame(jni()); + jobject j_rtp_receiver = + jni()->NewObject(*j_rtp_receiver_class_, j_rtp_receiver_ctor_, + jlongFromPointer(receiver.get())); + CHECK_EXCEPTION(jni()) << "error during NewObject"; + receiver->AddRef(); + rtp_receivers_[receiver] = NewGlobalRef(jni(), j_rtp_receiver); + + jobjectArray j_stream_array = ToJavaMediaStreamArray(jni(), streams); + jmethodID m = + GetMethodID(jni(), *j_observer_class_, "onAddTrack", + "(Lorg/webrtc/RtpReceiver;[Lorg/webrtc/MediaStream;)V"); + jni()->CallVoidMethod(*j_observer_global_, m, j_rtp_receiver, + j_stream_array); + CHECK_EXCEPTION(jni()) << "Error during CallVoidMethod"; + } + void SetConstraints(ConstraintsWrapper* constraints) { RTC_CHECK(!constraints_.get()) << "constraints already set!"; constraints_.reset(constraints); @@ -384,6 +404,7 @@ class PCOJava : public PeerConnectionObserver { private: typedef std::map NativeToJavaStreamsMap; + typedef std::map NativeToJavaRtpReceiverMap; void DisposeRemoteStream(const NativeToJavaStreamsMap::iterator& it) { jobject j_stream = it->second; @@ -394,6 +415,16 @@ class PCOJava : public PeerConnectionObserver { DeleteGlobalRef(jni(), j_stream); } + void DisposeRtpReceiver(const NativeToJavaRtpReceiverMap::iterator& it) { + jobject j_rtp_receiver = it->second; + rtp_receivers_.erase(it); + jni()->CallVoidMethod( + j_rtp_receiver, + GetMethodID(jni(), *j_rtp_receiver_class_, "dispose", "()V")); + CHECK_EXCEPTION(jni()) << "error during RtpReceiver.dispose()"; + DeleteGlobalRef(jni(), j_rtp_receiver); + } + jobject ToJavaCandidate(JNIEnv* jni, jclass* candidate_class, const cricket::Candidate& candidate) { @@ -424,6 +455,40 @@ class PCOJava : public PeerConnectionObserver { return java_candidates; } + jobjectArray ToJavaMediaStreamArray( + JNIEnv* jni, + const std::vector>& streams) { + jobjectArray java_streams = + jni->NewObjectArray(streams.size(), *j_media_stream_class_, nullptr); + CHECK_EXCEPTION(jni) << "error during NewObjectArray"; + for (size_t i = 0; i < streams.size(); ++i) { + jobject j_stream = GetOrCreateJavaStream(streams[i]); + jni->SetObjectArrayElement(java_streams, i, j_stream); + } + return java_streams; + } + + // If the NativeToJavaStreamsMap contains the stream, return it. + // Otherwise, create a new Java MediaStream. + jobject GetOrCreateJavaStream( + const rtc::scoped_refptr& stream) { + NativeToJavaStreamsMap::iterator it = remote_streams_.find(stream); + if (it != remote_streams_.end()) { + return it->second; + } + + // Java MediaStream holds one reference. Corresponding Release() is in + // MediaStream_free, triggered by MediaStream.dispose(). + stream->AddRef(); + jobject j_stream = + jni()->NewObject(*j_media_stream_class_, j_media_stream_ctor_, + reinterpret_cast(stream.get())); + CHECK_EXCEPTION(jni()) << "error during NewObject"; + + remote_streams_[stream] = NewGlobalRef(jni(), j_stream); + return j_stream; + } + JNIEnv* jni() { return AttachCurrentThreadIfNeeded(); } @@ -438,9 +503,12 @@ class PCOJava : public PeerConnectionObserver { const jmethodID j_video_track_ctor_; const ScopedGlobalRef j_data_channel_class_; const jmethodID j_data_channel_ctor_; + const ScopedGlobalRef j_rtp_receiver_class_; + const jmethodID j_rtp_receiver_ctor_; // C++ -> Java remote streams. The stored jobects are global refs and must be // manually deleted upon removal. Use DisposeRemoteStream(). NativeToJavaStreamsMap remote_streams_; + NativeToJavaRtpReceiverMap rtp_receivers_; std::unique_ptr constraints_; };