VideoCapturerAndroid: Fix threading issues

This CL makes the following changes:
 * Instead of creating a new thread per startCapture()/stopCapture() cycle, VideoCapturerAndroid has a single thread that is initialized in the constructor and kept during the lifetime of the instance. This is more convenient because then it is always possible to post runnables without if-checks. This way, a lot of synchronize statements can be removed. Also, when the camera thread is preserved after stopCapture() it is possible to post late returnBuffer() calls to the correct thread.
 * FramePool now enforces single thread use and returnBuffer() calls are posted to the camera thread. This is important because the camera should only be used from one thread, and we call camera.addCallbackBuffer() in returnBuffer().
 * switchCamera() no longer returns false on failure, but instead signals the result via the callback.
 * Update the test testCaptureAndAsyncRender() - it's not a valid use case to have outstanding frames when calling PeerConnectionFactory.dispose(). Instead, the renderer implementations should have release() functions that block until all frames are returned. The release() functions need to be called in the correct order with PeerConnectionFactory.dispose() last.

BUG=webrtc:4909
R=hbos@webrtc.org, perkj@webrtc.org

Review URL: https://codereview.webrtc.org/1350863002 .

Cr-Commit-Position: refs/heads/master@{#10025}

talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java

@@ -29,6 +29,7 @@ package org.webrtc;
 import android.hardware.Camera;
 import android.test.ActivityTestCase;
 import android.test.suitebuilder.annotation.SmallTest;
+import android.test.suitebuilder.annotation.MediumTest;
 import android.util.Size;
 
 import org.webrtc.CameraEnumerationAndroid.CaptureFormat;
@@ -38,6 +39,7 @@ import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.CountDownLatch;
 
 @SuppressWarnings("deprecation")
 public class VideoCapturerAndroidTest extends ActivityTestCase {
@@ -62,7 +64,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     }
   }
 
-  static class AsyncRenderer implements VideoRenderer.Callbacks {
+  static class FakeAsyncRenderer implements VideoRenderer.Callbacks {
     private final List<I420Frame> pendingFrames = new ArrayList<I420Frame>();
 
     @Override
@@ -74,18 +76,12 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     }
 
     // Wait until at least one frame have been received, before returning them.
-    public List<I420Frame> WaitForFrames() {
+    public List<I420Frame> waitForPendingFrames() throws InterruptedException {
       synchronized (pendingFrames) {
         while (pendingFrames.isEmpty()) {
-          try {
           pendingFrames.wait();
-          } catch (InterruptedException e) {
-            // Ignore.
         }
-        }
+        return new ArrayList<I420Frame>(pendingFrames);
-        final List<I420Frame> frames = new ArrayList<I420Frame>(pendingFrames);
-        pendingFrames.clear();
-        return frames;
       }
     }
   }
@@ -169,6 +165,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     track.dispose();
     source.dispose();
     factory.dispose();
+    assertTrue(capturer.isReleased());
   }
 
   @Override
@@ -213,6 +210,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     VideoCapturerAndroid capturer = VideoCapturerAndroid.create("", null);
     assertNotNull(capturer);
     capturer.dispose();
+    assertTrue(capturer.isReleased());
   }
 
   @SmallTest
@@ -250,7 +248,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
   }
 
   @SmallTest
