From f4c10dc560fbe46a4fee8bea9b6d7079d3e03ed0 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Thu, 17 Dec 2015 12:15:48 +0000 Subject: [PATCH] Don't hold a lock while rendering in VpxRenderer. --- .../ext/vp9/LibvpxVideoTrackRenderer.java | 41 ++++------ .../exoplayer/ext/vp9/VpxDecoderWrapper.java | 28 ++++--- .../exoplayer/ext/vp9/VpxOutputBuffer.java | 24 ++++-- .../ext/vp9/VpxOutputBufferRenderer.java | 2 +- .../exoplayer/ext/vp9/VpxRenderer.java | 82 +++++++++++-------- 5 files changed, 98 insertions(+), 79 deletions(-) diff --git a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/LibvpxVideoTrackRenderer.java b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/LibvpxVideoTrackRenderer.java index 8e6f891131..a60f508921 100644 --- a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/LibvpxVideoTrackRenderer.java +++ b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/LibvpxVideoTrackRenderer.java @@ -106,7 +106,6 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { private VpxDecoderWrapper decoder; private VpxInputBuffer inputBuffer; private VpxOutputBuffer outputBuffer; - private VpxOutputBuffer renderedOutputBuffer; private Bitmap bitmap; private boolean drawnToSurface; @@ -181,13 +180,7 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { return; } sourceIsReady = continueBufferingSource(positionUs); - try { - checkForDiscontinuity(positionUs); - } catch (VpxDecoderException e) { - notifyDecoderError(e); - throw new ExoPlaybackException(e); - } - + checkForDiscontinuity(positionUs); // Try and read a format if we don't have one already. if (format == null && !readFormat(positionUs)) { @@ -226,7 +219,7 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { if (outputBuffer.flags == VpxDecoderWrapper.FLAG_END_OF_STREAM) { outputStreamEnded = true; - releaseOutputBuffer(outputBuffer); + decoder.releaseOutputBuffer(outputBuffer); outputBuffer = null; return; } @@ -241,7 +234,7 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { if (droppedFrameCount == maxDroppedFrameCountToNotify) { notifyAndResetDroppedFrameCount(); } - releaseOutputBuffer(outputBuffer); + decoder.releaseOutputBuffer(outputBuffer); outputBuffer = null; return; } @@ -270,7 +263,7 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { renderBuffer(); } - private void renderBuffer() throws VpxDecoderException { + private void renderBuffer() { codecCounters.renderedOutputBufferCount++; notifyIfVideoSizeChanged(outputBuffer); if (outputBuffer.mode == VpxDecoder.OUTPUT_MODE_RGB && surface != null) { @@ -279,22 +272,16 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { drawnToSurface = true; notifyDrawnToSurface(surface); } + outputBuffer.release(); } else if (outputBuffer.mode == VpxDecoder.OUTPUT_MODE_YUV && outputBufferRenderer != null) { + // The renderer will release the buffer. outputBufferRenderer.setOutputBuffer(outputBuffer); + } else { + outputBuffer.release(); } - // Release the output buffer we rendered during the previous cycle, now that we delivered a new - // buffer. - releaseOutputBuffer(renderedOutputBuffer); - renderedOutputBuffer = outputBuffer; outputBuffer = null; } - private void releaseOutputBuffer(VpxOutputBuffer buffer) throws VpxDecoderException { - if (buffer != null) { - decoder.releaseOutputBuffer(buffer); - } - } - private void renderRgbFrame(VpxOutputBuffer outputBuffer, boolean scale) { if (bitmap == null || bitmap.getWidth() != outputBuffer.width || bitmap.getHeight() != outputBuffer.height) { @@ -350,7 +337,7 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { return true; } - private void checkForDiscontinuity(long positionUs) throws VpxDecoderException { + private void checkForDiscontinuity(long positionUs) { if (decoder == null) { return; } @@ -360,12 +347,12 @@ public final class LibvpxVideoTrackRenderer extends SampleSourceTrackRenderer { } } - private void flushDecoder() throws VpxDecoderException { + private void flushDecoder() { inputBuffer = null; - VpxOutputBuffer bufferToRelease = outputBuffer; - // Set this to null now because releaseOutputBuffer could throw an exception. - outputBuffer = null; - releaseOutputBuffer(bufferToRelease); + if (outputBuffer != null) { + decoder.releaseOutputBuffer(outputBuffer); + outputBuffer = null; + } decoder.flush(); } diff --git a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java index 6ad2c1c2c3..5dff64a846 100644 --- a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java +++ b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java @@ -24,14 +24,15 @@ import java.util.LinkedList; /** * Wraps {@link VpxDecoder}, exposing a higher level decoder interface. */ -/* package */ class VpxDecoderWrapper extends Thread { +/* package */ final class VpxDecoderWrapper extends Thread { public static final int FLAG_END_OF_STREAM = 1; private static final int INPUT_BUFFER_SIZE = 768 * 1024; // Value based on cs/SoftVpx.cpp. /** - * The total number of output buffers. {@link LibvpxVideoTrackRenderer} may hold on to 2 buffers - * at a time so this value should be high enough considering LibvpxVideoTrackRenderer requirement. + * The number of input buffers and the number of output buffers. The track renderer may limit the + * minimum possible value due to requiring multiple output buffers to be dequeued at a time for it + * to make progress. */ private static final int NUM_BUFFERS = 16; @@ -66,7 +67,7 @@ import java.util.LinkedList; availableOutputBufferCount = NUM_BUFFERS; for (int i = 0; i < NUM_BUFFERS; i++) { availableInputBuffers[i] = new VpxInputBuffer(); - availableOutputBuffers[i] = new VpxOutputBuffer(); + availableOutputBuffers[i] = new VpxOutputBuffer(this); } } @@ -105,19 +106,22 @@ import java.util.LinkedList; if (queuedOutputBuffers.isEmpty()) { return null; } - VpxOutputBuffer outputBuffer = queuedOutputBuffers.removeFirst(); - return outputBuffer; + return queuedOutputBuffers.removeFirst(); } } - public void releaseOutputBuffer(VpxOutputBuffer outputBuffer) throws VpxDecoderException { + public void releaseOutputBuffer(VpxOutputBuffer outputBuffer) { synchronized (lock) { - maybeThrowDecoderError(); availableOutputBuffers[availableOutputBufferCount++] = outputBuffer; maybeNotifyDecodeLoop(); } } + /** + * Flushes input/output buffers that have not been dequeued yet and returns ownership of any + * dequeued input buffer to the decoder. Flushes any pending output currently in the decoder. The + * caller is still responsible for releasing any dequeued output buffers. + */ public void flush() { synchronized (lock) { flushDecodedOutputBuffer = true; @@ -159,7 +163,7 @@ import java.util.LinkedList; * Should only be called whilst synchronized on the lock object. */ private void maybeNotifyDecodeLoop() { - if (!queuedInputBuffers.isEmpty() && availableOutputBufferCount > 0) { + if (canDecodeBuffer()) { lock.notify(); } } @@ -192,7 +196,7 @@ import java.util.LinkedList; // Wait until we have an input buffer to decode, and an output buffer to decode into. synchronized (lock) { - while (!released && (queuedInputBuffers.isEmpty() || availableOutputBufferCount == 0)) { + while (!released && !canDecodeBuffer()) { lock.wait(); } if (released) { @@ -238,6 +242,10 @@ import java.util.LinkedList; return true; } + private boolean canDecodeBuffer() { + return !queuedInputBuffers.isEmpty() && availableOutputBufferCount > 0; + } + /* package */ static final class VpxInputBuffer { public final SampleHolder sampleHolder; diff --git a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBuffer.java b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBuffer.java index a509e7bcb2..1a2b40a069 100644 --- a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBuffer.java +++ b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBuffer.java @@ -26,6 +26,12 @@ public final class VpxOutputBuffer { public static final int COLORSPACE_BT601 = 1; public static final int COLORSPACE_BT709 = 2; + private final VpxDecoderWrapper decoder; + + /* package */ VpxOutputBuffer(VpxDecoderWrapper decoder) { + this.decoder = decoder; + } + /** * RGB buffer for RGB mode. */ @@ -43,10 +49,16 @@ public final class VpxOutputBuffer { public int colorspace; /** - * This method is called from C++ through JNI after decoding is done. It will resize the - * buffer based on the given dimensions. + * Releases the buffer back to the decoder, allowing it to be reused. */ - public void initForRgbFrame(int width, int height) { + public void release() { + decoder.releaseOutputBuffer(this); + } + + /** + * Resizes the buffer based on the given dimensions. Called via JNI after decoding completes. + */ + /* package */ void initForRgbFrame(int width, int height) { this.width = width; this.height = height; int minimumRgbSize = width * height * 2; @@ -59,10 +71,10 @@ public final class VpxOutputBuffer { } /** - * This method is called from C++ through JNI after decoding is done. It will resize the - * buffer based on the given stride. + * Resizes the buffer based on the given stride. Called via JNI after decoding completes. */ - public void initForYuvFrame(int width, int height, int yStride, int uvStride, int colorspace) { + /* package */ void initForYuvFrame(int width, int height, int yStride, int uvStride, + int colorspace) { this.width = width; this.height = height; this.colorspace = colorspace; diff --git a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBufferRenderer.java b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBufferRenderer.java index 99855ce8df..0ab1fc475e 100644 --- a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBufferRenderer.java +++ b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxOutputBufferRenderer.java @@ -21,7 +21,7 @@ package com.google.android.exoplayer.ext.vp9; public interface VpxOutputBufferRenderer { /** - * Sets the output buffer to be rendered. + * Sets the output buffer to be rendered. The renderer is responsible for releasing the buffer. */ void setOutputBuffer(VpxOutputBuffer outputBuffer); diff --git a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxRenderer.java b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxRenderer.java index 6deeab5c3d..508e77deac 100644 --- a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxRenderer.java +++ b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxRenderer.java @@ -21,6 +21,7 @@ import android.opengl.GLSurfaceView; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.FloatBuffer; +import java.util.concurrent.atomic.AtomicReference; import javax.microedition.khronos.egl.EGLConfig; import javax.microedition.khronos.opengles.GL10; @@ -72,18 +73,21 @@ import javax.microedition.khronos.opengles.GL10; 1.0f, 1.0f, 1.0f, -1.0f); private final int[] yuvTextures = new int[3]; + private final AtomicReference pendingOutputBufferReference; private int program; private int texLocation; private int colorMatrixLocation; private FloatBuffer textureCoords; - private VpxOutputBuffer outputBuffer; private int previousWidth; private int previousStride; + private VpxOutputBuffer renderedOutputBuffer; // Accessed only from the GL thread. + public VpxRenderer() { previousWidth = -1; previousStride = -1; + pendingOutputBufferReference = new AtomicReference<>(); } /** @@ -92,8 +96,12 @@ import javax.microedition.khronos.opengles.GL10; * * @param outputBuffer OutputBuffer containing the YUV Frame to be rendered */ - public synchronized void setFrame(VpxOutputBuffer outputBuffer) { - this.outputBuffer = outputBuffer; + public void setFrame(VpxOutputBuffer outputBuffer) { + VpxOutputBuffer oldPendingOutputBuffer = pendingOutputBufferReference.getAndSet(outputBuffer); + if (oldPendingOutputBuffer != null) { + // The old pending output buffer will never be used for rendering, so release it now. + oldPendingOutputBuffer.release(); + } } @Override @@ -134,40 +142,44 @@ import javax.microedition.khronos.opengles.GL10; @Override public void onDrawFrame(GL10 unused) { - synchronized (this) { - VpxOutputBuffer outputBuffer = this.outputBuffer; - if (outputBuffer == null) { - // Nothing to render yet. - return; + VpxOutputBuffer pendingOutputBuffer = pendingOutputBufferReference.getAndSet(null); + if (pendingOutputBuffer == null && renderedOutputBuffer == null) { + // There is no output buffer to render at the moment. + return; + } + if (pendingOutputBuffer != null) { + if (renderedOutputBuffer != null) { + renderedOutputBuffer.release(); } + renderedOutputBuffer = pendingOutputBuffer; + } + VpxOutputBuffer outputBuffer = renderedOutputBuffer; + // Set color matrix. Assume BT709 if the color space is unknown. + float[] colorConversion = outputBuffer.colorspace == VpxOutputBuffer.COLORSPACE_BT601 + ? kColorConversion601 : kColorConversion709; + GLES20.glUniformMatrix3fv(colorMatrixLocation, 1, false, colorConversion, 0); - // Set color matrix. Assume BT709 if the color space is unknown. - float[] colorConversion = outputBuffer.colorspace == VpxOutputBuffer.COLORSPACE_BT601 - ? kColorConversion601 : kColorConversion709; - GLES20.glUniformMatrix3fv(colorMatrixLocation, 1, false, colorConversion, 0); - - for (int i = 0; i < 3; i++) { - int h = (i == 0) ? outputBuffer.height : (outputBuffer.height + 1) / 2; - GLES20.glActiveTexture(GLES20.GL_TEXTURE0 + i); - GLES20.glBindTexture(GLES20.GL_TEXTURE_2D, yuvTextures[i]); - GLES20.glPixelStorei(GLES20.GL_UNPACK_ALIGNMENT, 1); - GLES20.glTexImage2D(GLES20.GL_TEXTURE_2D, 0, GLES20.GL_LUMINANCE, - outputBuffer.yuvStrides[i], h, 0, GLES20.GL_LUMINANCE, GLES20.GL_UNSIGNED_BYTE, - outputBuffer.yuvPlanes[i]); - } - // Set cropping of stride if either width or stride has changed. - if (previousWidth != outputBuffer.width || previousStride != outputBuffer.yuvStrides[0]) { - float crop = (float) outputBuffer.width / outputBuffer.yuvStrides[0]; - textureCoords = nativeFloatBuffer( - 0.0f, 0.0f, - 0.0f, 1.0f, - crop, 0.0f, - crop, 1.0f); - GLES20.glVertexAttribPointer( - texLocation, 2, GLES20.GL_FLOAT, false, 0, textureCoords); - previousWidth = outputBuffer.width; - previousStride = outputBuffer.yuvStrides[0]; - } + for (int i = 0; i < 3; i++) { + int h = (i == 0) ? outputBuffer.height : (outputBuffer.height + 1) / 2; + GLES20.glActiveTexture(GLES20.GL_TEXTURE0 + i); + GLES20.glBindTexture(GLES20.GL_TEXTURE_2D, yuvTextures[i]); + GLES20.glPixelStorei(GLES20.GL_UNPACK_ALIGNMENT, 1); + GLES20.glTexImage2D(GLES20.GL_TEXTURE_2D, 0, GLES20.GL_LUMINANCE, + outputBuffer.yuvStrides[i], h, 0, GLES20.GL_LUMINANCE, GLES20.GL_UNSIGNED_BYTE, + outputBuffer.yuvPlanes[i]); + } + // Set cropping of stride if either width or stride has changed. + if (previousWidth != outputBuffer.width || previousStride != outputBuffer.yuvStrides[0]) { + float crop = (float) outputBuffer.width / outputBuffer.yuvStrides[0]; + textureCoords = nativeFloatBuffer( + 0.0f, 0.0f, + 0.0f, 1.0f, + crop, 0.0f, + crop, 1.0f); + GLES20.glVertexAttribPointer( + texLocation, 2, GLES20.GL_FLOAT, false, 0, textureCoords); + previousWidth = outputBuffer.width; + previousStride = outputBuffer.yuvStrides[0]; } GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT); GLES20.glDrawArrays(GLES20.GL_TRIANGLE_STRIP, 0, 4);