VideoCapturerAndroid: Fix threading issues

talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.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 20 matching lines @@
 import android.test.suitebuilder.annotation.SmallTest;
 import android.util.Size;
 
 import org.webrtc.CameraEnumerationAndroid.CaptureFormat;
 import org.webrtc.VideoRenderer.I420Frame;
 
 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 {
   static class RendererCallbacks implements VideoRenderer.Callbacks {
     private int framesRendered = 0;
     private Object frameLock = 0;
 
     @Override
     public void renderFrame(I420Frame frame) {
       synchronized (frameLock) {
         ++framesRendered;
         frameLock.notify();
       }
       VideoRenderer.renderFrameDone(frame);
     }
 
     public int WaitForNextFrameToRender() throws InterruptedException {
       synchronized (frameLock) {
         frameLock.wait();
         return framesRendered;
       }
     }
   }
 
-  static class AsyncRenderer implements VideoRenderer.Callbacks {
-    private final List<I420Frame> pendingFrames = new ArrayList<I420Frame>();
-
-    @Override
-    public void renderFrame(I420Frame frame) {
-      synchronized (pendingFrames) {
-        pendingFrames.add(frame);
-        pendingFrames.notifyAll();
-      }
-    }
-
-    // Wait until at least one frame have been received, before returning them.
-    public List<I420Frame> WaitForFrames() {
-      synchronized (pendingFrames) {
-        while (pendingFrames.isEmpty()) {
-          try {
-            pendingFrames.wait();
-          } catch (InterruptedException e) {
-            // Ignore.
-          }
-        }
-        final List<I420Frame> frames = new ArrayList<I420Frame>(pendingFrames);
-        pendingFrames.clear();
-        return frames;
-      }
-    }
-  }
-
   static class FakeCapturerObserver implements
       VideoCapturerAndroid.CapturerObserver {
     private int framesCaptured = 0;
     private int frameSize = 0;
     private Object frameLock = 0;
     private Object capturerStartLock = 0;
     private boolean captureStartResult = false;
     private List<Long> timestamps = new ArrayList<Long>();
 
     @Override
@@ skipping 140 matching lines @@
   // to a Java video renderer using the back facing video capturer.
   // It tests both the Java and the C++ layer.
   public void testStartBackFacingVideoCapturer() throws Exception {
     if (!HaveTwoCameras()) {
       return;
     }
     startCapturerAndRender(CameraEnumerationAndroid.getNameOfBackFacingDevice());
   }
 
   @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 {
     PeerConnectionFactory factory = new PeerConnectionFactory();
     VideoCapturerAndroid capturer = VideoCapturerAndroid.create("", null);
     VideoSource source =
         factory.createVideoSource(capturer, new MediaConstraints());
     VideoTrack track = factory.createVideoTrack("dummy", source);
 
-    if (HaveTwoCameras())
-      assertTrue(capturer.switchCamera(null));
-    else
-      assertFalse(capturer.switchCamera(null));
+    // Array with one element to avoid final problem in nested classes.
+    final boolean[] cameraSwitchSuccessful = new boolean[1];
+    final CountDownLatch barrier = new CountDownLatch(1);
+    capturer.switchCamera(new VideoCapturerAndroid.CameraSwitchHandler() {
+      @Override
+      public void onCameraSwitchDone(boolean isFrontCamera) {
+        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();
 
-    // Wait until the camera have been switched.
-    capturer.runCameraThreadUntilIdle();
-
+    // 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));
     assertTrue(callbacks.WaitForNextFrameToRender() > 0);
     track.dispose();
     source.dispose();
     factory.dispose();
   }
 
   @SmallTest
@@ skipping 78 matching lines @@
 
     observer.WaitForNextCapturedFrame();
     capturer.stopCapture();
 
     listOftimestamps = observer.getCopyAndResetListOftimeStamps();
     assertTrue(listOftimestamps.size() >= 2);
     for (Long timeStamp : listOftimestamps) {
       capturer.returnBuffer(timeStamp);
     }
   }
-
-  @SmallTest
-  // This test that we can capture frames, stop capturing, keep the frames for rendering, and then
-  // return the frames. It tests both the Java and the C++ layer.
-  public void testCaptureAndAsyncRender() {
-    final VideoCapturerAndroid capturer = VideoCapturerAndroid.create("", null);
-    // Helper class that sets everything up, captures at least one frame, and then shuts
-    // everything down.
-    class CaptureFramesRunnable implements Runnable {
-      public List<I420Frame> frames;
-
-      @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 until we get at least one frame.
-        frames = renderer.WaitForFrames();
-
-        // Stop everything.
-        track.dispose();
-        source.dispose();
-        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(), "[]");
-  }

[hbos, 2015/09/18 11:10:52]

I'm thinking it might be worth to tweak this test case so that dispose happens
properly. Looks like it was the only test like this. What do you think?

[magjed_webrtc, 2015/09/21 08:19:57]

Probably true. We have a similar test in testReturnBufferLate(), but it does not
test the android capture JNI layer, and that's where we had a crash recently. I
haved updated the test now.

 }