-  // This test that the default camera can be started and but the camera can
+  // This test that the default camera can be started and that the camera can
   // later be switched to another camera.
   // It tests both the Java and the C++ layer.
   public void testSwitchVideoCapturer() throws Exception {
@@ -260,14 +258,30 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
         factory.createVideoSource(capturer, new MediaConstraints());
     VideoTrack track = factory.createVideoTrack("dummy", source);
 
-    if (HaveTwoCameras())
+    // Array with one element to avoid final problem in nested classes.
-      assertTrue(capturer.switchCamera(null));
+    final boolean[] cameraSwitchSuccessful = new boolean[1];
-    else
+    final CountDownLatch barrier = new CountDownLatch(1);
-      assertFalse(capturer.switchCamera(null));
+    capturer.switchCamera(new VideoCapturerAndroid.CameraSwitchHandler() {
-
+      @Override
-    // Wait until the camera have been switched.
+      public void onCameraSwitchDone(boolean isFrontCamera) {
-    capturer.runCameraThreadUntilIdle();
+        cameraSwitchSuccessful[0] = true;
+        barrier.countDown();
+      }
+      @Override
+      public void onCameraSwitchError(String errorDescription) {
+        cameraSwitchSuccessful[0] = false;
+        barrier.countDown();
+      }
+    });
+    // Wait until the camera has been switched.
+    barrier.await();
 
+    // Check result.
+    if (HaveTwoCameras()) {
+      assertTrue(cameraSwitchSuccessful[0]);
+    } else {
+      assertFalse(cameraSwitchSuccessful[0]);
+    }
     // Ensure that frames are received.
     RendererCallbacks callbacks = new RendererCallbacks();
     track.addRenderer(new VideoRenderer(callbacks));
@@ -275,6 +289,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     track.dispose();
     source.dispose();
     factory.dispose();
+    assertTrue(capturer.isReleased());
   }
 
   @SmallTest
@@ -300,6 +315,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     track.dispose();
     source.dispose();
     factory.dispose();
+    assertTrue(capturer.isReleased());
   }
 
   @SmallTest
@@ -322,8 +338,12 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
       // Check the frame size.
       assertEquals(format.frameSize(), observer.frameSize());
       capturer.stopCapture();
+      for (long timestamp : observer.getCopyAndResetListOftimeStamps()) {
+        capturer.returnBuffer(timestamp);
+      }
     }
     capturer.dispose();
+    assertTrue(capturer.isReleased());
   }
 
   @SmallTest
@@ -365,55 +385,48 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     for (Long timeStamp : listOftimestamps) {
       capturer.returnBuffer(timeStamp);
     }
+    capturer.dispose();
+    assertTrue(capturer.isReleased());
   }
 
-  @SmallTest
+  @MediumTest
-  // This test that we can capture frames, stop capturing, keep the frames for rendering, and then
+  // This test that we can capture frames, keep the frames in a local renderer, stop capturing,
-  // return the frames. It tests both the Java and the C++ layer.
+  // and then return the frames. The difference between the test testReturnBufferLate() is that we
-  public void testCaptureAndAsyncRender() {
+  // also test the JNI and C++ AndroidVideoCapturer parts.
+  public void testReturnBufferLateEndToEnd() throws InterruptedException {
     final VideoCapturerAndroid capturer = VideoCapturerAndroid.create("", null);
-    // Helper class that sets everything up, captures at least one frame, and then shuts
+    final PeerConnectionFactory factory = new PeerConnectionFactory();
-    // everything down.
+    final VideoSource source = factory.createVideoSource(capturer, new MediaConstraints());
-    class CaptureFramesRunnable implements Runnable {
+    final VideoTrack track = factory.createVideoTrack("dummy", source);
-      public List<I420Frame> frames;
+    final FakeAsyncRenderer renderer = new FakeAsyncRenderer();
-
-      @Override
-      public void run() {
-        PeerConnectionFactory factory = new PeerConnectionFactory();
-        VideoSource source = factory.createVideoSource(capturer, new MediaConstraints());
-        VideoTrack track = factory.createVideoTrack("dummy", source);
-        AsyncRenderer renderer = new AsyncRenderer();
     track.addRenderer(new VideoRenderer(renderer));
+    // Wait for at least one frame that has not been returned.
+    assertFalse(renderer.waitForPendingFrames().isEmpty());
 
-        // Wait until we get at least one frame.
+    capturer.stopCapture();
-        frames = renderer.WaitForFrames();
 
-        // Stop everything.
+    // Dispose source and |capturer|.
     track.dispose();
     source.dispose();
+    // The pending frames should keep the JNI parts and |capturer| alive.
+    assertFalse(capturer.isReleased());
+
+    // Return the frame(s), on a different thread out of spite.
+    final List<I420Frame> pendingFrames = renderer.waitForPendingFrames();
+    final Thread returnThread = new Thread(new Runnable() {
+      @Override
+      public void run() {
+        for (I420Frame frame : pendingFrames) {
+          VideoRenderer.renderFrameDone(frame);
+        }
+      }
+    });
+    returnThread.start();
+    returnThread.join();
+
+    // Check that frames have successfully returned. This will cause |capturer| to be released.
+    assertTrue(capturer.isReleased());
+
     factory.dispose();
   }
 }
-
-    // Capture frames on a separate thread.
-    CaptureFramesRunnable captureFramesRunnable = new CaptureFramesRunnable();
-    Thread captureThread = new Thread(captureFramesRunnable);
-    captureThread.start();
-
-    // Wait until frames are captured, and then kill the thread.
-    try {
-      captureThread.join();
-    } catch (InterruptedException e) {
-      fail("Capture thread was interrupted");
-    }
-    captureThread = null;
-
-    // Assert that we have frames that have not been returned.
-    assertTrue(!captureFramesRunnable.frames.isEmpty());
-    // Return the frame(s).
-    for (I420Frame frame : captureFramesRunnable.frames) {
-      VideoRenderer.renderFrameDone(frame);
-    }
-    assertEquals(capturer.pendingFramesTimeStamps(), "[]");
-  }
-}

talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java

@@ -34,7 +34,7 @@ import android.hardware.Camera.PreviewCallback;
 import android.opengl.GLES11Ext;
 import android.opengl.GLES20;
 import android.os.Handler;
-import android.os.Looper;
+import android.os.HandlerThread;
 import android.os.SystemClock;
 import android.view.Surface;
 import android.view.WindowManager;
@@ -50,7 +50,7 @@ import java.util.HashMap;
 import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.Exchanger;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 // Android specific implementation of VideoCapturer.
@@ -60,29 +60,27 @@ import java.util.concurrent.TimeUnit;
 // front and back camera. It also provides methods for enumerating valid device
 // names.
 //
-// Threading notes: this class is called from C++ code, and from Camera
+// Threading notes: this class is called from C++ code, Android Camera callbacks, and possibly
-// Java callbacks.  Since these calls happen on different threads,
+// arbitrary Java threads. All public entry points are thread safe, and delegate the work to the
-// the entry points to this class are all synchronized.  This shouldn't present
+// camera thread. The internal *OnCameraThread() methods must check |camera| for null to check if
-// a performance bottleneck because only onPreviewFrame() is called more than
+// the camera has been stopped.
-// once (and is called serially on a single thread), so the lock should be
-// uncontended.  Note that each of these synchronized methods must check
-// |camera| for null to account for having possibly waited for stopCapture() to
-// complete.
 @SuppressWarnings("deprecation")
 public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallback {
   private final static String TAG = "VideoCapturerAndroid";
   private final static int CAMERA_OBSERVER_PERIOD_MS = 5000;
 
   private Camera camera;  // Only non-null while capturing.
-  private CameraThread cameraThread;
+  private HandlerThread cameraThread;
-  private Handler cameraThreadHandler;
+  private final Handler cameraThreadHandler;
   // |cameraSurfaceTexture| is used with setPreviewTexture. Must be a member, see issue webrtc:5021.
   private SurfaceTexture cameraSurfaceTexture;
   private Context applicationContext;
+  // Synchronization lock for |id|.
+  private final Object cameraIdLock = new Object();
   private int id;
   private Camera.CameraInfo info;
   private int cameraGlTexture = 0;
-  private final FramePool videoBuffers = new FramePool();
+  private final FramePool videoBuffers;
   // Remember the requested format in case we want to switch cameras.
   private int requestedWidth;
   private int requestedHeight;
@@ -91,6 +89,7 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
   private CaptureFormat captureFormat;
   private int cameraFramesCount;
   private int captureBuffersCount;
+  private final Object pendingCameraSwitchLock = new Object();
   private volatile boolean pendingCameraSwitch;
   private CapturerObserver frameObserver = null;
   private CameraErrorHandler errorHandler = null;
@@ -136,11 +135,9 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
       } else {
         cameraFramesCount = 0;
         captureBuffersCount = 0;
-        if (cameraThreadHandler != null) {
         cameraThreadHandler.postDelayed(this, CAMERA_OBSERVER_PERIOD_MS);
       }
     }
