Events refactor in Android session based capturing.

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

Index: webrtc/api/android/java/src/org/webrtc/CameraCapturer.java
diff --git a/webrtc/api/android/java/src/org/webrtc/CameraCapturer.java b/webrtc/api/android/java/src/org/webrtc/CameraCapturer.java
index 8724da0785cc6d69558c015d4f599e35af350735..88d54e0c6002db2bd1e9e61eeaa517cabb6e4f4c 100644
--- a/webrtc/api/android/java/src/org/webrtc/CameraCapturer.java
+++ b/webrtc/api/android/java/src/org/webrtc/CameraCapturer.java
@@ -39,6 +39,7 @@ public abstract class CameraCapturer implements CameraVideoCapturer {
         public void onDone(CameraSession session) {
           Logging.d(TAG, "Create session done");
           uiThreadHandler.removeCallbacks(openCameraTimeoutRunnable);
+          capturerObserver.onCapturerStarted(true /* success */);
           synchronized (stateLock) {
             sessionOpening = false;
             currentSession = session;
@@ -55,12 +56,16 @@ public abstract class CameraCapturer implements CameraVideoCapturer {
               switchState = SwitchState.IDLE;
               switchCameraInternal(switchEventsHandler);
             }
+
+            cameraStatistics = new CameraStatistics(surfaceHelper, eventsHandler);
+            firstFrameObserved = false;
           }
         }
 
         @Override
         public void onFailure(String error) {
           uiThreadHandler.removeCallbacks(openCameraTimeoutRunnable);
+          capturerObserver.onCapturerStarted(false /* success */);
           synchronized (stateLock) {
             openAttemptsRemaining--;
 
@@ -87,6 +92,75 @@ public abstract class CameraCapturer implements CameraVideoCapturer {
         }
       };
 
+  private final CameraSession.Events cameraSessionEventsHandler =
+      new CameraSession.Events() {

[magjed_webrtc, 2016/09/16 09:00:37]

nit: Can you move 'new CameraSession.Events() {' up to the previous line (I
think it fits), and reduce indentation to 2 spaces for the rest of the code?

[sakal, 2016/09/16 12:38:58]

Done.

+        public void onCameraError(CameraSession session, String error) {

[magjed_webrtc, 2016/09/16 09:00:37]

It would be nice with checkIsOnCameraThread checks for all these functions, and
comment in Events that they all happen on the camera thread.

[sakal, 2016/09/16 12:38:59]

Done.

+          synchronized (stateLock) {
+            if (session != currentSession) {
+              Logging.w(TAG, "onCameraError from another session: " + error);
+              return;
+            }
+            eventsHandler.onCameraError(error);
+            stopCapture();
+          }
+        }
+
+        public void onCameraOpening(CameraSession session) {
+          synchronized (stateLock) {
+            if (currentSession != null) {
+              Logging.w(TAG, "onCameraOpening while session was open.");
+              return;
+            }
+            eventsHandler.onCameraOpening(cameraName);
+          }
+        }
+
+        public void onCameraClosed(CameraSession session) {
+          synchronized (stateLock) {
+            if (session != currentSession && currentSession != null) {
+              Logging.d(TAG, "onCameraClosed from another session.");
+              return;
+            }
+            eventsHandler.onCameraClosed();

[magjed_webrtc, 2016/09/15 14:58:06]

Now we hold stateLock while making the callbacks. Does that increase the risk of
deadlock?

[sakal, 2016/09/15 15:08:18]

If the callbacks invoke method on other threads, deadlocks can happen. I don't
think in practice anyone does so.

+          }
+        }
+
+        public void onByteBufferFrameCaptured(
+            CameraSession session, byte[] data, int width, int height, int rotation,
+            long timestamp) {
+          synchronized (stateLock) {
+            if (session != currentSession) {

[magjed_webrtc, 2016/09/15 14:58:06]

Can we drop frames if we are in an incorrect state, e.g. stopping, as well?

[sakal, 2016/09/15 15:08:18]

I am not sure what you mean. The stopping state is inside the session object.
The session object drops frames itself if it is in stopping state. We should
never be holding a session that is in stopping state anyway.

[magjed_webrtc, 2016/09/16 09:00:37]

I was thinking if the frame dropping in the session could be made redundant by
some check here.

[sakal, 2016/09/16 12:38:58]

It is already redundant since we don't keep closed sessions. I kept it for extra
safety. Do you want to remove it?

[magjed_webrtc, 2016/09/16 12:59:19]

Good, no keep it.

+              Logging.w(TAG, "onByteBufferFrameCaptured from another session.");
+              return;
+            }
+            if (!firstFrameObserved) {
+              eventsHandler.onFirstFrameAvailable();
+              firstFrameObserved = true;
+            }
+            cameraStatistics.addFrame();
+            capturerObserver.onByteBufferFrameCaptured(data, width, height, rotation, timestamp);
+          }
+        }
+
+        public void onTextureFrameCaptured(
+            CameraSession session, int width, int height, int oesTextureId, float[] transformMatrix,
+            int rotation, long timestamp) {
+          synchronized (stateLock) {
+            if (session != currentSession) {
+              Logging.w(TAG, "onTextureFrameCaptured from another session.");
+              return;
+            }
+            if (!firstFrameObserved) {
+              eventsHandler.onFirstFrameAvailable();
+              firstFrameObserved = true;
+            }
+            cameraStatistics.addFrame();
+            capturerObserver.onTextureFrameCaptured(
+                width, height, oesTextureId, transformMatrix, rotation, timestamp);
+          }
+        }
+      };
+
   private final Runnable openCameraTimeoutRunnable = new Runnable() {
     @Override
     public void run() {
@@ -111,6 +185,9 @@ public abstract class CameraCapturer implements CameraVideoCapturer {
   private int openAttemptsRemaining;               /* guarded by stateLock */
   private SwitchState switchState = SwitchState.IDLE; /* guarded by stateLock */
   private CameraSwitchHandler switchEventsHandler; /* guarded by stateLock */
+  // Valid from onDone call until stopCapture, otherwise null.
+  private CameraStatistics cameraStatistics;       /* guarded by stateLock */
+  private boolean firstFrameObserved;              /* guarded by stateLock */
 
   public CameraCapturer(
       String cameraName, CameraEventsHandler eventsHandler, CameraEnumerator cameraEnumerator) {
@@ -181,8 +258,7 @@ public abstract class CameraCapturer implements CameraVideoCapturer {
       @Override
       public void run() {
         createCameraSession(
-            createSessionCallback,
-            eventsHandler, applicationContext, capturerObserver, surfaceHelper,
+            createSessionCallback, cameraSessionEventsHandler, applicationContext, surfaceHelper,
             cameraName, width, height, framerate);
       }
     }, delayMs);
@@ -199,9 +275,11 @@ public abstract class CameraCapturer implements CameraVideoCapturer {
       }
 
       if (currentSession != null) {
-        Logging.d(TAG, "Stop capture: Stopping session");
+        cameraStatistics.release();
+        cameraStatistics = null;
         currentSession.stop();
         currentSession = null;
+        capturerObserver.onCapturerStopped();
       } else {
         Logging.d(TAG, "Stop capture: No session open");
       }
@@ -326,9 +404,7 @@ public abstract class CameraCapturer implements CameraVideoCapturer {
   }
 
   abstract protected void createCameraSession(
-      CameraSession.CreateSessionCallback createSessionCallback,
-      CameraEventsHandler eventsHandler, Context applicationContext,
-      CameraVideoCapturer.CapturerObserver capturerObserver,
-      SurfaceTextureHelper surfaceTextureHelper,
+      CameraSession.CreateSessionCallback createSessionCallback, CameraSession.Events events,
+      Context applicationContext, SurfaceTextureHelper surfaceTextureHelper,
       String cameraName, int width, int height, int framerate);
 }