Events refactor in Android session based capturing.

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

 /*
  *  Copyright 2016 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 16 matching lines @@
   private static final String TAG = "CameraCapturer";
   private final static int MAX_OPEN_CAMERA_ATTEMPTS = 3;
   private final static int OPEN_CAMERA_DELAY_MS = 500;
   private final static int OPEN_CAMERA_TIMEOUT = 10000;
 
   private final CameraEnumerator cameraEnumerator;
   private final CameraEventsHandler eventsHandler;
   private final Handler uiThreadHandler;
 
   private final CameraSession.CreateSessionCallback createSessionCallback =
       new CameraSession.CreateSessionCallback() {

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

It would be nice with checkIsOnCameraThread for onDone and onFailure.

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

Done.

         @Override
         public void onDone(CameraSession session) {
           Logging.d(TAG, "Create session done");
           uiThreadHandler.removeCallbacks(openCameraTimeoutRunnable);
+          capturerObserver.onCapturerStarted(true /* success */);
           synchronized (stateLock) {
             sessionOpening = false;
             currentSession = session;
             stateLock.notifyAll();
 
             if (switchState == SwitchState.IN_PROGRESS) {
               if (switchEventsHandler != null) {
                 switchEventsHandler.onCameraSwitchDone(
                     cameraEnumerator.isFrontFacing(cameraName));
                 switchEventsHandler = null;
               }
               switchState = SwitchState.IDLE;
             } else if (switchState == SwitchState.PENDING) {
               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--;
 
             if (openAttemptsRemaining <= 0) {
               Logging.w(TAG, "Opening camera failed, passing: " + error);
               sessionOpening = false;
               stateLock.notifyAll();
 
               if (switchState != SwitchState.IDLE) {
                 if (switchEventsHandler != null) {
                   switchEventsHandler.onCameraSwitchError(error);
                   switchEventsHandler = null;
                 }
                 switchState = SwitchState.IDLE;
               }
 
               eventsHandler.onCameraError(error);
             } else {
               Logging.w(TAG, "Opening camera failed, retry: " + error);
 
               createSessionInternal(OPEN_CAMERA_DELAY_MS);
             }
           }
         }
       };
 
+  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() {
       eventsHandler.onCameraError("Camera failed to start within timeout.");
     }
   };
 
   // Initialized on initialize
   // -------------------------
   private Handler cameraThreadHandler;
   private Context applicationContext;
   private CapturerObserver capturerObserver;
   private SurfaceTextureHelper surfaceHelper;
 
   private final Object stateLock = new Object();
   private boolean sessionOpening;                  /* guarded by stateLock */
   private CameraSession currentSession;            /* guarded by stateLock */
   private String cameraName;                       /* guarded by stateLock */
   private int width;                               /* guarded by stateLock */
   private int height;                              /* guarded by stateLock */
   private int framerate;                           /* guarded by stateLock */
   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) {
     if (eventsHandler == null) {
       eventsHandler = new CameraEventsHandler() {
         @Override
         public void onCameraError(String errorDescription) {}
         @Override
         public void onCameraFreezed(String errorDescription) {}
         @Override
@@ skipping 50 matching lines @@
       createSessionInternal(0);
     }
   }
 
   private void createSessionInternal(int delayMs) {
     uiThreadHandler.postDelayed(openCameraTimeoutRunnable, delayMs + OPEN_CAMERA_TIMEOUT);
     cameraThreadHandler.postDelayed(new Runnable() {
       @Override
       public void run() {
         createCameraSession(
-            createSessionCallback,
-            eventsHandler, applicationContext, capturerObserver, surfaceHelper,
+            createSessionCallback, cameraSessionEventsHandler, applicationContext, surfaceHelper,
             cameraName, width, height, framerate);
       }
     }, delayMs);
   }
 
   @Override
   public void stopCapture() {
     Logging.d(TAG, "Stop capture");
 
     synchronized (stateLock) {
       while (sessionOpening) {
         Logging.d(TAG, "Stop capture: Waiting for session to open");
         ThreadUtils.waitUninterruptibly(stateLock);
       }
 
       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");
       }
     }
 
     Logging.d(TAG, "Stop capture done");
   }
 
   @Override
   public void onOutputFormatRequest(final int width, final int height, final int framerate) {
@@ skipping 104 matching lines @@
     Logging.d(TAG, "switchCamera done");
   }
 
   protected String getCameraName() {
     synchronized (stateLock) {
       return cameraName;
     }
   }
 
   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);
 }