diff --git a/webrtc/api/android/java/src/org/webrtc/EglRenderer.java b/webrtc/api/android/java/src/org/webrtc/EglRenderer.java index 70d7869269..d3bb13a7e8 100644 --- a/webrtc/api/android/java/src/org/webrtc/EglRenderer.java +++ b/webrtc/api/android/java/src/org/webrtc/EglRenderer.java @@ -76,7 +76,6 @@ public class EglRenderer implements VideoRenderer.Callbacks { private final Object handlerLock = new Object(); private Handler renderThreadHandler; - private final Object frameListenerLock = new Object(); private final ArrayList frameListeners = new ArrayList<>(); // Variables for fps reduction. @@ -364,27 +363,37 @@ public class EglRenderer implements VideoRenderer.Callbacks { * @param scale The scale of the Bitmap passed to the callback, or 0 if no Bitmap is * required. */ - public void addFrameListener(FrameListener listener, float scale) { - synchronized (frameListenerLock) { - frameListeners.add(new ScaleAndFrameListener(scale, listener)); - } + public void addFrameListener(final FrameListener listener, final float scale) { + postToRenderThread(new Runnable() { + @Override + public void run() { + frameListeners.add(new ScaleAndFrameListener(scale, listener)); + } + }); } /** * Remove any pending callback that was added with addFrameListener. If the callback is not in - * the queue, nothing happens. + * the queue, nothing happens. It is ensured that callback won't be called after this method + * returns. * * @param runnable The callback to remove. */ - public void removeFrameListener(FrameListener listener) { - synchronized (frameListenerLock) { - final Iterator iter = frameListeners.iterator(); - while (iter.hasNext()) { - if (iter.next().listener == listener) { - iter.remove(); + public void removeFrameListener(final FrameListener listener) { + final CountDownLatch latch = new CountDownLatch(1); + postToRenderThread(new Runnable() { + @Override + public void run() { + latch.countDown(); + final Iterator iter = frameListeners.iterator(); + while (iter.hasNext()) { + if (iter.next().listener == listener) { + iter.remove(); + } } } - } + }); + ThreadUtils.awaitUninterruptibly(latch); } // VideoRenderer.Callbacks interface. @@ -581,12 +590,10 @@ public class EglRenderer implements VideoRenderer.Callbacks { // Make temporary copy of callback list to avoid ConcurrentModificationException, in case // callbacks call addFramelistener or removeFrameListener. final ArrayList tmpList; - synchronized (frameListenerLock) { - if (frameListeners.isEmpty()) - return; - tmpList = new ArrayList<>(frameListeners); - frameListeners.clear(); - } + if (frameListeners.isEmpty()) + return; + tmpList = new ArrayList<>(frameListeners); + frameListeners.clear(); final float[] bitmapMatrix = RendererCommon.multiplyMatrices( RendererCommon.multiplyMatrices(texMatrix, diff --git a/webrtc/api/androidtests/src/org/webrtc/EglRendererTest.java b/webrtc/api/androidtests/src/org/webrtc/EglRendererTest.java index 2d8bc11bfa..8de4dbab50 100644 --- a/webrtc/api/androidtests/src/org/webrtc/EglRendererTest.java +++ b/webrtc/api/androidtests/src/org/webrtc/EglRendererTest.java @@ -29,6 +29,7 @@ public class EglRendererTest extends InstrumentationTestCase { final static int SURFACE_WAIT_MS = 1000; final static int TEST_FRAME_WIDTH = 4; final static int TEST_FRAME_HEIGHT = 4; + final static int REMOVE_FRAME_LISTENER_RACY_NUM_TESTS = 10; // Some arbitrary frames. final static ByteBuffer[][] TEST_FRAMES = { { @@ -263,4 +264,29 @@ public class EglRendererTest extends InstrumentationTestCase { checkBitmap(testFrameListener.resetAndGetBitmap(), scale); } } + + /** + * Checks that the frame listener will not be called with a frame that was delivered before the + * frame listener was added. + */ + @SmallTest + public void testFrameListenerNotCalledWithOldFrames() throws Exception { + feedFrame(0); + eglRenderer.addFrameListener(testFrameListener, 0f); + // Check the old frame does not trigger frame listener. + assertFalse(testFrameListener.waitForBitmap(RENDER_WAIT_MS)); + } + + /** Checks that the frame listener will not be called after it is removed. */ + @SmallTest + public void testRemoveFrameListenerNotRacy() throws Exception { + for (int i = 0; i < REMOVE_FRAME_LISTENER_RACY_NUM_TESTS; i++) { + feedFrame(0); + eglRenderer.addFrameListener(testFrameListener, 0f); + eglRenderer.removeFrameListener(testFrameListener); + feedFrame(1); + } + // Check the frame listener hasn't triggered. + assertFalse(testFrameListener.waitForBitmap(RENDER_WAIT_MS)); + } }