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

webrtc/api/android/jni/peerconnection_jni.cc

 /*
  *  Copyright 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 155 matching lines @@
   // the below methods and we only override one of them.
   // TODO(deadbeef): Remove once there's only one version of the methods.
   using PeerConnectionObserver::OnAddStream;
   using PeerConnectionObserver::OnRemoveStream;
   using PeerConnectionObserver::OnDataChannel;
 
   PCOJava(JNIEnv* jni, jobject j_observer)
       : 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 {
     ScopedLocalRefFrame local_ref_frame(jni());
     std::string sdp;
     RTC_CHECK(candidate->ToString(&sdp)) << "got so far: " << sdp;
     jclass candidate_class = FindClass(jni(), "org/webrtc/IceCandidate");
     jmethodID ctor = GetMethodID(jni(), candidate_class,
         "<init>", "(Ljava/lang/String;ILjava/lang/String;)V");
     jstring j_mid = JavaStringFromStdString(jni(), candidate->sdp_mid());
@@ skipping 57 matching lines @@
         jni(), *j_observer_class_, "onIceGatheringChange",
         "(Lorg/webrtc/PeerConnection$IceGatheringState;)V");
     jobject new_state_enum = JavaEnumFromIndex(
         jni(), "PeerConnection$IceGatheringState", new_state);
     jni()->CallVoidMethod(*j_observer_global_, m, new_state_enum);
     CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
   }
 
   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());
       // Java AudioTrack holds one reference. Corresponding Release() is in
       // MediaStreamTrack_free, triggered by AudioTrack.dispose().
       track->AddRef();
       jobject j_track =
           jni()->NewObject(*j_audio_track_class_, j_audio_track_ctor_,
                            reinterpret_cast<jlong>(track.get()), id);
       CHECK_EXCEPTION(jni()) << "error during NewObject";
@@ skipping 26 matching lines @@
                                             "Ljava/util/LinkedList;");
       jobject video_tracks = GetObjectField(jni(), j_stream, video_tracks_id);
       jmethodID add = GetMethodID(jni(),
                                   GetObjectClass(jni(), video_tracks),
                                   "add",
                                   "(Ljava/lang/Object;)Z");
       jboolean added = jni()->CallBooleanMethod(video_tracks, add, j_track);
       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");
     jni()->CallVoidMethod(*j_observer_global_, m, j_stream);
     CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
   }
 
   void OnRemoveStream(
       rtc::scoped_refptr<MediaStreamInterface> stream) override {
     ScopedLocalRefFrame local_ref_frame(jni());
@@ skipping 33 matching lines @@
   }
 
   void OnRenegotiationNeeded() override {
     ScopedLocalRefFrame local_ref_frame(jni());
     jmethodID m =
         GetMethodID(jni(), *j_observer_class_, "onRenegotiationNeeded", "()V");
     jni()->CallVoidMethod(*j_observer_global_, m);
     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);
   }
 
   const ConstraintsWrapper* constraints() { return constraints_.get(); }
 
  private:
   typedef std::map<MediaStreamInterface*, jobject> NativeToJavaStreamsMap;
+  typedef std::map<RtpReceiverInterface*, jobject> NativeToJavaRtpReceiverMap;
 
   void DisposeRemoteStream(const NativeToJavaStreamsMap::iterator& it) {
     jobject j_stream = it->second;
     remote_streams_.erase(it);
     jni()->CallVoidMethod(
         j_stream, GetMethodID(jni(), *j_media_stream_class_, "dispose", "()V"));
     CHECK_EXCEPTION(jni()) << "error during MediaStream.dispose()";
     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) {
     std::string sdp = webrtc::SdpSerializeCandidate(candidate);
     RTC_CHECK(!sdp.empty()) << "got an empty ICE candidate";
     jmethodID ctor = GetMethodID(jni, *candidate_class, "<init>",
                                  "(Ljava/lang/String;ILjava/lang/String;)V");
     jstring j_mid = JavaStringFromStdString(jni, candidate.transport_name());
     jstring j_sdp = JavaStringFromStdString(jni, sdp);
     // sdp_mline_index is not used, pass an invalid value -1.
@@ skipping 10 matching lines @@
     jobjectArray java_candidates =
         jni->NewObjectArray(candidates.size(), candidate_class, NULL);
     int i = 0;
     for (const cricket::Candidate& candidate : candidates) {
       jobject j_candidate = ToJavaCandidate(jni, &candidate_class, candidate);
       jni->SetObjectArrayElement(java_candidates, i++, j_candidate);
     }
     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();
   }
 
   const ScopedGlobalRef<jobject> j_observer_global_;
   const ScopedGlobalRef<jclass> j_observer_class_;
   const ScopedGlobalRef<jclass> j_media_stream_class_;
   const jmethodID j_media_stream_ctor_;
   const ScopedGlobalRef<jclass> j_audio_track_class_;
   const jmethodID j_audio_track_ctor_;
   const ScopedGlobalRef<jclass> j_video_track_class_;
   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_;
 };
 
 // Wrapper for a Java MediaConstraints object.  Copies all needed data so when
 // the constructor returns the Java object is no longer needed.
 class ConstraintsWrapper : public MediaConstraintsInterface {
  public:
   ConstraintsWrapper(JNIEnv* jni, jobject j_constraints) {
     PopulateConstraintsFromJavaPairList(
         jni, j_constraints, "mandatory", &mandatory_);
@@ skipping 1952 matching lines @@
   return JavaStringFromStdString(
       jni,
       reinterpret_cast<RtpReceiverInterface*>(j_rtp_receiver_pointer)->id());
 }
 
 JOW(void, RtpReceiver_free)(JNIEnv* jni, jclass, jlong j_rtp_receiver_pointer) {
   reinterpret_cast<RtpReceiverInterface*>(j_rtp_receiver_pointer)->Release();
 }
 
 }  // namespace webrtc_jni