android: Don't do a thread jump for incoming frames.

webrtc/api/java/jni/androidvideocapturer_jni.cc

Index: webrtc/api/java/jni/androidvideocapturer_jni.cc
diff --git a/webrtc/api/java/jni/androidvideocapturer_jni.cc b/webrtc/api/java/jni/androidvideocapturer_jni.cc
index 615fcb32d80c30895e4958215e96be5ef5ed9dda..5acd3dc087d8a550527df514624216366c0da125 100644
--- a/webrtc/api/java/jni/androidvideocapturer_jni.cc
+++ b/webrtc/api/java/jni/androidvideocapturer_jni.cc
@@ -181,9 +181,9 @@ void AndroidVideoCapturerJni::OnMemoryBufferFrame(void* video_frame,
       buffer->MutableData(webrtc::kUPlane), buffer->stride(webrtc::kUPlane),
       buffer->MutableData(webrtc::kVPlane), buffer->stride(webrtc::kVPlane),
       width, height);
-  AsyncCapturerInvoke("OnIncomingFrame",
-                      &webrtc::AndroidVideoCapturer::OnIncomingFrame,
-                      buffer, rotation, timestamp_ns);
+
+  rtc::CritScope cs(&capturer_lock_);
+  capturer_->OnIncomingFrame(buffer, rotation, timestamp_ns);
 }
 
 void AndroidVideoCapturerJni::OnTextureFrame(int width,
@@ -194,9 +194,8 @@ void AndroidVideoCapturerJni::OnTextureFrame(int width,
   rtc::scoped_refptr<webrtc::VideoFrameBuffer> buffer(
       surface_texture_helper_->CreateTextureFrame(width, height, handle));
 
-  AsyncCapturerInvoke("OnIncomingFrame",
-                      &webrtc::AndroidVideoCapturer::OnIncomingFrame,
-                      buffer, rotation, timestamp_ns);
+  rtc::CritScope cs(&capturer_lock_);

[pbos-webrtc, 2016/05/17 14:26:06]

Any risk of this deadlocking?

[nisse-webrtc, 2016/05/18 07:44:01]

No idea... The capturer_ pointer is declared with a GUARDED_BY annotation, but
I'm not sure what this lock is intended to protect.
Maybe a race with the Stop method (but in that case, there ought to be a check
that capturer_ != nullptr, after taking the lock.

[nisse-webrtc, 2016/05/18 12:28:03]

It seems capturer_lock_ was introduced in cl
https://codereview.webrtc.org/1307973002 

Magnus, can you explain why the lock was needed?

Anyway, I'm a bit weary of removing it in this cl, since the videoframefactory
used is not thread safe (in particular, UpdateCapturedFrame). Or if all frames
arrive on the same thread, that wouldn't be a problem?

[magjed_webrtc, 2016/05/18 15:12:50]

It was/is used to protect against callbacks from the Java capturer after
AndroidVideoCapturerJni::Stop is called. I don't think you can remove it - how
will you handle e.g. pending nativeOnTextureFrameCaptured() when Stop is called?

[nisse-webrtc, 2016/05/18 15:31:31]

Don't we need something like

  rtc::CritScope cs(&capturer_lock_);
  if (!capturer_)
    { ... frame arrive when stopped... ; return; }

I don't seen any such checks. Or is the opposite race, Stop shouldn't return
until after On..Frame is completed? I'm still a bit confused.

[magjed_webrtc, 2016/05/18 15:51:28]

We have exactly such a check in AsyncCapturerInvoke:
AsyncCapturerInvoke(...) {
  rtc::CritScope cs(&capturer_lock_);
  if (!invoker_) {
    LOG(LS_WARNING) << method_name << "() called for closed capturer.";
    return;
  }
  invoker_->AsyncInvoke<void>(rtc::Bind(method, capturer_, args...));
}
You have removed the thread jump by removing the use of AsyncCapturerInvoke in
these functions, but you still need the check.

[nisse-webrtc, 2016/05/19 07:01:04]

Done.

+  capturer_->OnIncomingFrame(buffer, rotation, timestamp_ns);
 }
 
 void AndroidVideoCapturerJni::OnOutputFormatRequest(int width,