Created a java wrapper for the callback OnAddTrack to PeerConnection.Observer

webrtc/api/android/jni/peerconnection_jni.cc

Index: webrtc/api/android/jni/peerconnection_jni.cc
diff --git a/webrtc/api/android/jni/peerconnection_jni.cc b/webrtc/api/android/jni/peerconnection_jni.cc
index 7012fc355a3f24257abf2326d918ed41e173ab9c..ce85e0776aabf5575f2840d1d59212c7ef45b136 100644
--- a/webrtc/api/android/jni/peerconnection_jni.cc
+++ b/webrtc/api/android/jni/peerconnection_jni.cc
@@ -173,23 +173,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_, "<init>", "(J)V")),
+        j_media_stream_ctor_(
+            GetMethodID(jni, *j_media_stream_class_, "<init>", "(J)V")),
         j_audio_track_class_(jni, FindClass(jni, "org/webrtc/AudioTrack")),
-        j_audio_track_ctor_(GetMethodID(
-            jni, *j_audio_track_class_, "<init>", "(J)V")),
+        j_audio_track_ctor_(
+            GetMethodID(jni, *j_audio_track_class_, "<init>", "(J)V")),
         j_video_track_class_(jni, FindClass(jni, "org/webrtc/VideoTrack")),
-        j_video_track_ctor_(GetMethodID(
-            jni, *j_video_track_class_, "<init>", "(J)V")),
+        j_video_track_ctor_(
+            GetMethodID(jni, *j_video_track_class_, "<init>", "(J)V")),
         j_data_channel_class_(jni, FindClass(jni, "org/webrtc/DataChannel")),
-        j_data_channel_ctor_(GetMethodID(
-            jni, *j_data_channel_class_, "<init>", "(J)V")) {
-  }
+        j_data_channel_ctor_(
+            GetMethodID(jni, *j_data_channel_class_, "<init>", "(J)V")),
+        j_rtp_receiver_class_(jni, FindClass(jni, "org/webrtc/RtpReceiver")),
+        j_rtp_receiver_ctor_(
+            GetMethodID(jni, *j_rtp_receiver_class_, "<init>", "(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 {
@@ -267,13 +271,9 @@ class PCOJava : public PeerConnectionObserver {
 
   void OnAddStream(rtc::scoped_refptr<MediaStreamInterface> 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<jlong>(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());
@@ -320,7 +320,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");
@@ -374,6 +373,25 @@ class PCOJava : public PeerConnectionObserver {
     CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
   }
 
+  void OnAddTrack(
+      rtc::scoped_refptr<RtpReceiverInterface> receiver,
+      std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams) override {

[magjed_webrtc, 2016/12/02 12:19:46]

this should be const-ref.

[Zhi Huang, 2016/12/03 00:03:02]

Oh this is my fault. Hopefully it's not too late to change the C++ interface.

Thanks for pointing out.

+    ScopedLocalRefFrame local_ref_frame(jni());
+    jobject j_rtp_receiver = jni()->NewObject(
+        *j_rtp_receiver_class_, j_rtp_receiver_ctor_, (jlong)receiver.get());

[magjed_webrtc, 2016/12/02 12:19:46]

It's dangerous to cast the pointer to a jlong that way, you need to use the
jlongFromPointer() function. Same for the other places in this CL where a cast
to jlong is needed.

[Zhi Huang, 2016/12/03 00:03:03]

Done.

There are many places, besides this CL, using (jlong) for casting. Maybe it
makes sense to make a separate CL to replacing all the dangerous usage.

+    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);
@@ -383,6 +401,7 @@ class PCOJava : public PeerConnectionObserver {
 
  private:
   typedef std::map<MediaStreamInterface*, jobject> NativeToJavaStreamsMap;
+  typedef std::map<RtpReceiverInterface*, jobject> NativeToJavaRtpReceiverMap;
 
   void DisposeRemoteStream(const NativeToJavaStreamsMap::iterator& it) {
     jobject j_stream = it->second;
@@ -393,6 +412,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"));

[skvlad, 2016/11/28 21:35:42]

Calling dispose() from Java will call Release() on the RefCountInterface each
time, potentially bringing it down to zero while it is still in use. Either make
dispose() private (so that only JNI code can call it, not Java clients), or add
a check to prevent it from being called more than once.

[Taylor Brandstetter, 2016/11/28 23:15:12]

That's possible before this CL, though. I agree it's bad, but it's a fundamental
problem with how "dispose" works in our Java API, and I think it needs to be
addressed separately.

+    CHECK_EXCEPTION(jni()) << "error during RtpReceiver.dispose()";
+    DeleteGlobalRef(jni(), j_rtp_receiver);

[skvlad, 2016/11/28 21:35:42]

You could just store ScopedGlobalRefs in the map and avoid explicit creation and
destruction of global refs. Either way is fine though.

+  }
+
   jobject ToJavaCandidate(JNIEnv* jni,
                           jclass* candidate_class,
                           const cricket::Candidate& candidate) {
@@ -423,6 +452,41 @@ class PCOJava : public PeerConnectionObserver {
     return java_candidates;
   }
 
+  jobjectArray ToJavaMediaStreamArray(
+      JNIEnv* jni,
+      std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams) {

[magjed_webrtc, 2016/12/02 12:19:46]

const-ref

[Zhi Huang, 2016/12/03 00:03:02]

Done.

+    jobjectArray java_streams =
+        jni->NewObjectArray(streams.size(), *j_media_stream_class_, nullptr);
+    CHECK_EXCEPTION(jni) << "error during NewObjectArray";
+    int i = 0;
+    for (rtc::scoped_refptr<MediaStreamInterface> stream : streams) {

[magjed_webrtc, 2016/12/02 12:19:47]

I think it would be cleaner with:
for (size_t i = 0; i < streams.size(); ++i) {
  jobject j_stream = GetOrCreateJavaStream(streams[i]);
  jni->SetObjectArrayElement(java_streams, i, j_stream);
}

[Zhi Huang, 2016/12/03 00:03:02]

Done.

+      jobject j_stream = GetOrCreateJavaStream(stream);
+      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(
+      rtc::scoped_refptr<MediaStreamInterface> stream) {

[magjed_webrtc, 2016/12/02 12:19:46]

Use const-ref, or raw MediaStreamInterface*, I'm not sure what's best. This is a
very unusual case where we do manual AddRef().

[Zhi Huang, 2016/12/03 00:03:02]

Done.

+    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<jlong>(stream.get()));
+    CHECK_EXCEPTION(jni()) << "error during NewObject";
+
+    remote_streams_[stream] = NewGlobalRef(jni(), j_stream);
+    return j_stream;
+  }
+
   JNIEnv* jni() {
     return AttachCurrentThreadIfNeeded();
   }
@@ -437,9 +501,12 @@ class PCOJava : public PeerConnectionObserver {
   const jmethodID j_video_track_ctor_;
   const ScopedGlobalRef<jclass> j_data_channel_class_;
   const jmethodID j_data_channel_ctor_;
+  const ScopedGlobalRef<jclass> 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<ConstraintsWrapper> constraints_;
 };