Android MediaCodecVideoDecoder: Manage lifetime of texture frames

talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java

Index: talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java
diff --git a/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java b/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java
index 42af9c7fd00e8c618cfbc48fb27e49cd481f9b22..6ef94378cd80b0b481b7fd6f7afd3840477377b0 100644
--- a/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java
+++ b/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java
@@ -27,24 +27,23 @@
 
 package org.webrtc;
 
-import android.graphics.SurfaceTexture;
 import android.media.MediaCodec;
 import android.media.MediaCodecInfo;
 import android.media.MediaCodecInfo.CodecCapabilities;
 import android.media.MediaCodecList;
 import android.media.MediaFormat;
-import android.opengl.GLES11Ext;
-import android.opengl.GLES20;
 import android.os.Build;
+import android.os.SystemClock;
 import android.view.Surface;
 
 import org.webrtc.Logging;
 
 import java.nio.ByteBuffer;
 import java.util.Arrays;
+import java.util.LinkedList;
 import java.util.List;
-
-import javax.microedition.khronos.egl.EGLContext;
+import java.util.Queue;
+import java.util.concurrent.TimeUnit;
 
 // Java-side of peerconnection_jni.cc:MediaCodecVideoDecoder.
 // This class is an implementation detail of the Java PeerConnection API.
