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

webrtc/api/java/jni/androidvideocapturer_jni.cc

 /*
  *  Copyright 2015 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 163 matching lines @@
 
   rtc::scoped_refptr<webrtc::VideoFrameBuffer> buffer =
       buffer_pool_.CreateBuffer(width, height);
   libyuv::NV21ToI420(
       y_plane, width,
       vu_plane, width,
       buffer->MutableData(webrtc::kYPlane), buffer->stride(webrtc::kYPlane),
       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,
                                              int height,
                                              int rotation,
                                              int64_t timestamp_ns,
                                              const NativeHandleImpl& handle) {
   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,
                                                     int height,
                                                     int fps) {
   AsyncCapturerInvoke("OnOutputFormatRequest",
                       &webrtc::AndroidVideoCapturer::OnOutputFormatRequest,
                       width, height, fps);
 }
 
@@ skipping 29 matching lines @@
 
 JOW(void, VideoCapturer_00024NativeObserver_nativeOnOutputFormatRequest)
     (JNIEnv* jni, jclass, jlong j_capturer, jint j_width, jint j_height,
         jint j_fps) {
   LOG(LS_INFO) << "NativeObserver_nativeOnOutputFormatRequest";
   reinterpret_cast<AndroidVideoCapturerJni*>(j_capturer)->OnOutputFormatRequest(
       j_width, j_height, j_fps);
 }
 
 }  // namespace webrtc_jni