Fix some bugs in the HardwareVideoDecoder.

This change preserves rotation through the decoder, rather than requiring
callers to keep track of rotation.  The test now uses a non-zero rotation
to ensure it is preserved.

Commit 3814524 inadvertently reverted several changes that weren't merged
properly before submit.  In particular, it clobbered some log messages,
comments, and error codes.  This change fixes those mistakes.

BUG=webrtc:7760

Change-Id: If529ee59fc56de7937e362dc15591295e2cf9f79
Reviewed-on: https://chromium-review.googlesource.com/546415
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#18752}

webrtc/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoDecoderTest.java

@@ -67,6 +67,7 @@ public final class HardwareVideoDecoderTest {
     VideoDecoder decoder = decoderFactory.createDecoder(supportedCodecs[0].name);
 
     final long presentationTimestampUs = 20000;
+    final int rotation = 270;
 
     final CountDownLatch decodeDone = new CountDownLatch(1);
     final AtomicReference<VideoFrame> decoded = new AtomicReference<>();
@@ -98,7 +99,7 @@ public final class HardwareVideoDecoderTest {
     // First, encode a frame.
     VideoFrame.I420Buffer buffer = new I420BufferImpl(SETTINGS.width, SETTINGS.height);
     VideoFrame frame =
-        new VideoFrame(buffer, 0 /* rotation */, presentationTimestampUs * 1000, new Matrix());
+        new VideoFrame(buffer, rotation, presentationTimestampUs * 1000, new Matrix());
     VideoEncoder.EncodeInfo info = new VideoEncoder.EncodeInfo(
         new EncodedImage.FrameType[] {EncodedImage.FrameType.VideoFrameKey});
 
@@ -113,11 +114,11 @@ public final class HardwareVideoDecoderTest {
     ThreadUtils.awaitUninterruptibly(decodeDone);
 
     frame = decoded.get();
-    assertEquals(frame.getRotation(), 0);
+    assertEquals(frame.getRotation(), rotation);
     assertEquals(frame.getTimestampNs(), presentationTimestampUs * 1000);
     assertEquals(frame.getTransformMatrix(), new Matrix());
-    assertEquals(frame.getBuffer().getWidth(), SETTINGS.width);
+    assertEquals(frame.getWidth(), SETTINGS.width);
-    assertEquals(frame.getBuffer().getHeight(), SETTINGS.height);
+    assertEquals(frame.getHeight(), SETTINGS.height);
 
     assertEquals(decoder.release(), VideoCodecStatus.OK);
     assertEquals(encoder.release(), VideoCodecStatus.OK);

webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java

@@ -50,7 +50,18 @@ class HardwareVideoDecoder implements VideoDecoder {
 
   private final String codecName;
   private final VideoCodecType codecType;
-  private final Deque<Long> decodeStartTimes;
+
+  private static class FrameInfo {
+    final long decodeStartTimeMs;
+    final int rotation;
+
+    FrameInfo(long decodeStartTimeMs, int rotation) {
+      this.decodeStartTimeMs = decodeStartTimeMs;
+      this.rotation = rotation;
+    }
+  }
+
+  private final Deque<FrameInfo> frameInfos;
   private int colorFormat;
 
   // Output thread runs a loop which polls MediaCodec for decoded output buffers.  It reformats
@@ -95,7 +106,7 @@ class HardwareVideoDecoder implements VideoDecoder {
     this.codecName = codecName;
     this.codecType = codecType;
     this.colorFormat = colorFormat;
-    this.decodeStartTimes = new LinkedBlockingDeque<>();
+    this.frameInfos = new LinkedBlockingDeque<>();
   }
 
   @Override
@@ -161,8 +172,8 @@ class HardwareVideoDecoder implements VideoDecoder {
 
     int size = frame.buffer.remaining();
     if (size == 0) {
-      Logging.e(TAG, "decode() - no input data");
+      Logging.e(TAG, "decode() - input buffer empty");
-      return VideoCodecStatus.ERROR;
+      return VideoCodecStatus.ERR_PARAMETER;
     }
 
     // Load dimensions from shared memory under the dimension lock.
@@ -177,7 +188,7 @@ class HardwareVideoDecoder implements VideoDecoder {
         && (frame.encodedWidth != width || frame.encodedHeight != height)) {
       VideoCodecStatus status = reinitDecode(frame.encodedWidth, frame.encodedHeight);
       if (status != VideoCodecStatus.OK) {
-        return VideoCodecStatus.FALLBACK_SOFTWARE;
+        return status;
       }
     }
 
@@ -208,13 +219,13 @@ class HardwareVideoDecoder implements VideoDecoder {
       index = codec.dequeueInputBuffer(0 /* timeout */);
     } catch (IllegalStateException e) {
       Logging.e(TAG, "dequeueInputBuffer failed", e);
-      return VideoCodecStatus.FALLBACK_SOFTWARE;
+      return VideoCodecStatus.ERROR;
     }
     if (index < 0) {
       // Decoder is falling behind.  No input buffers available.
       // The decoder can't simply drop frames; it might lose a key frame.
       Logging.e(TAG, "decode() - no HW buffers available; decoder falling behind");
-      return VideoCodecStatus.FALLBACK_SOFTWARE;
+      return VideoCodecStatus.ERROR;
     }
 
     ByteBuffer buffer;
@@ -222,23 +233,23 @@ class HardwareVideoDecoder implements VideoDecoder {
       buffer = codec.getInputBuffers()[index];
     } catch (IllegalStateException e) {
       Logging.e(TAG, "getInputBuffers failed", e);
-      return VideoCodecStatus.FALLBACK_SOFTWARE;
+      return VideoCodecStatus.ERROR;
     }
 
     if (buffer.capacity() < size) {
       Logging.e(TAG, "decode() - HW buffer too small");
-      return VideoCodecStatus.FALLBACK_SOFTWARE;
+      return VideoCodecStatus.ERROR;
     }
     buffer.put(frame.buffer);
 
-    decodeStartTimes.offer(SystemClock.elapsedRealtime());
+    frameInfos.offer(new FrameInfo(SystemClock.elapsedRealtime(), frame.rotation));
     try {
       codec.queueInputBuffer(
           index, 0 /* offset */, size, frame.captureTimeMs * 1000, 0 /* flags */);
     } catch (IllegalStateException e) {
       Logging.e(TAG, "queueInputBuffer failed", e);
-      decodeStartTimes.pollLast();
+      frameInfos.pollLast();
-      return VideoCodecStatus.FALLBACK_SOFTWARE;
+      return VideoCodecStatus.ERROR;
     }
     if (keyFrameRequired) {
       keyFrameRequired = false;
@@ -278,7 +289,7 @@ class HardwareVideoDecoder implements VideoDecoder {
       codec = null;
       callback = null;
       outputThread = null;
-      decodeStartTimes.clear();
+      frameInfos.clear();
     }
     return VideoCodecStatus.OK;
   }
@@ -325,10 +336,12 @@ class HardwareVideoDecoder implements VideoDecoder {
         return;
       }
 
-      Long decodeStartTimeMs = decodeStartTimes.poll();
+      FrameInfo frameInfo = frameInfos.poll();
       Integer decodeTimeMs = null;
-      if (decodeStartTimeMs != null) {
+      int rotation = 0;
-        decodeTimeMs = (int) (SystemClock.elapsedRealtime() - decodeStartTimeMs);
+      if (frameInfo != null) {
+        decodeTimeMs = (int) (SystemClock.elapsedRealtime() - frameInfo.decodeStartTimeMs);
+        rotation = frameInfo.rotation;
       }
 
       hasDecodedFirstFrame = true;
@@ -374,8 +387,7 @@ class HardwareVideoDecoder implements VideoDecoder {
       codec.releaseOutputBuffer(result, false);
 
       long presentationTimeNs = info.presentationTimeUs * 1000;
-      VideoFrame frame =
+      VideoFrame frame = new VideoFrame(frameBuffer, rotation, presentationTimeNs, new Matrix());
-          new VideoFrame(frameBuffer, 0 /* rotation */, presentationTimeNs, new Matrix());
 
       // Note that qp is parsed on the C++ side.
       callback.onDecodedFrame(frame, decodeTimeMs, null /* qp */);
@@ -454,7 +466,7 @@ class HardwareVideoDecoder implements VideoDecoder {
     codec = null;
     callback = null;
     outputThread = null;
-    decodeStartTimes.clear();
+    frameInfos.clear();
     Logging.d(TAG, "Release on output thread done");
   }
 
@@ -492,14 +504,15 @@ class HardwareVideoDecoder implements VideoDecoder {
     copyPlane(src, vPos, uvStride, frameBuffer.getDataV(), 0, frameBuffer.getStrideV(), chromaWidth,
         chromaHeight);
 
-    // If the sliceHeight is odd, duplicate the last rows of chroma.
+    // If the sliceHeight is odd, duplicate the last rows of chroma.  Copy the last row of the U and
+    // V channels and append them at the end of each channel.
     if (sliceHeight % 2 != 0) {
       int strideU = frameBuffer.getStrideU();
-      uPos = chromaHeight * strideU;
+      int endU = chromaHeight * strideU;
-      copyRow(frameBuffer.getDataU(), uPos - strideU, frameBuffer.getDataU(), uPos, chromaWidth);
+      copyRow(frameBuffer.getDataU(), endU - strideU, frameBuffer.getDataU(), endU, chromaWidth);
       int strideV = frameBuffer.getStrideV();
-      vPos = chromaHeight * strideV;
+      int endV = chromaHeight * strideV;
-      copyRow(frameBuffer.getDataV(), vPos - strideV, frameBuffer.getDataV(), vPos, chromaWidth);
+      copyRow(frameBuffer.getDataV(), endV - strideV, frameBuffer.getDataV(), endV, chromaWidth);
     }
   }