Add PeerConnectionObserver#onRemoveTrack to android sdk

webrtc/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc

Index: webrtc/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc
diff --git a/webrtc/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc b/webrtc/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc
index 18767925933ffb6006845d83b1fd2d3edb0954f5..e48fa41d3bf9ceab8fc06109f164c2e482465dd3 100644
--- a/webrtc/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc
+++ b/webrtc/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc
@@ -10,6 +10,7 @@
 
 #include "webrtc/sdk/android/src/jni/pc/peerconnectionobserver_jni.h"
 
+#include <webrtc/pc/mediastreamobserver.h>
 #include <string>
 
 #include "webrtc/sdk/android/src/jni/classreferenceholder.h"
@@ -38,17 +39,18 @@ PeerConnectionObserverJni::PeerConnectionObserverJni(JNIEnv* jni,
           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_rtp_receiver_class_(jni, FindClass(jni, "org/webrtc/RtpReceiver")),
-      j_rtp_receiver_ctor_(
-          GetMethodID(jni, *j_rtp_receiver_class_, "<init>", "(J)V")) {}
+          GetMethodID(jni, *j_data_channel_class_, "<init>", "(J)V")) {}
 
 PeerConnectionObserverJni::~PeerConnectionObserverJni() {
   ScopedLocalRefFrame local_ref_frame(jni());
+  stream_observers_.clear();
+  while (!remote_tracks_.empty()) {
+    NativeToJavaMediaTrackMap::iterator it = remote_tracks_.begin();
+    DeleteGlobalRef(jni(), it->second);
+    remote_tracks_.erase(it);
+  }
   while (!remote_streams_.empty())
     DisposeRemoteStream(remote_streams_.begin());
-  while (!rtp_receivers_.empty())
-    DisposeRtpReceiver(rtp_receivers_.begin());
 }
 
 void PeerConnectionObserverJni::OnIceCandidate(
@@ -132,47 +134,155 @@ void PeerConnectionObserverJni::OnAddStream(
   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";
-    jfieldID audio_tracks_id = GetFieldID(
-        jni(), *j_media_stream_class_, "audioTracks", "Ljava/util/LinkedList;");
-    jobject audio_tracks = GetObjectField(jni(), j_stream, audio_tracks_id);
-    jmethodID add = GetMethodID(jni(), GetObjectClass(jni(), audio_tracks),
-                                "add", "(Ljava/lang/Object;)Z");
-    jboolean added = jni()->CallBooleanMethod(audio_tracks, add, j_track);
-    CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
-    RTC_CHECK(added);
+    OnAudioTrackAdded(track, stream);
   }
 
   for (const auto& track : stream->GetVideoTracks()) {
-    jstring id = JavaStringFromStdString(jni(), track->id());
-    // Java VideoTrack holds one reference. Corresponding Release() is in
-    // MediaStreamTrack_free, triggered by VideoTrack.dispose().
-    track->AddRef();
-    jobject j_track =
-        jni()->NewObject(*j_video_track_class_, j_video_track_ctor_,
-                         reinterpret_cast<jlong>(track.get()), id);
-    CHECK_EXCEPTION(jni()) << "error during NewObject";
-    jfieldID video_tracks_id = GetFieldID(
-        jni(), *j_media_stream_class_, "videoTracks", "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);
+    OnVideoTrackAdded(track, 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";
+
+  webrtc::MediaStreamObserver* observer =
+      new webrtc::MediaStreamObserver(stream);
+  observer->SignalVideoTrackAdded.connect(
+      this, &PeerConnectionObserverJni::OnVideoTrackAdded);
+  observer->SignalAudioTrackAdded.connect(
+      this, &PeerConnectionObserverJni::OnAudioTrackAdded);
+  observer->SignalAudioTrackRemoved.connect(
+      this, &PeerConnectionObserverJni::OnAudioTrackRemoved);
+  observer->SignalVideoTrackRemoved.connect(
+      this, &PeerConnectionObserverJni::OnVideoTrackRemoved);
+  stream_observers_.push_back(
+      std::unique_ptr<webrtc::MediaStreamObserver>(observer));
+}
+
+void PeerConnectionObserverJni::OnAudioTrackAdded(
+    webrtc::AudioTrackInterface* track,
+    webrtc::MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jstring id = JavaStringFromStdString(jni(), track->id());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+  // 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), id);
+  CHECK_EXCEPTION(jni()) << "error during NewObject";
+  remote_tracks_[track] = NewGlobalRef(jni(), j_track);
+  jfieldID audio_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "audioTracks", "Ljava/util/LinkedList;");
+  jobject audio_tracks = GetObjectField(jni(), j_stream, audio_tracks_id);
+  jmethodID add = GetMethodID(jni(), GetObjectClass(jni(), audio_tracks), "add",
+                              "(Ljava/lang/Object;)Z");
+  jboolean added = jni()->CallBooleanMethod(audio_tracks, add, j_track);
+  CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
+  RTC_CHECK(added);
+
+  std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>> streams;
+  streams.push_back(rtc::scoped_refptr<webrtc::MediaStreamInterface>(stream));
+  jobjectArray j_stream_array = NativeToJavaMediaStreamArray(jni(), streams);
+  jmethodID m =
+      GetMethodID(jni(), *j_observer_class_, "onAddTrack",
+                  "(Lorg/webrtc/MediaStreamTrack;[Lorg/webrtc/MediaStream;)V");
+  jni()->CallVoidMethod(*j_observer_global_, m, j_track, j_stream_array);
+  CHECK_EXCEPTION(jni()) << "Error during CallVoidMethod";
+}
+
+void PeerConnectionObserverJni::OnVideoTrackAdded(
+    webrtc::VideoTrackInterface* track,
+    webrtc::MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+
+  jstring id = JavaStringFromStdString(jni(), track->id());
+  // Java VideoTrack holds one reference. Corresponding Release() is in
+  // MediaStreamTrack_free, triggered by VideoTrack.dispose().
+  track->AddRef();
+  jobject j_track = jni()->NewObject(*j_video_track_class_, j_video_track_ctor_,
+                                     reinterpret_cast<jlong>(track), id);
+  CHECK_EXCEPTION(jni()) << "error during NewObject";
+  remote_tracks_[track] = NewGlobalRef(jni(), j_track);
+  jfieldID video_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "videoTracks", "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);
+
+  std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>> streams;
+  streams.push_back(rtc::scoped_refptr<webrtc::MediaStreamInterface>(stream));
+  jobjectArray j_stream_array = NativeToJavaMediaStreamArray(jni(), streams);
+  jmethodID m =
+      GetMethodID(jni(), *j_observer_class_, "onAddTrack",
+                  "(Lorg/webrtc/MediaStreamTrack;[Lorg/webrtc/MediaStream;)V");
+  jni()->CallVoidMethod(*j_observer_global_, m, j_track, j_stream_array);
+  CHECK_EXCEPTION(jni()) << "Error during CallVoidMethod";
+}
+
+void PeerConnectionObserverJni::OnAudioTrackRemoved(
+    webrtc::AudioTrackInterface* track,
+    webrtc::MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+  NativeToJavaMediaTrackMap::iterator track_it = remote_tracks_.find(track);
+  RTC_CHECK(track_it != remote_tracks_.end());
+  jobject j_track = track_it->second;
+
+  jfieldID audio_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "audioTracks", "Ljava/util/LinkedList;");
+  jobject video_tracks = GetObjectField(jni(), j_stream, audio_tracks_id);
+  jmethodID remove = GetMethodID(jni(), GetObjectClass(jni(), video_tracks),
+                                 "remove", "(Ljava/lang/Object;)Z");
+  jboolean removed = jni()->CallBooleanMethod(video_tracks, remove, j_track);
+  CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
+  RTC_CHECK(removed);
+
+  std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>> streams;
+  streams.push_back(rtc::scoped_refptr<webrtc::MediaStreamInterface>(stream));
+  jobjectArray j_stream_array = NativeToJavaMediaStreamArray(jni(), streams);
+  jmethodID m =
+      GetMethodID(jni(), *j_observer_class_, "onRemoveTrack",
+                  "(Lorg/webrtc/MediaStreamTrack;[Lorg/webrtc/MediaStream;)V");
+  jni()->CallVoidMethod(*j_observer_global_, m, j_track, j_stream_array);
+  CHECK_EXCEPTION(jni()) << "Error during CallVoidMethod";
+
+  DisposeRemoteTrack(track_it);
+}
+
+void PeerConnectionObserverJni::OnVideoTrackRemoved(
+    webrtc::VideoTrackInterface* track,
+    webrtc::MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+  NativeToJavaMediaTrackMap::iterator track_it = remote_tracks_.find(track);
+  RTC_CHECK(track_it != remote_tracks_.end());
+  jobject j_track = track_it->second;
+
+  jfieldID video_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "videoTracks", "Ljava/util/LinkedList;");
+  jobject video_tracks = GetObjectField(jni(), j_stream, video_tracks_id);
+  jmethodID remove = GetMethodID(jni(), GetObjectClass(jni(), video_tracks),
+                                 "remove", "(Ljava/lang/Object;)Z");
+  jboolean removed = jni()->CallBooleanMethod(video_tracks, remove, j_track);
+  CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
+  RTC_CHECK(removed);
+
+  std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>> streams;
+  streams.push_back(rtc::scoped_refptr<webrtc::MediaStreamInterface>(stream));
+  jobjectArray j_stream_array = NativeToJavaMediaStreamArray(jni(), streams);
+  jmethodID m =
+      GetMethodID(jni(), *j_observer_class_, "onRemoveTrack",
+                  "(Lorg/webrtc/MediaStreamTrack;[Lorg/webrtc/MediaStream;)V");
+  jni()->CallVoidMethod(*j_observer_global_, m, j_track, j_stream_array);
+  CHECK_EXCEPTION(jni()) << "Error during CallVoidMethod";
+
+  DisposeRemoteTrack(track_it);
 }
 
 void PeerConnectionObserverJni::OnRemoveStream(
@@ -186,6 +296,16 @@ void PeerConnectionObserverJni::OnRemoveStream(
                             "(Lorg/webrtc/MediaStream;)V");
   jni()->CallVoidMethod(*j_observer_global_, m, j_stream);
   CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
+
+  stream_observers_.erase(
+      std::remove_if(
+          stream_observers_.begin(), stream_observers_.end(),
+          [stream](
+              const std::unique_ptr<webrtc::MediaStreamObserver>& observer) {
+            return observer->stream() == stream;
+          }),
+      stream_observers_.end());
+
   // Release the refptr reference so that DisposeRemoteStream can assert
   // it removes the final reference.
   stream = nullptr;
@@ -226,20 +346,9 @@ void PeerConnectionObserverJni::OnAddTrack(
     rtc::scoped_refptr<webrtc::RtpReceiverInterface> receiver,
     const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&
         streams) {
-  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 = NativeToJavaMediaStreamArray(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";
+  // PCO::onAddTrack is broken and called only one time when the MediaStream
+  // is added to PC. Android sdk observes MediaStream's tracks modification
+  // with MediaStreamObserver - see PeerConnectionObserverJni::OnAddStream

[Taylor Brandstetter, 2017/08/29 19:48:09]

Can you explain the issue more? Can this be fixed, rather than piggybacking off
of OnAddStream? Relying on OnAddStream isn't a future-proof solution, since it
will be possible later to have a track without a stream.

It will also be possible in the future to have a track that's a part of two
streams, which I don't think the code above would support. So that's another
reason to use OnAddTrack and keep the JNI code as minimal as possible.

[korniltsev, 2017/08/30 21:47:47]

It was not actually broken, I was wrong when I wrote it. The problem with this
method is - it did not modify org.webrtc.MediaStream#audioTracks/videoTracks
fields and onAddStream did create org.webrtc.VideoTrack/AudioTrack only
once(when the stream is added).

On 2017/08/29 19:48:09, Taylor Brandstetter wrote:
Yes. Rewritten.
onAddTrack is implemented with PeerConnectionObserver
onRemoveTrack is implemented with MediaStreamObserver

 }
 
 void PeerConnectionObserverJni::SetConstraints(
@@ -258,15 +367,21 @@ void PeerConnectionObserverJni::DisposeRemoteStream(
   DeleteGlobalRef(jni(), j_stream);
 }
 
-void PeerConnectionObserverJni::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);
+void PeerConnectionObserverJni::DisposeRemoteTrack(
+    const NativeToJavaMediaTrackMap::iterator& it) {
+  RTC_CHECK(it != remote_tracks_.end());
+  webrtc::MediaStreamTrackInterface* track = it->first;
+  jobject j_track = it->second;
+  remote_tracks_.erase(it);
+  jmethodID dispose;
+  if (track->kind() == webrtc::MediaStreamTrackInterface::kVideoKind) {
+    dispose = GetMethodID(jni(), *j_video_track_class_, "dispose", "()V");
+  } else {
+    dispose = GetMethodID(jni(), *j_audio_track_class_, "dispose", "()V");
+  }
+  jni()->CallVoidMethod(j_track, dispose);
+  CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
+  DeleteGlobalRef(jni(), j_track);
 }
 
 // If the NativeToJavaStreamsMap contains the stream, return it.
@@ -277,7 +392,6 @@ jobject PeerConnectionObserverJni::GetOrCreateJavaStream(
   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();