-    }
   };
 
   // Camera error handler - invoked when camera stops receiving frames
@@ -149,6 +146,15 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     public void onCameraError(String errorDescription);
   }
 
+  // Camera switch handler - one of these functions are invoked with the result of switchCamera().
+  // The callback may be called on an arbitrary thread.
+  public interface CameraSwitchHandler {
+    // Invoked on success. |isFrontCamera| is true if the new camera is front facing.
+    void onCameraSwitchDone(boolean isFrontCamera);
+    // Invoked on failure, e.g. camera is stopped or only one camera available.
+    void onCameraSwitchError(String errorDescription);
+  }
+
   public static VideoCapturerAndroid create(String name,
       CameraErrorHandler errorHandler) {
     VideoCapturer capturer = VideoCapturer.create(name);
@@ -162,41 +168,48 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
 
   // Switch camera to the next valid camera id. This can only be called while
   // the camera is running.
-  // Returns true on success. False if the next camera does not support the
+  public void switchCamera(final CameraSwitchHandler handler) {
-  // current resolution.
+    if (Camera.getNumberOfCameras() < 2) {
-  public synchronized boolean switchCamera(final Runnable switchDoneEvent) {
+      if (handler != null) {
-    if (Camera.getNumberOfCameras() < 2 )
+        handler.onCameraSwitchError("No camera to switch to.");
-      return false;
-
-    if (cameraThreadHandler == null) {
-        Logging.e(TAG, "Calling switchCamera() for stopped camera.");
-        return false;
       }
+      return;
+    }
+    synchronized (pendingCameraSwitchLock) {
       if (pendingCameraSwitch) {
         // Do not handle multiple camera switch request to avoid blocking
         // camera thread by handling too many switch request from a queue.
         Logging.w(TAG, "Ignoring camera switch request.");
-      return false;
+        if (handler != null) {
+          handler.onCameraSwitchError("Pending camera switch already in progress.");
+        }
+        return;
       }
-
       pendingCameraSwitch = true;
-    id = (id + 1) % Camera.getNumberOfCameras();
+    }
     cameraThreadHandler.post(new Runnable() {
       @Override public void run() {
-        switchCameraOnCameraThread(switchDoneEvent);
+        if (camera == null) {
+          if (handler != null) {
+            handler.onCameraSwitchError("Camera is stopped.");
+          }
+          return;
+        }
+        switchCameraOnCameraThread();
+        synchronized (pendingCameraSwitchLock) {
+          pendingCameraSwitch = false;
+        }
+        if (handler != null) {
+          handler.onCameraSwitchDone(info.facing == Camera.CameraInfo.CAMERA_FACING_FRONT);
+        }
       }
     });
-    return true;
   }
 
   // Requests a new output format from the video capturer. Captured frames
   // by the camera will be scaled/or dropped by the video capturer.
-  public synchronized void onOutputFormatRequest(
+  // TODO(magjed/perkj): Document what this function does. Change name?
-      final int width, final int height, final int fps) {
+  public void onOutputFormatRequest(final int width, final int height, final int fps) {
-    if (cameraThreadHandler == null) {
-      Logging.e(TAG, "Calling onOutputFormatRequest() for already stopped camera.");
-      return;
-    }
     cameraThreadHandler.post(new Runnable() {
       @Override public void run() {
         onOutputFormatRequestOnCameraThread(width, height, fps);
@@ -206,12 +219,7 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
 
   // Reconfigure the camera to capture in a new format. This should only be called while the camera
   // is running.
-  public synchronized void changeCaptureFormat(
+  public void changeCaptureFormat(final int width, final int height, final int framerate) {
-      final int width, final int height, final int framerate) {
-    if (cameraThreadHandler == null) {
-      Logging.e(TAG, "Calling changeCaptureFormat() for already stopped camera.");
-      return;
-    }
     cameraThreadHandler.post(new Runnable() {
       @Override public void run() {
         startPreviewOnCameraThread(width, height, framerate);
@@ -219,66 +227,95 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     });
   }
 
-  public synchronized List<CaptureFormat> getSupportedFormats() {
+  // Helper function to retrieve the current camera id synchronously. Note that the camera id might
-    return CameraEnumerationAndroid.getSupportedFormats(id);
+  // change at any point by switchCamera() calls.
+  private int getCurrentCameraId() {
+    synchronized (cameraIdLock) {
+      return id;
+    }
   }
 
-  // Return a list of timestamps for the frames that have been sent out, but not returned yet.
+  public List<CaptureFormat> getSupportedFormats() {
-  // Useful for logging and testing.
+    return CameraEnumerationAndroid.getSupportedFormats(getCurrentCameraId());
-  public String pendingFramesTimeStamps() {
+  }
-    return videoBuffers.pendingFramesTimeStamps();
+
+  // Called from native code.
+  private String getSupportedFormatsAsJson() throws JSONException {
+    return CameraEnumerationAndroid.getSupportedFormatsAsJson(getCurrentCameraId());
   }
 
   private VideoCapturerAndroid() {
     Logging.d(TAG, "VideoCapturerAndroid");
+    cameraThread = new HandlerThread(TAG);
+    cameraThread.start();
+    cameraThreadHandler = new Handler(cameraThread.getLooper());
+    videoBuffers = new FramePool(cameraThread);
+  }
+
+  private void checkIsOnCameraThread() {
+    if (Thread.currentThread() != cameraThread) {
+      throw new IllegalStateException("Wrong thread");
+    }
   }
 
   // Called by native code.
   // Initializes local variables for the camera named |deviceName|. If |deviceName| is empty, the
   // first available device is used in order to be compatible with the generic VideoCapturer class.
-  synchronized boolean init(String deviceName) {
+  boolean init(String deviceName) {
     Logging.d(TAG, "init: " + deviceName);
     if (deviceName == null)
       return false;
 
-    boolean foundDevice = false;
     if (deviceName.isEmpty()) {
+      synchronized (cameraIdLock) {
         this.id = 0;
-      foundDevice = true;
+      }
+      return true;
     } else {
       for (int i = 0; i < Camera.getNumberOfCameras(); ++i) {
-        String existing_device = CameraEnumerationAndroid.getDeviceName(i);
+        if (deviceName.equals(CameraEnumerationAndroid.getDeviceName(i))) {
-        if (existing_device != null && deviceName.equals(existing_device)) {
+          synchronized (cameraIdLock) {
             this.id = i;
-          foundDevice = true;
+          }
+          return true;
         }
       }
     }
-    return foundDevice;
+    return false;
   }
 
-  String getSupportedFormatsAsJson() throws JSONException {
+  // Called by native code to quit the camera thread. This needs to be done manually, otherwise the
-    return CameraEnumerationAndroid.getSupportedFormatsAsJson(id);
+  // thread and handler will not be garbage collected.
+  private void release() {
+    if (isReleased()) {
+      throw new IllegalStateException("Already released");
+    }
+    cameraThreadHandler.post(new Runnable() {
+      @Override
+      public void run() {
+        if (camera != null) {
+          throw new IllegalStateException("Release called while camera is running");
+        }
+        if (videoBuffers.pendingFramesCount() != 0) {
+          throw new IllegalStateException("Release called with pending frames left");
+        }
+      }
+    });
+    cameraThread.quitSafely();
+    ThreadUtils.joinUninterruptibly(cameraThread);
+    cameraThread = null;
   }
 
-  private class CameraThread extends Thread {
+  // Used for testing purposes to check if release() has been called.
-    private Exchanger<Handler> handlerExchanger;
+  public boolean isReleased() {
-    public CameraThread(Exchanger<Handler> handlerExchanger) {
+    return (cameraThread == null);
-      this.handlerExchanger = handlerExchanger;
   }
 
-    @Override public void run() {
+  // Called by native code.
-      Looper.prepare();
-      exchange(handlerExchanger, new Handler());
-      Looper.loop();
-    }
-  }
-
-  // Called by native code.  Returns true if capturer is started.
   //
   // Note that this actually opens the camera, and Camera callbacks run on the
   // thread that calls open(), so this is done on the CameraThread.
-  synchronized void startCapture(
+  void startCapture(
       final int width, final int height, final int framerate,
       final Context applicationContext, final CapturerObserver frameObserver) {
     Logging.d(TAG, "startCapture requested: " + width + "x" + height
@@ -289,14 +326,6 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     if (frameObserver == null) {
       throw new RuntimeException("frameObserver not set.");
     }
-    if (cameraThreadHandler != null) {
-      throw new RuntimeException("Camera has already been started.");
-    }
-
-    Exchanger<Handler> handlerExchanger = new Exchanger<Handler>();
-    cameraThread = new CameraThread(handlerExchanger);
-    cameraThread.start();
-    cameraThreadHandler = exchange(handlerExchanger, null);
     cameraThreadHandler.post(new Runnable() {
       @Override public void run() {
         startCaptureOnCameraThread(width, height, framerate, frameObserver,
@@ -309,13 +338,19 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
       int width, int height, int framerate, CapturerObserver frameObserver,
       Context applicationContext) {
     Throwable error = null;
+    checkIsOnCameraThread();
+    if (camera != null) {
+      throw new RuntimeException("Camera has already been started.");
+    }
     this.applicationContext = applicationContext;
     this.frameObserver = frameObserver;
     try {
+      synchronized (cameraIdLock) {
         Logging.d(TAG, "Opening camera " + id);
         camera = Camera.open(id);
         info = new Camera.CameraInfo();
         Camera.getCameraInfo(id, info);
+      }
       // No local renderer (we only care about onPreviewFrame() buffers, not a
       // directly-displayed UI element).  Camera won't capture without
       // setPreview{Texture,Display}, so we create a SurfaceTexture and hand
@@ -347,7 +382,6 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     }
     Logging.e(TAG, "startCapture failed", error);
     stopCaptureOnCameraThread();
-    cameraThreadHandler = null;
     frameObserver.OnCapturerStarted(false);
     if (errorHandler != null) {
       errorHandler.onCameraError("Camera can not be started.");
@@ -357,6 +391,7 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
 
   // (Re)start preview with the closest supported format to |width| x |height| @ |framerate|.
   private void startPreviewOnCameraThread(int width, int height, int framerate) {
+    checkIsOnCameraThread();
     Logging.d(
         TAG, "startPreviewOnCameraThread requested: " + width + "x" + height + "@" + framerate);
     if (camera == null) {
@@ -420,31 +455,24 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
   }
 
   // Called by native code.  Returns true when camera is known to be stopped.
-  synchronized void stopCapture() throws InterruptedException {
+  void stopCapture() throws InterruptedException {
-    if (cameraThreadHandler == null) {
-      Logging.e(TAG, "Calling stopCapture() for already stopped camera.");
-      return;
-    }
     Logging.d(TAG, "stopCapture");
+    final CountDownLatch barrier = new CountDownLatch(1);
     cameraThreadHandler.post(new Runnable() {
         @Override public void run() {
           stopCaptureOnCameraThread();
+          barrier.countDown();
         }
     });
-    cameraThread.join();
+    barrier.await();
-    cameraThreadHandler = null;
     Logging.d(TAG, "stopCapture done");
   }
 
   private void stopCaptureOnCameraThread() {
-    doStopCaptureOnCameraThread();
+    checkIsOnCameraThread();
-    Looper.myLooper().quit();
-    return;
-  }
-
-  private void doStopCaptureOnCameraThread() {
     Logging.d(TAG, "stopCaptureOnCameraThread");
     if (camera == null) {
+      Logging.e(TAG, "Calling stopCapture() for already stopped camera.");
       return;
     }
 
@@ -462,25 +490,26 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     Logging.d(TAG, "Release camera.");
     camera.release();
     camera = null;
+    cameraSurfaceTexture.release();
     cameraSurfaceTexture = null;
   }
 
-  private void switchCameraOnCameraThread(Runnable switchDoneEvent) {
+  private void switchCameraOnCameraThread() {
+    checkIsOnCameraThread();
     Logging.d(TAG, "switchCameraOnCameraThread");
-
+    stopCaptureOnCameraThread();
-    doStopCaptureOnCameraThread();
+    synchronized (cameraIdLock) {
+      id = (id + 1) % Camera.getNumberOfCameras();
+    }
     startCaptureOnCameraThread(requestedWidth, requestedHeight, requestedFramerate, frameObserver,
         applicationContext);
-    pendingCameraSwitch = false;
     Logging.d(TAG, "switchCameraOnCameraThread done");
-    if (switchDoneEvent != null) {
-      switchDoneEvent.run();
-    }
   }
 
-  private void onOutputFormatRequestOnCameraThread(
+  private void onOutputFormatRequestOnCameraThread(int width, int height, int fps) {
-      int width, int height, int fps) {
+    checkIsOnCameraThread();
     if (camera == null) {
+      Logging.e(TAG, "Calling onOutputFormatRequest() on stopped camera.");
       return;
     }
     Logging.d(TAG, "onOutputFormatRequestOnCameraThread: " + width + "x" + height +
@@ -488,9 +517,13 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     frameObserver.OnOutputFormatRequest(width, height, fps);
   }
 
-  void returnBuffer(long timeStamp) {
+  public void returnBuffer(final long timeStamp) {
+    cameraThreadHandler.post(new Runnable() {
+      @Override public void run() {
         videoBuffers.returnBuffer(timeStamp);
       }
+    });
+  }
 
   private int getDeviceOrientation() {
     int orientation = 0;
@@ -518,9 +551,7 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
   // Called on cameraThread so must not "synchronized".
   @Override
   public void onPreviewFrame(byte[] data, Camera callbackCamera) {
-    if (Thread.currentThread() != cameraThread) {
+    checkIsOnCameraThread();
-      throw new RuntimeException("Camera callback not on camera thread?!?");
-    }
     if (camera == null) {
       return;
     }
@@ -549,37 +580,12 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     }
   }
 
-  // runCameraThreadUntilIdle make sure all posted messages to the cameraThread
-  // is processed before returning. It does that by itself posting a message to
-  // to the message queue and waits until is has been processed.
-  // It is used in tests.
-  void runCameraThreadUntilIdle() {
-    if (cameraThreadHandler == null)
-      return;
-    final Exchanger<Boolean> result = new Exchanger<Boolean>();
-    cameraThreadHandler.post(new Runnable() {
-      @Override public void run() {
-        exchange(result, true); // |true| is a dummy here.
-      }
-    });
-    exchange(result, false);  // |false| is a dummy value here.
-    return;
-  }
-
-  // Exchanges |value| with |exchanger|, converting InterruptedExceptions to
-  // RuntimeExceptions (since we expect never to see these).
-  private static <T> T exchange(Exchanger<T> exchanger, T value) {
-    try {
-      return exchanger.exchange(value);
-    } catch (InterruptedException e) {
-      throw new RuntimeException(e);
-    }
-  }
-
   // Class used for allocating and bookkeeping video frames. All buffers are
   // direct allocated so that they can be directly used from native code. This class is
-  // synchronized and can be called from multiple threads.
+  // not thread-safe, and enforces single thread use.
   private static class FramePool {
+    // Thread that all calls should be made on.
+    private final Thread thread;
     // Arbitrary queue depth.  Higher number means more memory allocated & held,
     // lower number means more sensitivity to processing time in the client (and
     // potentially stalling the capturer if it runs out of buffers to write to).
@@ -593,12 +599,24 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
     private int frameSize = 0;
     private Camera camera;
 
-    synchronized int numCaptureBuffersAvailable() {
+    public FramePool(Thread thread) {
+      this.thread = thread;
+    }
+
+    private void checkIsOnValidThread() {
+      if (Thread.currentThread() != thread) {
+        throw new IllegalStateException("Wrong thread");
+      }
+    }
+
+    public int numCaptureBuffersAvailable() {
+      checkIsOnValidThread();
       return queuedBuffers.size();
     }
 
     // Discards previous queued buffers and adds new callback buffers to camera.
-    synchronized void queueCameraBuffers(int frameSize, Camera camera) {
+    public void queueCameraBuffers(int frameSize, Camera camera) {
+      checkIsOnValidThread();
       this.camera = camera;
       this.frameSize = frameSize;
 
@@ -612,7 +630,14 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
           + " buffers of size " + frameSize + ".");
     }
 
-    synchronized String pendingFramesTimeStamps() {
+    // Return number of pending frames that have not been returned.
+    public int pendingFramesCount() {
+      checkIsOnValidThread();
+      return pendingBuffers.size();
+    }
+
+    public String pendingFramesTimeStamps() {
+      checkIsOnValidThread();
       List<Long> timeStampsMs = new ArrayList<Long>();
       for (Long timeStampNs : pendingBuffers.keySet()) {
         timeStampsMs.add(TimeUnit.NANOSECONDS.toMillis(timeStampNs));
@@ -620,7 +645,8 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
       return timeStampsMs.toString();
     }
 
-    synchronized void stopReturnBuffersToCamera() {
+    public void stopReturnBuffersToCamera() {
+      checkIsOnValidThread();
       this.camera = null;
       queuedBuffers.clear();
       // Frames in |pendingBuffers| need to be kept alive until they are returned.
@@ -630,7 +656,8 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
                    : " Pending buffers: " + pendingFramesTimeStamps() + "."));
     }
 
-    synchronized boolean reserveByteBuffer(byte[] data, long timeStamp) {
+    public boolean reserveByteBuffer(byte[] data, long timeStamp) {
+      checkIsOnValidThread();
       final ByteBuffer buffer = queuedBuffers.remove(data);
       if (buffer == null) {
         // Frames might be posted to |onPreviewFrame| with the previous format while changing
@@ -654,7 +681,8 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba
       return true;
     }
 
-    synchronized void returnBuffer(long timeStamp) {
+    public void returnBuffer(long timeStamp) {
+      checkIsOnValidThread();
       final ByteBuffer returnedFrame = pendingBuffers.remove(timeStamp);
       if (returnedFrame == null) {
         throw new RuntimeException("unknown data buffer with time stamp "

talk/app/webrtc/java/jni/androidvideocapturer_jni.cc

@@ -88,6 +88,10 @@ bool AndroidVideoCapturerJni::Init(jstring device_name) {
 
 AndroidVideoCapturerJni::~AndroidVideoCapturerJni() {
   LOG(LS_INFO) << "AndroidVideoCapturerJni dtor";
+  jni()->CallVoidMethod(
+      *j_capturer_global_,
+      GetMethodID(jni(), *j_video_capturer_class_, "release", "()V"));
+  CHECK_EXCEPTION(jni()) << "error during VideoCapturerAndroid.release()";
 }
 
 void AndroidVideoCapturerJni::Start(int width, int height, int framerate,