@@ -96,14 +95,18 @@ public class MediaCodecVideoDecoder {
   private int height;
   private int stride;
   private int sliceHeight;
+  private boolean hasDecodedFirstFrame;
+  private final Queue<Long> decodeStartTimeMs = new LinkedList<Long>();
   private boolean useSurface;
-  private int textureID = 0;
-  private SurfaceTexture surfaceTexture = null;
-  private Surface surface = null;
-  private EglBase eglBase;
 
-  private MediaCodecVideoDecoder() {
-  }
+  // The below variables are only used when the decode decodes to a Surface.
+  private TextureListener textureListener;
+  // |isWaitingForTexture| is true when waiting for the transition:
+  // MediaCodec.releaseOutputBuffer() -> onTextureFrameAvailable().
+  private boolean isWaitingForTexture;
+  private Surface surface = null;
+  private final Queue<DecodedOutputBuffer>
+      dequeuedSurfaceOutputBuffers = new LinkedList<DecodedOutputBuffer>();
 
   // Helper struct for findVp8Decoder() below.
   private static class DecoderProperties {
@@ -197,12 +200,13 @@ public class MediaCodecVideoDecoder {
     }
   }
 
-  // Pass null in |sharedContext| to configure the codec for ByteBuffer output.
-  private boolean initDecode(VideoCodecType type, int width, int height, EGLContext sharedContext) {
+  // Pass null in |surfaceTextureHelper| to configure the codec for ByteBuffer output.
+  private boolean initDecode(
+      VideoCodecType type, int width, int height, SurfaceTextureHelper surfaceTextureHelper) {
     if (mediaCodecThread != null) {
       throw new RuntimeException("Forgot to release()?");
     }
-    useSurface = (sharedContext != null);
+    useSurface = (surfaceTextureHelper != null);
     String mime = null;
     String[] supportedCodecPrefixes = null;
     if (type == VideoCodecType.VIDEO_CODEC_VP8) {
@@ -221,9 +225,6 @@ public class MediaCodecVideoDecoder {
     Logging.d(TAG, "Java initDecode: " + type + " : "+ width + " x " + height +
         ". Color: 0x" + Integer.toHexString(properties.colorFormat) +
         ". Use Surface: " + useSurface);
-    if (sharedContext != null) {
-      Logging.d(TAG, "Decoder shared EGL Context: " + sharedContext);
-    }
     runningInstance = this; // Decoder is now running and can be queried for stack traces.
     mediaCodecThread = Thread.currentThread();
     try {
@@ -233,16 +234,8 @@ public class MediaCodecVideoDecoder {
       sliceHeight = height;
 
       if (useSurface) {
-        // Create shared EGL context.
-        eglBase = new EglBase(sharedContext, EglBase.ConfigType.PIXEL_BUFFER);
-        eglBase.createDummyPbufferSurface();
-        eglBase.makeCurrent();
-
-        // Create output surface
-        textureID = GlUtil.generateTexture(GLES11Ext.GL_TEXTURE_EXTERNAL_OES);
-        Logging.d(TAG, "Video decoder TextureID = " + textureID);
-        surfaceTexture = new SurfaceTexture(textureID);
-        surface = new Surface(surfaceTexture);
+        textureListener = new TextureListener(surfaceTextureHelper);
+        surface = new Surface(surfaceTextureHelper.getSurfaceTexture());
       }
 
       MediaFormat format = MediaFormat.createVideoFormat(mime, width, height);
@@ -281,15 +274,15 @@ public class MediaCodecVideoDecoder {
     }
     mediaCodec = null;
     mediaCodecThread = null;
+    decodeStartTimeMs.clear();
+    hasDecodedFirstFrame = false;
     runningInstance = null;
+    dequeuedSurfaceOutputBuffers.clear();
+    isWaitingForTexture = false;
     if (useSurface) {
       surface.release();
       surface = null;
-      Logging.d(TAG, "Delete video decoder TextureID " + textureID);
-      GLES20.glDeleteTextures(1, new int[] {textureID}, 0);
-      textureID = 0;
-      eglBase.release();
-      eglBase = null;
+      textureListener.release();
     }
     Logging.d(TAG, "Java releaseDecoder done");
   }
@@ -312,6 +305,8 @@ public class MediaCodecVideoDecoder {
     try {
       inputBuffers[inputBufferIndex].position(0);
       inputBuffers[inputBufferIndex].limit(size);
+      decodeStartTimeMs.add(SystemClock.elapsedRealtime());
+

[magjed_webrtc, 2015/10/30 12:28:10]

I don't think we need this empty line.

[perkj_webrtc, 2015/11/06 14:31:12]

Done.

       mediaCodec.queueInputBuffer(inputBufferIndex, 0, size, timestampUs, 0);
       return true;
     }
@@ -321,97 +316,221 @@ public class MediaCodecVideoDecoder {
     }
   }
 
-  // Helper structs for dequeueOutputBuffer() below.
-  private static class DecodedByteBuffer {
-    public DecodedByteBuffer(int index, int offset, int size, long presentationTimestampUs) {
+  // Helper struct for dequeueOutputBuffer() below.
+  private static class DecodedOutputBuffer {
+    public DecodedOutputBuffer(int index, int offset, int size, long presentationTimestampUs,
+        long decodeTime) {
       this.index = index;
       this.offset = offset;
       this.size = size;
       this.presentationTimestampUs = presentationTimestampUs;
+      this.decodeTimeMs = decodeTime;
     }
 
     private final int index;
     private final int offset;
     private final int size;
     private final long presentationTimestampUs;
+    private final long decodeTimeMs;

[AlexG, 2015/10/30 21:06:10]

I would call it decodeLatency - it can be bigger than actual decode time.

[perkj_webrtc, 2015/11/06 14:31:12]

Do you feel strong about it ? I can change here, but 
it is called decode_time in 
webrtc/modules/video_coding/main/source/timing.cc where this eventually end up. 
Also the statistics call this decode time.
webrtc/video/receive_statistics_proxy.cc

   }
 
+  // Helper struct for dequeueTextureBuffer() below.
   private static class DecodedTextureBuffer {
     private final int textureID;
+    private final float[] transformMatrix;
     private final long presentationTimestampUs;
+    private final long decodeTimeMs;
 
-    public DecodedTextureBuffer(int textureID, long presentationTimestampUs) {
+    public DecodedTextureBuffer(int textureID, float[] transformMatrix,
+        long presentationTimestampUs, long decodeTimeMs) {
       this.textureID = textureID;
+      this.transformMatrix = transformMatrix;
       this.presentationTimestampUs = presentationTimestampUs;
+      this.decodeTimeMs = decodeTimeMs;
     }
   }
 
-  // Returns null if no decoded buffer is available, and otherwise either a DecodedByteBuffer or
-  // DecodedTexturebuffer depending on |useSurface| configuration.
+  // Poll based texture listener.
+  private static class TextureListener
+      implements SurfaceTextureHelper.OnTextureFrameAvailableListener {
+    public static class TextureInfo {
+      private final int textureID;
+      private final float[] transformMatrix;
+
+      TextureInfo(int textureId, float[] transformMatrix) {
+        this.textureID = textureId;
+        this.transformMatrix = transformMatrix;
+      }
+    }
+    private final SurfaceTextureHelper surfaceTextureHelper;
+    private TextureInfo textureInfo;
+    // |newFrameLock| is used to synchronize arrival of new frames with wait()/notifyAll().
+    private final Object newFrameLock = new Object();
+
+    public TextureListener(SurfaceTextureHelper surfaceTextureHelper) {
+      this.surfaceTextureHelper = surfaceTextureHelper;
+      surfaceTextureHelper.setListener(this);
+    }
+
+    // Callback from |surfaceTextureHelper|. May be called on an arbitrary thread.
+    @Override
+    public void onTextureFrameAvailable(
+        int oesTextureId, float[] transformMatrix, long timestampNs) {
+      synchronized (newFrameLock) {
+        if (textureInfo != null) {
+          Logging.e(TAG,
+              "Unexpected onTextureFrameAvailable() called while already holding a texture.");
+          throw new IllegalStateException("Already holding a texture.");
+        }
+        // |timestampNs| is always zero on some Android versions.
+        textureInfo = new TextureInfo(oesTextureId, transformMatrix);
+        newFrameLock.notifyAll();
+      }
+    }
+
+    // Dequeues and returns a TextureInfo if available, or null otherwise.
+    public TextureInfo dequeueTextureInfo(int timeoutMs) {
+      synchronized (newFrameLock) {
+        if (textureInfo == null && timeoutMs > 0) {
+          try {
+            newFrameLock.wait(timeoutMs);
+          } catch(InterruptedException e) {
+            // Restore the interrupted status by reinterrupting the thread.
+            Thread.currentThread().interrupt();
+          }
+        }
+        TextureInfo returnedInfo = textureInfo;
+        textureInfo = null;
+        return returnedInfo;
+      }
+    }
+
+    public void release() {
+      // SurfaceTextureHelper.disconnect() will block until any onTextureFrameAvailable() in
+      // progress is done. Therefore, the call to disconnect() must be outside any synchronized
+      // statement that is also used in the onTextureFrameAvailable() above to avoid deadlocks.
+      surfaceTextureHelper.disconnect();
+      synchronized (newFrameLock) {
+        if (textureInfo != null) {
+          surfaceTextureHelper.returnTextureFrame();
+          textureInfo = null;
+        }
+      }
+    }
+  }
+
+  // Returns null if no decoded buffer is available, and otherwise a DecodedByteBuffer.
   // Throws IllegalStateException if call is made on the wrong thread, if color format changes to an
   // unsupported format, or if |mediaCodec| is not in the Executing state. Throws CodecException
   // upon codec error.
-  private Object dequeueOutputBuffer(int dequeueTimeoutUs)
-      throws IllegalStateException, MediaCodec.CodecException {

[magjed_webrtc, 2015/10/30 12:28:10]

Put 'throws IllegalStateException, MediaCodec.CodecException' back.

+  private DecodedOutputBuffer dequeueOutputBuffer(int dequeueTimeoutMs) {
     checkOnMediaCodecThread();
+    if (decodeStartTimeMs.isEmpty()) {
+      return null;
+    }
     // Drain the decoder until receiving a decoded buffer or hitting
     // MediaCodec.INFO_TRY_AGAIN_LATER.
     final MediaCodec.BufferInfo info = new MediaCodec.BufferInfo();
     while (true) {
-      final int result = mediaCodec.dequeueOutputBuffer(info, dequeueTimeoutUs);
+      final int result = mediaCodec.dequeueOutputBuffer(
+          info, TimeUnit.MILLISECONDS.toMicros(dequeueTimeoutMs));
       switch (result) {
-        case MediaCodec.INFO_TRY_AGAIN_LATER:
-          return null;
-        case MediaCodec.INFO_OUTPUT_BUFFERS_CHANGED:
-          outputBuffers = mediaCodec.getOutputBuffers();
-          Logging.d(TAG, "Decoder output buffers changed: " + outputBuffers.length);
-          break;
-        case MediaCodec.INFO_OUTPUT_FORMAT_CHANGED:
-          MediaFormat format = mediaCodec.getOutputFormat();
-          Logging.d(TAG, "Decoder format changed: " + format.toString());
-          width = format.getInteger(MediaFormat.KEY_WIDTH);
-          height = format.getInteger(MediaFormat.KEY_HEIGHT);
-          if (!useSurface && format.containsKey(MediaFormat.KEY_COLOR_FORMAT)) {
-            colorFormat = format.getInteger(MediaFormat.KEY_COLOR_FORMAT);
-            Logging.d(TAG, "Color: 0x" + Integer.toHexString(colorFormat));
-            if (!supportedColorList.contains(colorFormat)) {
-              throw new IllegalStateException("Non supported color format: " + colorFormat);
-            }
-          }
-          if (format.containsKey("stride")) {
-            stride = format.getInteger("stride");
-          }
-          if (format.containsKey("slice-height")) {
-            sliceHeight = format.getInteger("slice-height");
-          }
-          Logging.d(TAG, "Frame stride and slice height: " + stride + " x " + sliceHeight);
-          stride = Math.max(width, stride);
-          sliceHeight = Math.max(height, sliceHeight);
-          break;
-        default:
-          // Output buffer decoded.
-          if (useSurface) {
-            mediaCodec.releaseOutputBuffer(result, true /* render */);
-            // TODO(magjed): Wait for SurfaceTexture.onFrameAvailable() before returning a texture
-            // frame.
-            return new DecodedTextureBuffer(textureID, info.presentationTimeUs);
-          } else {
-            return new DecodedByteBuffer(result, info.offset, info.size, info.presentationTimeUs);
+      case MediaCodec.INFO_OUTPUT_BUFFERS_CHANGED:

[magjed_webrtc, 2015/10/30 12:28:10]

Revert switch case indentation change - it was correct before.

[perkj_webrtc, 2015/11/06 14:31:12]

Done.

+        outputBuffers = mediaCodec.getOutputBuffers();
+        Logging.d(TAG, "Decoder output buffers changed: " + outputBuffers.length);
+        if (hasDecodedFirstFrame) {

[AlexG, 2015/10/30 21:06:10]

May be just ignore it? I think it will be ok if we can swallow up some codec
misbehavior without reporting error.

[perkj_webrtc, 2015/11/06 14:31:12]

Well, what does that mean for the output buffers we are currently using in
dequeuedSurfaceOutputBuffers? I guess we then also must support dropping them
all.

+          throw new RuntimeException("Unexpected output buffer change event.");
+        }
+        break;
+      case MediaCodec.INFO_OUTPUT_FORMAT_CHANGED:
+        MediaFormat format = mediaCodec.getOutputFormat();
+        Logging.d(TAG, "Decoder format changed: " + format.toString());
+        int new_width = format.getInteger(MediaFormat.KEY_WIDTH);
+        int new_height = format.getInteger(MediaFormat.KEY_HEIGHT);
+        if (hasDecodedFirstFrame && (new_width != width || new_height != height)) {

[AlexG, 2015/10/30 21:06:10]

Should we throw an exception here? Why we don't allow on-fly resolution change?

[perkj_webrtc, 2015/11/06 14:31:12]

We reset the decoder from c++
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...

So this is here to make sure that behavior is not changed without modifying the
code in Java. If the width and height changes, we need to read the width and
height into each output frame now that we might render the output buffer to the
Surface at a later time in dequeTextureFrame.

I must admit that I have no idea why this class previously did not support on
the fly size change, I hoped you knew? :-) But I assume that if size change, so
would the outputBuffers which would be a problem for all items in
dequeuedOutputBuffers.

+          throw new RuntimeException("Unexpected size change. Configured " + width + "*" +
+              height + ". New " + new_width + "*" + new_height);
+        }
+        width = format.getInteger(MediaFormat.KEY_WIDTH);
+        height = format.getInteger(MediaFormat.KEY_HEIGHT);
+
+        if (!useSurface && format.containsKey(MediaFormat.KEY_COLOR_FORMAT)) {
+          colorFormat = format.getInteger(MediaFormat.KEY_COLOR_FORMAT);
+          Logging.d(TAG, "Color: 0x" + Integer.toHexString(colorFormat));
+          if (!supportedColorList.contains(colorFormat)) {
+            throw new IllegalStateException("Non supported color format: " + colorFormat);
           }
+        }
+        if (format.containsKey("stride")) {
+          stride = format.getInteger("stride");
+        }
+        if (format.containsKey("slice-height")) {
+          sliceHeight = format.getInteger("slice-height");
+        }
+        Logging.d(TAG, "Frame stride and slice height: " + stride + " x " + sliceHeight);
+        stride = Math.max(width, stride);
+        sliceHeight = Math.max(height, sliceHeight);
+        break;
+      case MediaCodec.INFO_TRY_AGAIN_LATER:
+        return null;
+      default:
+        hasDecodedFirstFrame = true;
+        return new DecodedOutputBuffer(result, info.offset, info.size, info.presentationTimeUs,
+            SystemClock.elapsedRealtime() - decodeStartTimeMs.poll());

[magjed_webrtc, 2015/10/30 12:28:10]

Use remove() instead of poll() because you are not polling. It's an exceptional
circumstance if |decodeStartTimeMs is empty here and throwing a
NoSuchElementException is more appropriate than misleading poll() call followed
by null pointer exception crash.

[perkj_webrtc, 2015/11/06 14:31:12]

Done.

       }
     }
   }
 
+  // Returns null if no decoded buffer is available, and otherwise a DecodedTextureBuffer.
+  // Throws IllegalStateException if call is made on the wrong thread, if color format changes to an
+  // unsupported format, or if |mediaCodec| is not in the Executing state. Throws CodecException
+  // upon codec error.
+  private DecodedTextureBuffer dequeueTextureBuffer(int dequeueTimeoutMs) {
+    checkOnMediaCodecThread();
+    if (!useSurface) {
+      throw new IllegalStateException("dequeueTexture() called for byte buffer decoding.");
+    }
+
+    DecodedOutputBuffer outputBuffer = dequeueOutputBuffer(dequeueTimeoutMs);
+    if (outputBuffer != null) {
+      dequeuedSurfaceOutputBuffers.add(outputBuffer);
+    }
+
+    if (dequeuedSurfaceOutputBuffers.isEmpty())

[AlexG, 2015/10/30 21:06:10]

nit: add brackets

[perkj_webrtc, 2015/11/06 14:31:12]

Done.

+      return null;
+
+    // Get the first frame in the queue.
+    DecodedOutputBuffer firstBuffer = dequeuedSurfaceOutputBuffers.peek();

[AlexG, 2015/10/30 21:06:10]

I think you may add a condition to call releaseOutputBuffer(render = false) if
dequeuedSurfaceOutputBuffers size is more than 2 or 3.

[perkj_webrtc, 2015/11/06 14:31:12]

done

+
+    if (!isWaitingForTexture) {
+      // releaseOutputBuffer renders to the decoder output surface.
+      mediaCodec.releaseOutputBuffer(firstBuffer.index, true /* render */);

[magjed_webrtc, 2015/10/30 12:28:10]

I would prefer to remove |firstBuffer| and write it like this instead:
if (!isWaitingForTexture) {
  // releaseOutputBuffer renders to the decoder output surface.
  mediaCodec.releaseOutputBuffer(
      dequeuedSurfaceOutputBuffers.peek().index, true /* render */);
  isWaitingForTexture = true;
}
and below:
...
  final DecodedOutputBuffer outputBuffer =
      dequeuedSurfaceOutputBuffers.remove();
  return new DecodedTextureBuffer(info.textureID, info.transformMatrix,
      outputBuffer.presentationTimestampUs, outputBuffer.decodeTimeMs);
}

If you feel strongly about reducing decode time caused by polling latency, you
can duplicate the first 5 lines or put the two if-statements in a for-loop with
2 runs.

[perkj_webrtc, 2015/11/06 14:31:12]

ok, I am leaning towards reducing the poll intervall in a follow up cl.

+      isWaitingForTexture = true;
+    }
+
+    // We are waiting for a frame to be rendered to the decoder surface.
+    // Check if it is ready now by waiting max |dequeueTimeoutMs|. There can only be one frame
+    // rendered at the time.

[magjed_webrtc, 2015/10/30 12:28:10]

s/rendered at the time/rendererd at a time/g

[perkj_webrtc, 2015/11/06 14:31:12]

Done.

[magjed_webrtc, 2015/11/09 16:30:52]

No?

+    TextureListener.TextureInfo info = textureListener.dequeueTextureInfo(dequeueTimeoutMs);
+    if (info != null) {
+      isWaitingForTexture = false;
+      dequeuedSurfaceOutputBuffers.remove();
+      return new DecodedTextureBuffer(info.textureID, info.transformMatrix,
+          firstBuffer.presentationTimestampUs, firstBuffer.decodeTimeMs);
+    }
+    return null;
+  }
+
   // Release a dequeued output byte buffer back to the codec for re-use. Should only be called for
   // non-surface decoding.
   // Throws IllegalStateException if the call is made on the wrong thread, if codec is configured
   // for surface decoding, or if |mediaCodec| is not in the Executing state. Throws
   // MediaCodec.CodecException upon codec error.
-  private void returnDecodedByteBuffer(int index)
+  private void returnDecodedOutputBuffer(int index)
       throws IllegalStateException, MediaCodec.CodecException {
     checkOnMediaCodecThread();
     if (useSurface) {
-      throw new IllegalStateException("returnDecodedByteBuffer() called for surface decoding.");
+      throw new IllegalStateException("returnDecodedOutputBuffer() called for surface decoding.");
     }
     mediaCodec.releaseOutputBuffer(index, false /* render */);
   }