VideoCapturerAndroid: Fix threading issues

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

 /*
  * libjingle
  * Copyright 2015 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 16 matching lines @@
 
 package org.webrtc;
 
 import android.content.Context;
 import android.graphics.SurfaceTexture;
 import android.hardware.Camera;
 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;
 
 import org.json.JSONException;
 import org.webrtc.CameraEnumerationAndroid.CaptureFormat;
 import org.webrtc.Logging;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 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.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.FutureTask;
 import java.util.concurrent.TimeUnit;
 
 // Android specific implementation of VideoCapturer.
 // An instance of this class can be created by an application using
 // VideoCapturerAndroid.create();
 // This class extends VideoCapturer with a method to easily switch between the
 // 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
-// Java callbacks.  Since these calls happen on different threads,
-// the entry points to this class are all synchronized.  This shouldn't present
-// a performance bottleneck because only onPreviewFrame() is called more than
-// 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.
+// Threading notes: this class is called from C++ code, Android Camera callbacks, and possibly
+// arbitrary Java threads. All public entry points are thread safe, and delegate the work to the
+// camera thread. The internal *OnCameraThread() methods must check |camera| for null to check if
+// the camera has been stopped.
 @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 Handler cameraThreadHandler;
+  private final HandlerThread cameraThread;
+  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| in getCurrentCameraId() and switchCameraOnCameraThread().
+  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;
   private int requestedFramerate;
   // The capture format will be the closest supported format to the requested format.
   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;
 
   // Camera error callback.
   private final Camera.ErrorCallback cameraErrorCallback =
       new Camera.ErrorCallback() {
     @Override
     public void onError(int error, Camera camera) {
       String errorMessage;
@@ skipping 25 matching lines @@
           String.format("%.1f", averageCaptureBuffersCount) +
           ". Pending buffers: " + videoBuffers.pendingFramesTimeStamps());
       if (cameraFramesCount == 0) {
         Logging.e(TAG, "Camera freezed.");
         if (errorHandler != null) {
           errorHandler.onCameraError("Camera failure.");
         }
       } else {
         cameraFramesCount = 0;
         captureBuffersCount = 0;
-        if (cameraThreadHandler != null) {
-          cameraThreadHandler.postDelayed(this, CAMERA_OBSERVER_PERIOD_MS);
-        }
+        cameraThreadHandler.postDelayed(this, CAMERA_OBSERVER_PERIOD_MS);
       }
     }
   };
 
   // Camera error handler - invoked when camera stops receiving frames
   // or any camera exception happens on camera thread.
   public static interface CameraErrorHandler {
     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);
     if (capturer != null) {
       VideoCapturerAndroid capturerAndroid = (VideoCapturerAndroid) capturer;
       capturerAndroid.errorHandler = errorHandler;
       return capturerAndroid;
     }
     return null;
   }
 
   // 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
-  // current resolution.
-  public synchronized boolean switchCamera(final Runnable switchDoneEvent) {
-    if (Camera.getNumberOfCameras() < 2 )
-      return false;
-
-    if (cameraThreadHandler == null) {
-        Logging.e(TAG, "Calling switchCamera() for stopped camera.");
-        return false;
+  public void switchCamera(final CameraSwitchHandler handler) {
+    if (Camera.getNumberOfCameras() < 2) {
+      if (handler != null) {
+        handler.onCameraSwitchError("No camera to switch to.");
+      }
+      return;
     }
-    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;
+    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.");
+        if (handler != null) {
+          handler.onCameraSwitchError("Pending camera switch already in progress.");
+        }
+        return;
+      }
+      pendingCameraSwitch = true;
     }
-
-    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(
-      final int width, final int height, final int fps) {
-    if (cameraThreadHandler == null) {
-      Logging.e(TAG, "Calling onOutputFormatRequest() for already stopped camera.");
-      return;
-    }
+  // TODO(magjed/perkj): Document what this function does. Change name?
+  public void onOutputFormatRequest(final int width, final int height, final int fps) {
     cameraThreadHandler.post(new Runnable() {
       @Override public void run() {
         onOutputFormatRequestOnCameraThread(width, height, fps);
       }
     });
   }
 
   // Reconfigure the camera to capture in a new format. This should only be called while the camera
   // is running.
-  public synchronized void changeCaptureFormat(
-      final int width, final int height, final int framerate) {
-    if (cameraThreadHandler == null) {
-      Logging.e(TAG, "Calling changeCaptureFormat() for already stopped camera.");
-      return;
-    }
+  public void changeCaptureFormat(final int width, final int height, final int framerate) {
     cameraThreadHandler.post(new Runnable() {
       @Override public void run() {
         startPreviewOnCameraThread(width, height, framerate);
       }
     });
   }
 
-  public synchronized List<CaptureFormat> getSupportedFormats() {
-    return CameraEnumerationAndroid.getSupportedFormats(id);
+  // Helper function to retrieve the current camera id synchronously. Note that the camera id might
+  // change at any point by switchCamera() calls.
+  private int getCurrentCameraId() {
+    synchronized (cameraIdLock) {
+      return id;
+    }

[hbos, 2015/09/22 09:12:46]

If |id| is volatile you would not have to lock here.
When incrementing (read-and-write as a single op) you would still need a lock if
set by multiple threads, but if it's only ever the camera thread that sets |id|
then you could get rid of |cameraIdLock| altogether by using volatile.

If we can't get rid of |cameraIdLock| then this comment is optional.

[magjed_webrtc, 2015/09/22 15:04:21]

Acknowledged.

   }
 
-  // Return a list of timestamps for the frames that have been sent out, but not returned yet.
-  // Useful for logging and testing.
-  public String pendingFramesTimeStamps() {
-    return videoBuffers.pendingFramesTimeStamps();
+  public List<CaptureFormat> getSupportedFormats() {
+    return CameraEnumerationAndroid.getSupportedFormats(getCurrentCameraId());
+  }
+
+  // Called from native code.
+  private String getSupportedFormatsAsJson() throws JSONException {
+    return CameraEnumerationAndroid.getSupportedFormatsAsJson(getCurrentCameraId());
+  }
+
+  // Return number of frames that have been sent out, but not returned yet. Useful for logging and
+  // testing. This function blocks until the camera thread has time to process the request.
+  public int pendingFramesCount() throws InterruptedException {
+    final FutureTask<Integer> futureTask = new FutureTask<Integer>(new Callable<Integer>() {
+      @Override
+      public Integer call() {
+        return videoBuffers.pendingFramesCount();
+      }
+    });
+    cameraThreadHandler.post(futureTask);
+    try {
+      return futureTask.get();
+    } catch (ExecutionException e) {
+      throw new RuntimeException("FramePool.pendingFramesCount() threw exception: " + e);
+    }
   }
 
   private VideoCapturerAndroid() {
     Logging.d(TAG, "VideoCapturerAndroid");
+    cameraThread = new HandlerThread(TAG);
+    cameraThread.start();
+    cameraThreadHandler = new Handler(cameraThread.getLooper());
+    videoBuffers = new FramePool(cameraThread);
   }
 
   // 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()) {
       this.id = 0;

[hbos, 2015/09/22 09:12:46]

If on camera thread, rename to initOnCameraThread and |id| can be set without
lock if volatile. But this could be some other C++ thread?
Use |cameraIdLock|?

[magjed_webrtc, 2015/09/22 15:04:21]

It is not on camera thread, but it will be called from JNI code in connection
with a create() call. Synchronization is not strictly necessary, because
create() will block until this is done. I have added synchronization anyway to
be robust.

-      foundDevice = true;
+      return true;
     } else {
       for (int i = 0; i < Camera.getNumberOfCameras(); ++i) {
-        String existing_device = CameraEnumerationAndroid.getDeviceName(i);
-        if (existing_device != null && deviceName.equals(existing_device)) {
+        if (deviceName.equals(CameraEnumerationAndroid.getDeviceName(i))) {
           this.id = i;
-          foundDevice = true;
+          return true;
         }
       }
     }
-    return foundDevice;
+    return false;
   }
 
-  String getSupportedFormatsAsJson() throws JSONException {
-    return CameraEnumerationAndroid.getSupportedFormatsAsJson(id);
+  // Called by native code to quit the camera thread. This needs to be done manually, otherwise the
+  // thread and handler will not be garbage collected.
+  private void release() {
+    cameraThread.quit();
   }
 
-  private class CameraThread extends Thread {
-    private Exchanger<Handler> handlerExchanger;
-    public CameraThread(Exchanger<Handler> handlerExchanger) {
-      this.handlerExchanger = handlerExchanger;
-    }
-
-    @Override public void run() {
-      Looper.prepare();
-      exchange(handlerExchanger, new Handler());
-      Looper.loop();
-    }
-  }
-
-  // Called by native code.  Returns true if capturer is started.
+  // Called by native code.
   //
   // 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
         + "@" + framerate);
     if (applicationContext == null) {
       throw new RuntimeException("applicationContext not set.");
     }
     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,
             applicationContext);
       }
     });
   }
 
   private void startCaptureOnCameraThread(
       int width, int height, int framerate, CapturerObserver frameObserver,
       Context applicationContext) {
     Throwable error = null;
+    if (camera != null) {
+      throw new RuntimeException("Camera has already been started.");
+    }
     this.applicationContext = applicationContext;
     this.frameObserver = frameObserver;
     try {
       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
@@ skipping 18 matching lines @@
       // Start camera observer.
       cameraFramesCount = 0;
       captureBuffersCount = 0;
       cameraThreadHandler.postDelayed(cameraObserver, CAMERA_OBSERVER_PERIOD_MS);
       return;
     } catch (RuntimeException e) {
       error = e;
     }
     Logging.e(TAG, "startCapture failed", error);
     stopCaptureOnCameraThread();
-    cameraThreadHandler = null;
     frameObserver.OnCapturerStarted(false);
     if (errorHandler != null) {
       errorHandler.onCameraError("Camera can not be started.");
     }
     return;
   }
 
   // (Re)start preview with the closest supported format to |width| x |height| @ |framerate|.
   private void startPreviewOnCameraThread(int width, int height, int framerate) {
     Logging.d(
@@ skipping 52 matching lines @@
     // (Re)start preview.
     Logging.d(TAG, "Start capturing: " + captureFormat);
     this.captureFormat = captureFormat;
     camera.setParameters(parameters);
     videoBuffers.queueCameraBuffers(captureFormat.frameSize(), camera);
     camera.setPreviewCallbackWithBuffer(this);
     camera.startPreview();
   }
 
   // Called by native code.  Returns true when camera is known to be stopped.
-  synchronized void stopCapture() throws InterruptedException {
-    if (cameraThreadHandler == null) {
-      Logging.e(TAG, "Calling stopCapture() for already stopped camera.");
-      return;
-    }
+  void stopCapture() throws InterruptedException {
     Logging.d(TAG, "stopCapture");
+    final CountDownLatch barrier = new CountDownLatch(1);
     cameraThreadHandler.post(new Runnable() {
         @Override public void run() {
           stopCaptureOnCameraThread();
+          barrier.countDown();
         }
     });
-    cameraThread.join();
-    cameraThreadHandler = null;
+    barrier.await();
     Logging.d(TAG, "stopCapture done");
   }
 
   private void stopCaptureOnCameraThread() {
-    doStopCaptureOnCameraThread();
-    Looper.myLooper().quit();
-    return;
-  }
-
-  private void doStopCaptureOnCameraThread() {
     Logging.d(TAG, "stopCaptureOnCameraThread");
     if (camera == null) {
+      Logging.e(TAG, "Calling stopCapture() for already stopped camera.");
       return;
     }
 
     cameraThreadHandler.removeCallbacks(cameraObserver);
     Logging.d(TAG, "Stop preview.");
     camera.stopPreview();
     camera.setPreviewCallbackWithBuffer(null);
     videoBuffers.stopReturnBuffersToCamera();
     captureFormat = null;
 
     if (cameraGlTexture != 0) {
       GLES20.glDeleteTextures(1, new int[] {cameraGlTexture}, 0);
       cameraGlTexture = 0;
     }
     Logging.d(TAG, "Release camera.");
     camera.release();
     camera = null;
     cameraSurfaceTexture = null;

[hbos, 2015/09/22 09:12:46]

Does cameraSurfaceTexture not have to be released as well or is this done
implicitly by camera.release()?

[magjed_webrtc, 2015/09/22 15:04:21]

Let's try. What can possibly go wrong?

[hbos, 2015/09/23 07:37:36]

YOLO. Random opengl thread crashes incoming.

Probably a good idea: "Always call this method when you are done with
SurfaceTexture. Failing to do so may delay resource deallocation for a
significant amount of time."

   }
 
-  private void switchCameraOnCameraThread(Runnable switchDoneEvent) {
+  private void switchCameraOnCameraThread() {
     Logging.d(TAG, "switchCameraOnCameraThread");
-
-    doStopCaptureOnCameraThread();
+    stopCaptureOnCameraThread();
+    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(
-      int width, int height, int fps) {
+  private void onOutputFormatRequestOnCameraThread(int width, int height, int fps) {
     if (camera == null) {
+      Logging.e(TAG, "Calling onOutputFormatRequest() on stopped camera.");
       return;
     }
     Logging.d(TAG, "onOutputFormatRequestOnCameraThread: " + width + "x" + height +
         "@" + fps);
     frameObserver.OnOutputFormatRequest(width, height, fps);
   }
 
-  void returnBuffer(long timeStamp) {
-    videoBuffers.returnBuffer(timeStamp);
+  public void returnBuffer(final long timeStamp) {
+    cameraThreadHandler.post(new Runnable() {
+      @Override public void run() {
+        videoBuffers.returnBuffer(timeStamp);
+      }
+    });
   }
 
   private int getDeviceOrientation() {
     int orientation = 0;
 
     WindowManager wm = (WindowManager) applicationContext.getSystemService(
         Context.WINDOW_SERVICE);
     switch(wm.getDefaultDisplay().getRotation()) {
       case Surface.ROTATION_90:
         orientation = 90;
@@ skipping 39 matching lines @@
     // data.length >= videoBuffers.frameSize.
     if (videoBuffers.reserveByteBuffer(data, captureTimeNs)) {
       cameraFramesCount++;
       frameObserver.OnFrameCaptured(data, videoBuffers.frameSize, captureFormat.width,
           captureFormat.height, rotation, captureTimeNs);
     } else {
       Logging.w(TAG, "reserveByteBuffer failed - dropping frame.");
     }
   }
 
-  // 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).
     private static final int numCaptureBuffers = 3;
     // This container tracks the buffers added as camera callback buffers. It is needed for finding
     // the corresponding ByteBuffer given a byte[].
     private final Map<byte[], ByteBuffer> queuedBuffers = new IdentityHashMap<byte[], ByteBuffer>();
     // This container tracks the frames that have been sent but not returned. It is needed for
     // keeping the buffers alive and for finding the corresponding ByteBuffer given a timestamp.
     private final Map<Long, ByteBuffer> pendingBuffers = new HashMap<Long, ByteBuffer>();
     private int frameSize = 0;
     private Camera camera;
 
-    synchronized int numCaptureBuffersAvailable() {
+    public FramePool(Thread thread) {
+      this.thread = thread;
+    }
+
+    private void checkIsOnValidThread() {

[hbos, 2015/09/22 09:12:46]

Optional: Could we have a checkIsOnCameraThread like this for the
*OnCameraThread methods?

[magjed_webrtc, 2015/09/22 15:04:21]

Done.

+      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;
 
       queuedBuffers.clear();
       for (int i = 0; i < numCaptureBuffers; ++i) {
         final ByteBuffer buffer = ByteBuffer.allocateDirect(frameSize);
         camera.addCallbackBuffer(buffer.array());
         queuedBuffers.put(buffer.array(), buffer);
       }
       Logging.d(TAG, "queueCameraBuffers enqueued " + numCaptureBuffers
           + " 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));
       }
       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.
       Logging.d(TAG, "stopReturnBuffersToCamera called."
             + (pendingBuffers.isEmpty() ?
                    " All buffers have been returned."
                    : " 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
         // capture format in |startPreviewOnCameraThread|. Drop these old frames.
         Logging.w(TAG, "Received callback buffer from previous configuration with length: "
             + (data == null ? "null" : data.length));
         return false;
       }
       if (buffer.capacity() != frameSize) {
         throw new IllegalStateException("Callback buffer has unexpected frame size");
       }
       if (pendingBuffers.containsKey(timeStamp)) {
         Logging.e(TAG, "Timestamp already present in pending buffers - they need to be unique");
         return false;
       }
       pendingBuffers.put(timeStamp, buffer);
       if (queuedBuffers.isEmpty()) {
         Logging.v(TAG, "Camera is running out of capture buffers."
             + " Pending buffers: " + pendingFramesTimeStamps());
       }
       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 "
             + timeStamp + "returned?!?");
       }
 
       if (camera != null && returnedFrame.capacity() == frameSize) {
         camera.addCallbackBuffer(returnedFrame.array());
         if (queuedBuffers.isEmpty()) {
           Logging.v(TAG, "Frame returned when camera is running out of capture"
@@ skipping 61 matching lines @@
     }
 
     private native void nativeCapturerStarted(long nativeCapturer,
         boolean success);
     private native void nativeOnFrameCaptured(long nativeCapturer,
         byte[] data, int length, int width, int height, int rotation, long timeStamp);
     private native void nativeOnOutputFormatRequest(long nativeCapturer,
         int width, int height, int fps);
   }
 }