VideoCapturerAndroid: Fix threading issues

talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java

Index: talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
diff --git a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
index 8e195ac937c9db19f5a9ce658c8bc483bdb1e460..8a85473b6abf941f2e40186aa40ba65bf1b2753e 100644
--- a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
+++ b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
@@ -38,6 +38,7 @@ 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 {
@@ -62,34 +63,6 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
     }
   }
 
-  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;
@@ -250,7 +223,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
   }
 
   @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 {
@@ -260,14 +233,30 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
         factory.createVideoSource(capturer, new MediaConstraints());
     VideoTrack track = factory.createVideoTrack("dummy", source);
 
-    if (HaveTwoCameras())
-      assertTrue(capturer.switchCamera(null));
-    else
-      assertFalse(capturer.switchCamera(null));
-
-    // Wait until the camera have been switched.
-    capturer.runCameraThreadUntilIdle();
-
+    // 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();
+
+    // 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));
@@ -366,54 +355,4 @@ public class VideoCapturerAndroidTest extends ActivityTestCase {
       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.

 }