Refactor stopCapture to be asynchronous in VideoCapturerAndroid.

webrtc/api/android/java/src/org/webrtc/VideoCapturerAndroid.java

 /*
  *  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 28 matching lines @@
 // camera thread. The internal *OnCameraThread() methods must check |camera| for null to check if
 // the camera has been stopped.
 // TODO(magjed): This class name is now confusing - rename to Camera1VideoCapturer.
 @SuppressWarnings("deprecation")
 public class VideoCapturerAndroid implements
     CameraVideoCapturer,
     android.hardware.Camera.PreviewCallback,
     SurfaceTextureHelper.OnTextureFrameAvailableListener {
   private static final String TAG = "VideoCapturerAndroid";
   private static final int CAMERA_STOP_TIMEOUT_MS = 7000;
+  // 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 NUMBER_OF_CAPTURE_BUFFERS = 3;
+  private final static int MAX_OPEN_CAMERA_ATTEMPTS = 3;
+  private final static int OPEN_CAMERA_DELAY_MS = 500;
+  private static enum CameraState { UNINITIALIAZED, IDLE, STARTING, RUNNING }
 
+  private final Set<byte[]> queuedBuffers = new HashSet<byte[]>();
+  private final AtomicBoolean generateCapturerEvents = new AtomicBoolean();
+  private final boolean isCapturingToTexture;
+  private final CameraEventsHandler eventsHandler;
+
+  // Initialized on initialize
+  // -------------------------
+  // Use postOnCameraThread() instead of posting directly to the handler - this way all
+  // callbacks with a specifed token can be removed at once.
+  private Handler cameraThreadHandler;
+  private Context applicationContext;
+  private CapturerObserver capturerObserver = null;
+  private SurfaceTextureHelper surfaceHelper;
+
+  // Internal state - will only be touch from the camera thread
+  // ----------------------------------------------------------
   private android.hardware.Camera camera;  // Only non-null while capturing.
-  private final AtomicBoolean isCameraRunning = new AtomicBoolean();
-  // Use maybePostOnCameraThread() instead of posting directly to the handler - this way all
-  // callbacks with a specifed token can be removed at once.
-  private volatile Handler cameraThreadHandler;
-  private Context applicationContext;
-  // Synchronization lock for |id|.
-  private final Object cameraIdLock = new Object();
-  private int id;
-  private android.hardware.Camera.CameraInfo info;
-  private CameraStatistics cameraStatistics;
+  private CameraState cameraState;
   // 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.
+  // Populated with the information of the active camera.
+  private android.hardware.Camera.CameraInfo info;
+  // Active capture format.
   private CaptureFormat captureFormat;
-  private final Object pendingCameraSwitchLock = new Object();
-  private volatile boolean pendingCameraSwitch;
-  private CapturerObserver frameObserver = null;
-  private final CameraEventsHandler eventsHandler;
+  private CameraStatistics cameraStatistics;
   private boolean firstFrameReported;
-  // 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 NUMBER_OF_CAPTURE_BUFFERS = 3;
-  private final Set<byte[]> queuedBuffers = new HashSet<byte[]>();
-  private final boolean isCapturingToTexture;
-  private SurfaceTextureHelper surfaceHelper;
-  private final static int MAX_OPEN_CAMERA_ATTEMPTS = 3;
-  private final static int OPEN_CAMERA_DELAY_MS = 500;
   private int openCameraAttempts;
 
+  // Locked objected

[magjed_webrtc, 2016/07/20 14:56:08]

objects

[sakal, 2016/07/21 11:17:24]

Done.

+  // ---------------
+  private final Object cameraIdLock = new Object();
+  private int id; // Only edited from camera thread while holding the lock.
+
+  // Only allow one camera switch a time.
+  private final Object cameraSwitchLock = new Object();
+  private boolean pendingCameraSwitch;
+  private CameraSwitchHandler switchEventsHandler;
+
   // Camera error callback.
   private final android.hardware.Camera.ErrorCallback cameraErrorCallback =
       new android.hardware.Camera.ErrorCallback() {
     @Override
     public void onError(int error, android.hardware.Camera camera) {
       String errorMessage;
       if (error == android.hardware.Camera.CAMERA_ERROR_SERVER_DIED) {
         errorMessage = "Camera server died!";
       } else {
         errorMessage = "Camera error: " + error;
       }
       Logging.e(TAG, errorMessage);
       if (eventsHandler != null) {
         eventsHandler.onCameraError(errorMessage);
       }
     }
   };
 
+  private void setCameraState(CameraState newState) {
+    cameraState = newState;

[magjed_webrtc, 2016/07/20 14:56:08]

Add a debug log with the current state and the new state.

[sakal, 2016/07/21 11:17:23]

Done.

+  }
+
   public static VideoCapturerAndroid create(String name,
       CameraEventsHandler eventsHandler) {
     return VideoCapturerAndroid.create(name, eventsHandler, false /* captureToTexture */);
   }
 
   // Use ctor directly instead.
   @Deprecated
   public static VideoCapturerAndroid create(String name,
       CameraEventsHandler eventsHandler, boolean captureToTexture) {
     try {
@@ skipping 23 matching lines @@
   // Switch camera to the next valid camera id. This can only be called while
   // the camera is running.
   @Override
   public void switchCamera(final CameraSwitchHandler switchEventsHandler) {
     if (android.hardware.Camera.getNumberOfCameras() < 2) {
       if (switchEventsHandler != null) {
         switchEventsHandler.onCameraSwitchError("No camera to switch to.");
       }
       return;
     }
-    synchronized (pendingCameraSwitchLock) {
+
+    synchronized (cameraSwitchLock) {
       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 (switchEventsHandler != null) {
           switchEventsHandler.onCameraSwitchError("Pending camera switch already in progress.");
         }
         return;
       }
+
       pendingCameraSwitch = true;
     }
-    final boolean didPost = maybePostOnCameraThread(new Runnable() {
+
+    postOnCameraThread(new Runnable() {
       @Override
       public void run() {
+        // Set switchEventsHandler only here because now openCameraOnCameraThread cannot be called
+        // in-between these calls.
+        synchronized (cameraSwitchLock) {
+          VideoCapturerAndroid.this.switchEventsHandler = switchEventsHandler;
+        }
         switchCameraOnCameraThread();
-        synchronized (pendingCameraSwitchLock) {
+        synchronized (cameraSwitchLock) {
           pendingCameraSwitch = false;
         }
-        if (switchEventsHandler != null) {
-          switchEventsHandler.onCameraSwitchDone(
-              info.facing == android.hardware.Camera.CameraInfo.CAMERA_FACING_FRONT);
-        }
       }
     });
-    if (!didPost && switchEventsHandler != null) {
-      switchEventsHandler.onCameraSwitchError("Camera is stopped.");
-    }
   }
 
   // Requests a new output format from the video capturer. Captured frames
   // by the camera will be scaled/or dropped by the video capturer.
   // It does not matter if width and height are flipped. I.E, |width| = 640, |height| = 480 produce
   // the same result as |width| = 480, |height| = 640.
   // TODO(magjed/perkj): Document what this function does. Change name?
   @Override
   public void onOutputFormatRequest(final int width, final int height, final int framerate) {
-    maybePostOnCameraThread(new Runnable() {
+    postOnCameraThread(new Runnable() {
       @Override public void run() {
         onOutputFormatRequestOnCameraThread(width, height, framerate);
       }
     });
   }
 
   // Reconfigure the camera to capture in a new format. This should only be called while the camera
   // is running.
   @Override
   public void changeCaptureFormat(final int width, final int height, final int framerate) {
-    maybePostOnCameraThread(new Runnable() {
+    postOnCameraThread(new Runnable() {
       @Override public void run() {
-        startPreviewOnCameraThread(width, height, framerate);
+        requestedWidth = width;
+        requestedHeight = height;
+        requestedFramerate = framerate;
+
+        startPreviewOnCameraThread();
       }
     });
   }
 
-  // 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() {
+  @Override
+  public List<CaptureFormat> getSupportedFormats() {
     synchronized (cameraIdLock) {
-      return id;
+      return Camera1Enumerator.getSupportedFormats(id);
     }
   }
 
-  @Override
-  public List<CaptureFormat> getSupportedFormats() {
-    return Camera1Enumerator.getSupportedFormats(getCurrentCameraId());
-  }
-
   // Returns true if this VideoCapturer is setup to capture video frames to a SurfaceTexture.
   public boolean isCapturingToTexture() {
     return isCapturingToTexture;
   }
 
   public VideoCapturerAndroid(String cameraName, CameraEventsHandler eventsHandler,
       boolean captureToTexture) {
+    cameraState = CameraState.UNINITIALIAZED;

[magjed_webrtc, 2016/07/20 14:56:08]

I prefer if you move this to the declaration.

[sakal, 2016/07/21 11:17:24]

Done.

+
     if (android.hardware.Camera.getNumberOfCameras() == 0) {
       throw new RuntimeException("No cameras available");
     }
     if (cameraName == null || cameraName.equals("")) {
       this.id = 0;
     } else {
       this.id = Camera1Enumerator.getCameraIndex(cameraName);
     }
     this.eventsHandler = eventsHandler;
     isCapturingToTexture = captureToTexture;
     Logging.d(TAG, "VideoCapturerAndroid isCapturingToTexture : " + isCapturingToTexture);
   }
 
   private void checkIsOnCameraThread() {
     if (cameraThreadHandler == null) {
-      Logging.e(TAG, "Camera is not initialized - can't check thread.");
+      throw new IllegalStateException("Camera is not initialized - can't check thread.");
     } else if (Thread.currentThread() != cameraThreadHandler.getLooper().getThread()) {
       throw new IllegalStateException("Wrong thread");
     }
   }
 
-  private boolean maybePostOnCameraThread(Runnable runnable) {
-    return maybePostDelayedOnCameraThread(0 /* delayMs */, runnable);
+  private void postOnCameraThread(Runnable runnable) {
+    postDelayedOnCameraThread(0 /* delayMs */, runnable);
   }
 
-  private boolean maybePostDelayedOnCameraThread(int delayMs, Runnable runnable) {
-    return cameraThreadHandler != null && isCameraRunning.get()
-        && cameraThreadHandler.postAtTime(
-            runnable, this /* token */, SystemClock.uptimeMillis() + delayMs);
+  private void postDelayedOnCameraThread(int delayMs, Runnable runnable) {
+    cameraThreadHandler.postAtTime(

[magjed_webrtc, 2016/07/20 14:56:07]

You have removed the logic for handling failure to post at some places. Can you
throw a RuntimeException here if postAtTime returns false? I think it will only
happen if someone quits the looper/thread, which should not happen before
dispose is called.

[sakal, 2016/07/21 11:17:24]

Done.

+        runnable, this /* token */, SystemClock.uptimeMillis() + delayMs);
   }
 
   @Override
   public void dispose() {
     Logging.d(TAG, "dispose");
+    stopCapture();
+    // Nothing should be running on the camera thread so should be safe

[magjed_webrtc, 2016/07/20 14:56:08]

nit: dot at end of sentence.

[sakal, 2016/07/21 11:17:23]

Done.

+    cameraState = CameraState.UNINITIALIAZED;
   }
 
   private boolean isInitialized() {
-    return applicationContext != null && frameObserver != null;
+    return applicationContext != null && capturerObserver != null;

[magjed_webrtc, 2016/07/20 14:56:08]

Maybe this is cleaner:
return cameraState != UNINITIALIAZED;
?

[sakal, 2016/07/21 11:17:24]

Done.

   }
 
   @Override
   public void initialize(SurfaceTextureHelper surfaceTextureHelper, Context applicationContext,
-      CapturerObserver frameObserver) {
+      CapturerObserver capturerObserver) {
     Logging.d(TAG, "initialize");
     if (applicationContext == null) {
       throw new IllegalArgumentException("applicationContext not set.");
     }
-    if (frameObserver == null) {
-      throw new IllegalArgumentException("frameObserver not set.");
+    if (capturerObserver == null) {
+      throw new IllegalArgumentException("capturerObserver not set.");
     }
     if (isInitialized()) {
       throw new IllegalStateException("Already initialized");
     }
     this.applicationContext = applicationContext;
-    this.frameObserver = frameObserver;
+    this.capturerObserver = capturerObserver;
     this.surfaceHelper = surfaceTextureHelper;
     this.cameraThreadHandler =
         surfaceTextureHelper == null ? null : surfaceTextureHelper.getHandler();
+
+    // Nothing should be running on the camera thread so should be safe
+    cameraState = CameraState.IDLE;

[magjed_webrtc, 2016/07/20 14:56:07]

nit: dot at end of sentence.

[sakal, 2016/07/21 11:17:23]

Done.

   }
 
   // Note that this actually opens the camera, and Camera callbacks run on the
   // thread that calls open(), so this is done on the CameraThread.
   @Override
   public void startCapture(final int width, final int height, final int framerate) {
     Logging.d(TAG, "startCapture requested: " + width + "x" + height + "@" + framerate);
-    if (!isInitialized()) {
-      throw new IllegalStateException("startCapture called in uninitialized state");
-    }
+
+    generateCapturerEvents.set(true);
     if (surfaceHelper == null) {
-      frameObserver.onCapturerStarted(false /* success */);
+      capturerObserver.onCapturerStarted(false /* success */);
       if (eventsHandler != null) {
         eventsHandler.onCameraError("No SurfaceTexture created.");
       }
       return;
     }
-    if (isCameraRunning.getAndSet(true)) {
-      Logging.e(TAG, "Camera has already been started.");
-      return;
-    }
-    final boolean didPost = maybePostOnCameraThread(new Runnable() {
+
+    postOnCameraThread(new Runnable() {
       @Override
       public void run() {
-        openCameraAttempts = 0;
         startCaptureOnCameraThread(width, height, framerate);
       }
     });
-    if (!didPost) {
-      frameObserver.onCapturerStarted(false);
-      if (eventsHandler != null) {
-        eventsHandler.onCameraError("Could not post task to camera thread.");
-      }
-      isCameraRunning.set(false);
-    }
   }
 
   private void startCaptureOnCameraThread(final int width, final int height, final int framerate) {
     checkIsOnCameraThread();
-    if (!isCameraRunning.get()) {
-      Logging.e(TAG, "startCaptureOnCameraThread: Camera is stopped");
+
+    openCameraAttempts = 0;
+    firstFrameReported = false;

[magjed_webrtc, 2016/07/20 14:56:08]

Move this line under the state check.

[sakal, 2016/07/21 11:17:24]

Done.

+    if (cameraState != CameraState.IDLE) {
+      Logging.e(TAG, "Camera has already been started.");
       return;
     }
-    if (camera != null) {
-      Logging.e(TAG, "startCaptureOnCameraThread: Camera has already been started.");
-      return;
-    }
-    this.firstFrameReported = false;
 
+    setCameraState(CameraState.STARTING);
+
+    requestedWidth = width;
+    requestedHeight = height;
+    requestedFramerate = framerate;
+
+    openCameraOnCameraThread();
+  }
+
+  private void openCameraOnCameraThread() {
     try {
+      Logging.d(TAG, "Opening camera " + id);
+      if (eventsHandler != null) {
+        eventsHandler.onCameraOpening(id);
+      }
+
       try {
-        synchronized (cameraIdLock) {
-          Logging.d(TAG, "Opening camera " + id);
-          if (eventsHandler != null) {
-            eventsHandler.onCameraOpening(id);
-          }
-          camera = android.hardware.Camera.open(id);
-          info = new android.hardware.Camera.CameraInfo();
-          android.hardware.Camera.getCameraInfo(id, info);
-        }
+        camera = android.hardware.Camera.open(id);
       } catch (RuntimeException e) {
         openCameraAttempts++;
         if (openCameraAttempts < MAX_OPEN_CAMERA_ATTEMPTS) {
           Logging.e(TAG, "Camera.open failed, retrying", e);
-          maybePostDelayedOnCameraThread(OPEN_CAMERA_DELAY_MS, new Runnable() {
+          postDelayedOnCameraThread(OPEN_CAMERA_DELAY_MS, new Runnable() {
             @Override
             public void run() {
-              startCaptureOnCameraThread(width, height, framerate);
+              openCameraOnCameraThread();
             }
           });
           return;
         }
         throw e;
       }
 
+      setCameraState(CameraState.RUNNING);
+
+      info = new android.hardware.Camera.CameraInfo();
+      android.hardware.Camera.getCameraInfo(id, info);
+
       camera.setPreviewTexture(surfaceHelper.getSurfaceTexture());
 
       Logging.d(TAG, "Camera orientation: " + info.orientation +
           " .Device orientation: " + getDeviceOrientation());
       camera.setErrorCallback(cameraErrorCallback);
-      startPreviewOnCameraThread(width, height, framerate);
-      frameObserver.onCapturerStarted(true);
+      startPreviewOnCameraThread();
+      capturerObserver.onCapturerStarted(true);
       if (isCapturingToTexture) {
         surfaceHelper.startListening(this);
       }
 
-      // Start camera observer.
+      // Start camera statistics collector.
       cameraStatistics = new CameraStatistics(surfaceHelper, eventsHandler);
     } catch (IOException|RuntimeException e) {
       Logging.e(TAG, "startCapture failed", e);
       // Make sure the camera is released.
-      stopCaptureOnCameraThread(true /* stopHandler */);
-      frameObserver.onCapturerStarted(false);
+      releaseCameraOnCameraThread();
+      capturerObserver.onCapturerStarted(false);
       if (eventsHandler != null) {
         eventsHandler.onCameraError("Camera can not be started.");
       }
-     }
+    }
+
+    synchronized (cameraSwitchLock) {

[magjed_webrtc, 2016/07/20 14:56:08]

This block should be moved inside the success path of the try-catch statement,
i.e. close to 'capturerObserver.onCapturerStarted(true);' Also, you should call
onCameraSwitchError if we fail to start the camera.

[sakal, 2016/07/21 11:17:24]

Done.

+      if (switchEventsHandler != null) {
+        switchEventsHandler.onCameraSwitchDone(
+            info.facing == android.hardware.Camera.CameraInfo.CAMERA_FACING_FRONT);
+      }
+      switchEventsHandler = null;
+    }
   }
 
   // (Re)start preview with the closest supported format to |width| x |height| @ |framerate|.
-  private void startPreviewOnCameraThread(int width, int height, int framerate) {
+  private void startPreviewOnCameraThread() {
     checkIsOnCameraThread();
-    if (!isCameraRunning.get() || camera == null) {
-      Logging.e(TAG, "startPreviewOnCameraThread: Camera is stopped");
+    if (cameraState != CameraState.RUNNING) {
+      Logging.e(TAG, "startPreviewOnCameraThread: Camera is not running");
       return;
     }
-    Logging.d(
-        TAG, "startPreviewOnCameraThread requested: " + width + "x" + height + "@" + framerate);
-
-    requestedWidth = width;
-    requestedHeight = height;
-    requestedFramerate = framerate;
+    Logging.d(TAG, "startPreviewOnCameraThread requested: "
+        + requestedWidth + "x" + requestedHeight + "@" + requestedFramerate);
 
     // Find closest supported format for |width| x |height| @ |framerate|.
     final android.hardware.Camera.Parameters parameters = camera.getParameters();
     final List<CaptureFormat.FramerateRange> supportedFramerates =
         Camera1Enumerator.convertFramerates(parameters.getSupportedPreviewFpsRange());
     Logging.d(TAG, "Available fps ranges: " + supportedFramerates);
 
     final CaptureFormat.FramerateRange fpsRange =
-        CameraEnumerationAndroid.getClosestSupportedFramerateRange(supportedFramerates, framerate);
+        CameraEnumerationAndroid.getClosestSupportedFramerateRange(
+            supportedFramerates, requestedFramerate);
 
     final Size previewSize = CameraEnumerationAndroid.getClosestSupportedSize(
-        Camera1Enumerator.convertSizes(parameters.getSupportedPreviewSizes()), width, height);
+        Camera1Enumerator.convertSizes(parameters.getSupportedPreviewSizes()),
+        requestedWidth, requestedHeight);
 
     final CaptureFormat captureFormat =
         new CaptureFormat(previewSize.width, previewSize.height, fpsRange);
 
     // Check if we are already using this capture format, then we don't need to do anything.
     if (captureFormat.equals(this.captureFormat)) {
       return;
     }
 
     // Update camera parameters.
     Logging.d(TAG, "isVideoStabilizationSupported: " +
         parameters.isVideoStabilizationSupported());
     if (parameters.isVideoStabilizationSupported()) {
       parameters.setVideoStabilization(true);
     }
     // Note: setRecordingHint(true) actually decrease frame rate on N5.
     // parameters.setRecordingHint(true);
     if (captureFormat.framerate.max > 0) {
       parameters.setPreviewFpsRange(captureFormat.framerate.min, captureFormat.framerate.max);
     }
     parameters.setPreviewSize(previewSize.width, previewSize.height);
 
     if (!isCapturingToTexture) {
       parameters.setPreviewFormat(captureFormat.imageFormat);
     }
     // Picture size is for taking pictures and not for preview/video, but we need to set it anyway
     // as a workaround for an aspect ratio problem on Nexus 7.
     final Size pictureSize = CameraEnumerationAndroid.getClosestSupportedSize(
-        Camera1Enumerator.convertSizes(parameters.getSupportedPictureSizes()), width, height);
+        Camera1Enumerator.convertSizes(parameters.getSupportedPictureSizes()),
+        requestedWidth, requestedHeight);
     parameters.setPictureSize(pictureSize.width, pictureSize.height);
 
     // Temporarily stop preview if it's already running.
     if (this.captureFormat != null) {
       camera.stopPreview();
       // Calling |setPreviewCallbackWithBuffer| with null should clear the internal camera buffer
       // queue, but sometimes we receive a frame with the old resolution after this call anyway.
       camera.setPreviewCallbackWithBuffer(null);
     }
 
@@ skipping 17 matching lines @@
         queuedBuffers.add(buffer.array());
         camera.addCallbackBuffer(buffer.array());
       }
       camera.setPreviewCallbackWithBuffer(this);
     }
     camera.startPreview();
   }
 
   // Blocks until camera is known to be stopped.
   @Override
-  public void stopCapture() throws InterruptedException {
+  public void stopCapture() {
     Logging.d(TAG, "stopCapture");
-    final CountDownLatch barrier = new CountDownLatch(1);
-    final boolean didPost = maybePostOnCameraThread(new Runnable() {
+
+    postOnCameraThread(new Runnable() {
       @Override public void run() {
-        stopCaptureOnCameraThread(true /* stopHandler */);
-        barrier.countDown();
+        stopCaptureOnCameraThread();
       }
     });
-    if (!didPost) {
-      Logging.e(TAG, "Calling stopCapture() for already stopped camera.");
-      return;
-    }
-    if (!barrier.await(CAMERA_STOP_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
-      Logging.e(TAG, "Camera stop timeout");
-      printStackTrace();
-      if (eventsHandler != null) {
-        eventsHandler.onCameraError("Camera stop timeout");
+
+    synchronized (generateCapturerEvents) {
+      while (generateCapturerEvents.get()) {
+        try {
+          generateCapturerEvents.wait();
+        } catch (InterruptedException e) {
+          Logging.w(TAG, "Interrupt while waiting capturer to stop: "
+              + e.getMessage());
+        }
       }
     }
-    frameObserver.onCapturerStopped();
+
     Logging.d(TAG, "stopCapture done");
   }
 
-  private void stopCaptureOnCameraThread(boolean stopHandler) {
+  private void stopCaptureOnCameraThread() {
     checkIsOnCameraThread();
     Logging.d(TAG, "stopCaptureOnCameraThread");
-    // Note that the camera might still not be started here if startCaptureOnCameraThread failed
-    // and we posted a retry.
+
+    if (cameraState == CameraState.IDLE) {
+      Logging.d(TAG, "Calling stopCapture() for already stopped camera.");

[magjed_webrtc, 2016/07/20 14:56:08]

You need to notify generateCapturerEvents here.

[sakal, 2016/07/21 11:17:24]

I modified code to use the state instead.

+      return;
+    }
+
+    final CameraState oldState = cameraState;
+
+    // Clear the cameraThreadHandler first, in case stopPreview or
+    // other driver code deadlocks. Deadlock in
+    // android.hardware.Camera._stopPreview(Native Method) has
+    // been observed on Nexus 5 (hammerhead), OS version LMY48I.
+    // The camera might post another one or two preview frames
+    // before stopped, so we have to check |isCameraRunning|.
+    // Remove all pending Runnables posted from |this|.
+    cameraThreadHandler.removeCallbacksAndMessages(this /* token */);
+    setCameraState(CameraState.IDLE); // No more frames will be delivered
+    synchronized (generateCapturerEvents) {
+      generateCapturerEvents.set(false);
+      generateCapturerEvents.notifyAll();
+    }
+
+    if (oldState == CameraState.STARTING) {
+      Logging.d(TAG, "Camera starting while calling stopCapture, cancelling.");
+      setCameraState(CameraState.IDLE);
+      return;
+    }
+
+    capturerObserver.onCapturerStopped();

[magjed_webrtc, 2016/07/20 14:56:07]

I think you should notify capturerObserver and eventsHandler regardless of the
state.

[sakal, 2016/07/21 11:17:24]

Done.

+
+    releaseCameraOnCameraThread();
+    if (eventsHandler != null) {
+      eventsHandler.onCameraClosed();
+    }
+
+    Logging.d(TAG, "stopCaptureOnCameraThread done");
+  }
+
+  private void releaseCameraOnCameraThread() {
+    checkIsOnCameraThread();
 
     // Make sure onTextureFrameAvailable() is not called anymore.
-    if (surfaceHelper != null) {
-      surfaceHelper.stopListening();
-    }
-    if (stopHandler) {
-      // Clear the cameraThreadHandler first, in case stopPreview or
-      // other driver code deadlocks. Deadlock in
-      // android.hardware.Camera._stopPreview(Native Method) has
-      // been observed on Nexus 5 (hammerhead), OS version LMY48I.
-      // The camera might post another one or two preview frames
-      // before stopped, so we have to check |isCameraRunning|.
-      // Remove all pending Runnables posted from |this|.
-      isCameraRunning.set(false);
-      cameraThreadHandler.removeCallbacksAndMessages(this /* token */);
-    }
+    surfaceHelper.stopListening();
     if (cameraStatistics != null) {
       cameraStatistics.release();
       cameraStatistics = null;
     }
     Logging.d(TAG, "Stop preview.");
     if (camera != null) {
       camera.stopPreview();
       camera.setPreviewCallbackWithBuffer(null);
     }
     queuedBuffers.clear();
     captureFormat = null;
-
     Logging.d(TAG, "Release camera.");
     if (camera != null) {
       camera.release();
       camera = null;
     }
-    if (eventsHandler != null) {
-      eventsHandler.onCameraClosed();
-    }
-    Logging.d(TAG, "stopCaptureOnCameraThread done");
   }
 
   private void switchCameraOnCameraThread() {
     checkIsOnCameraThread();
-    if (!isCameraRunning.get()) {
+    Logging.d(TAG, "switchCameraOnCameraThread");
+
+    if (cameraState == CameraState.IDLE) {

[magjed_webrtc, 2016/07/20 14:56:07]

Check for UNINITIALIZED as well? Or make the opposite check, i.e. state !=
STARTING && state != RUNNING.

[sakal, 2016/07/21 11:17:24]

I made every public function to check if camera is initialized.

       Logging.e(TAG, "switchCameraOnCameraThread: Camera is stopped");
+      switchEventsHandler.onCameraSwitchError("Camera is stopped.");
       return;
     }
-    Logging.d(TAG, "switchCameraOnCameraThread");
-    stopCaptureOnCameraThread(false /* stopHandler */);
+
+    stopCaptureOnCameraThread();
     synchronized (cameraIdLock) {
       id = (id + 1) % android.hardware.Camera.getNumberOfCameras();
     }
-    startCaptureOnCameraThread(requestedWidth, requestedHeight, requestedFramerate);
+    startCaptureOnCameraThread(requestedWidth, requestedHeight,
+        requestedFramerate);
+
     Logging.d(TAG, "switchCameraOnCameraThread done");
   }
 
   private void onOutputFormatRequestOnCameraThread(int width, int height, int framerate) {
     checkIsOnCameraThread();
     Logging.d(TAG, "onOutputFormatRequestOnCameraThread: " + width + "x" + height +
         "@" + framerate);
-    frameObserver.onOutputFormatRequest(width, height, framerate);
+    capturerObserver.onOutputFormatRequest(width, height, framerate);
   }
 
   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 17 matching lines @@
     if (info.facing == android.hardware.Camera.CameraInfo.CAMERA_FACING_BACK) {
       rotation = 360 - rotation;
     }
     return (info.orientation + rotation) % 360;
   }
 
   // Called on cameraThread so must not "synchronized".
   @Override
   public void onPreviewFrame(byte[] data, android.hardware.Camera callbackCamera) {
     checkIsOnCameraThread();
-    if (!isCameraRunning.get()) {
-      Logging.e(TAG, "onPreviewFrame: Camera is stopped");
+
+    if (cameraState != CameraState.RUNNING) {
+      Logging.d(TAG, "onPreviewFrame: Camera is stopped");
       return;
     }
     if (!queuedBuffers.contains(data)) {
       // |data| is an old invalid buffer.
       return;
     }
     if (camera != callbackCamera) {
       throw new RuntimeException("Unexpected camera in callback!");
     }
 
     final long captureTimeNs =
         TimeUnit.MILLISECONDS.toNanos(SystemClock.elapsedRealtime());
 
     if (eventsHandler != null && !firstFrameReported) {
       eventsHandler.onFirstFrameAvailable();
       firstFrameReported = true;
     }
 
     cameraStatistics.addFrame();
-    frameObserver.onByteBufferFrameCaptured(data, captureFormat.width, captureFormat.height,
+    capturerObserver.onByteBufferFrameCaptured(data, captureFormat.width, captureFormat.height,
         getFrameOrientation(), captureTimeNs);
     camera.addCallbackBuffer(data);
   }
 
   @Override
   public void onTextureFrameAvailable(
       int oesTextureId, float[] transformMatrix, long timestampNs) {
     checkIsOnCameraThread();
-    if (!isCameraRunning.get()) {
-      Logging.e(TAG, "onTextureFrameAvailable: Camera is stopped");
+
+    if (cameraState != CameraState.RUNNING) {
+      Logging.d(TAG, "onTextureFrameAvailable: Camera is stopped");
       surfaceHelper.returnTextureFrame();
       return;
     }
     if (eventsHandler != null && !firstFrameReported) {
       eventsHandler.onFirstFrameAvailable();
       firstFrameReported = true;
     }
 
     int rotation = getFrameOrientation();
     if (info.facing == android.hardware.Camera.CameraInfo.CAMERA_FACING_FRONT) {
       // Undo the mirror that the OS "helps" us with.
       // http://developer.android.com/reference/android/hardware/Camera.html#setDisplayOrientation(int)
       transformMatrix =
           RendererCommon.multiplyMatrices(transformMatrix, RendererCommon.horizontalFlipMatrix());
     }
     cameraStatistics.addFrame();
-    frameObserver.onTextureFrameCaptured(captureFormat.width, captureFormat.height, oesTextureId,
+    capturerObserver.onTextureFrameCaptured(captureFormat.width, captureFormat.height, oesTextureId,
         transformMatrix, rotation, timestampNs);
   }
 }