Fix and optimize input buffer filling in HardwareVideoEncoder.

Previously input buffers would be filled incorrectly for sparsely
packed buffers where stride is not equal to the plane width.

Bug: webrtc:8478
Change-Id: I080fa3c354a27982bb996be8c1e41b103384e4bc
Reviewed-on: https://webrtc-review.googlesource.com/17321
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20550}

sdk/android/BUILD.gn

@@ -131,6 +131,7 @@ rtc_static_library("video_jni") {
     "src/jni/videotrack_jni.cc",
     "src/jni/wrapped_native_i420_buffer.cc",
     "src/jni/wrapped_native_i420_buffer.h",
+    "src/jni/yuvhelper.cc",
   ]
 
   configs += [ ":libjingle_peerconnection_jni_warnings_config" ]
@@ -386,9 +387,9 @@ dist_jar("libwebrtc") {
 
 android_library("libjingle_peerconnection_java") {
   java_files = [
+    "api/org/webrtc/AudioProcessingFactory.java",
     "api/org/webrtc/AudioSource.java",
     "api/org/webrtc/AudioTrack.java",
-    "api/org/webrtc/AudioProcessingFactory.java",
     "api/org/webrtc/CallSessionFileRotatingLogSink.java",
     "api/org/webrtc/Camera1Capturer.java",
     "api/org/webrtc/Camera1Enumerator.java",
@@ -454,6 +455,7 @@ android_library("libjingle_peerconnection_java") {
     "api/org/webrtc/VideoSource.java",
     "api/org/webrtc/VideoTrack.java",
     "api/org/webrtc/YuvConverter.java",
+    "api/org/webrtc/YuvHelper.java",
     "src/java/org/webrtc/AndroidVideoTrackSourceObserver.java",
     "src/java/org/webrtc/BaseBitrateAdjuster.java",
     "src/java/org/webrtc/BitrateAdjuster.java",
@@ -507,16 +509,16 @@ if (rtc_include_tests) {
     android_manifest = "instrumentationtests/AndroidManifest.xml"
 
     java_files = [
-      "instrumentationtests/src/org/webrtc/DefaultAudioProcessingFactoryTest.java",
       "instrumentationtests/src/org/webrtc/Camera1CapturerUsingByteBufferTest.java",
       "instrumentationtests/src/org/webrtc/Camera1CapturerUsingTextureTest.java",
       "instrumentationtests/src/org/webrtc/Camera2CapturerTest.java",
       "instrumentationtests/src/org/webrtc/CameraVideoCapturerTestFixtures.java",
+      "instrumentationtests/src/org/webrtc/DefaultAudioProcessingFactoryTest.java",
       "instrumentationtests/src/org/webrtc/EglRendererTest.java",
       "instrumentationtests/src/org/webrtc/FileVideoCapturerTest.java",
       "instrumentationtests/src/org/webrtc/GlRectDrawerTest.java",
-      "instrumentationtests/src/org/webrtc/HardwareVideoEncoderTest.java",
       "instrumentationtests/src/org/webrtc/HardwareVideoDecoderTest.java",
+      "instrumentationtests/src/org/webrtc/HardwareVideoEncoderTest.java",
       "instrumentationtests/src/org/webrtc/MediaCodecVideoEncoderTest.java",
       "instrumentationtests/src/org/webrtc/NetworkMonitorTest.java",
       "instrumentationtests/src/org/webrtc/PeerConnectionTest.java",
@@ -525,6 +527,7 @@ if (rtc_include_tests) {
       "instrumentationtests/src/org/webrtc/SurfaceViewRendererOnMeasureTest.java",
       "instrumentationtests/src/org/webrtc/VideoFileRendererTest.java",
       "instrumentationtests/src/org/webrtc/WebRtcJniBootTest.java",
+      "instrumentationtests/src/org/webrtc/YuvHelperTest.java",
     ]
 
     data = [

sdk/android/api/org/webrtc/YuvHelper.java

@@ -0,0 +1,74 @@
+/*
+ *  Copyright 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+package org.webrtc;
+
+import java.nio.ByteBuffer;
+
+/** Wraps libyuv methods to Java. All passed byte buffers must be direct byte buffers. */
+public class YuvHelper {
+  /** Helper method for copying I420 to tightly packed destination buffer. */
+  public static void I420Copy(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU, int srcStrideU,
+      ByteBuffer srcV, int srcStrideV, ByteBuffer dst, int width, int height) {
+    final int chromaHeight = (height + 1) / 2;
+    final int chromaWidth = (width + 1) / 2;
+
+    final int minSize = width * height + chromaWidth * chromaHeight * 2;
+    if (dst.capacity() < minSize) {
+      throw new IllegalArgumentException("Expected destination buffer capacity to be at least "
+          + minSize + " was " + dst.capacity());
+    }
+
+    final int startY = 0;
+    final int startU = height * width;
+    final int startV = startU + chromaHeight * chromaWidth;
+
+    dst.position(startY);
+    final ByteBuffer dstY = dst.slice();
+    dst.position(startU);
+    final ByteBuffer dstU = dst.slice();
+    dst.position(startV);
+    final ByteBuffer dstV = dst.slice();
+
+    I420Copy(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, width, dstU, chromaWidth,
+        dstV, chromaWidth, width, height);
+  }
+
+  /** Helper method for copying I420 to tightly packed NV12 destination buffer. */
+  public static void I420ToNV12(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU, int srcStrideU,
+      ByteBuffer srcV, int srcStrideV, ByteBuffer dst, int width, int height) {
+    final int chromaWidth = (width + 1) / 2;
+    final int chromaHeight = (height + 1) / 2;
+
+    final int minSize = width * height + chromaWidth * chromaHeight * 2;
+    if (dst.capacity() < minSize) {
+      throw new IllegalArgumentException("Expected destination buffer capacity to be at least "
+          + minSize + " was " + dst.capacity());
+    }
+
+    final int startY = 0;
+    final int startUV = height * width;
+
+    dst.position(startY);
+    final ByteBuffer dstY = dst.slice();
+    dst.position(startUV);
+    final ByteBuffer dstUV = dst.slice();
+
+    I420ToNV12(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, width, dstUV,
+        chromaWidth * 2, width, height);
+  }
+
+  public static native void I420Copy(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU,
+      int srcStrideU, ByteBuffer srcV, int srcStrideV, ByteBuffer dstY, int dstStrideY,
+      ByteBuffer dstU, int dstStrideU, ByteBuffer dstV, int dstStrideV, int width, int height);
+  public static native void I420ToNV12(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU,
+      int srcStrideU, ByteBuffer srcV, int srcStrideV, ByteBuffer dstY, int dstStrideY,
+      ByteBuffer dstUV, int dstStrideUV, int width, int height);
+}

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

@@ -148,6 +148,8 @@ public final class HardwareVideoDecoderTest {
 
   @Before
   public void setUp() {
+    NativeLibrary.initialize(new NativeLibrary.DefaultLoader());
+
     eglBase = new EglBase14(null, EglBase.CONFIG_PLAIN);
     eglBase.createDummyPbufferSurface();
     eglBase.makeCurrent();

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

@@ -328,6 +328,8 @@ public class HardwareVideoEncoderTest {
   // # Tests
   @Before
   public void setUp() {
+    NativeLibrary.initialize(new NativeLibrary.DefaultLoader());
+
     eglBase = new EglBase14(null, EglBase.CONFIG_PLAIN);
     eglBase.createDummyPbufferSurface();
     eglBase.makeCurrent();

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

@@ -0,0 +1,132 @@
+/*
+ *  Copyright 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+package org.webrtc;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import android.support.test.filters.SmallTest;
+import java.nio.ByteBuffer;
+import org.chromium.base.test.BaseJUnit4ClassRunner;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(BaseJUnit4ClassRunner.class)
+public class YuvHelperTest {
+  private static final int TEST_WIDTH = 3;
+  private static final int TEST_HEIGHT = 3;
+  private static final int TEST_CHROMA_WIDTH = 2;
+  private static final int TEST_CHROMA_HEIGHT = 2;
+
+  private static final int TEST_I420_STRIDE_Y = 3;
+  private static final int TEST_I420_STRIDE_V = 2;
+  private static final int TEST_I420_STRIDE_U = 4;
+
+  private static final ByteBuffer TEST_I420_Y = getTestY();
+  private static final ByteBuffer TEST_I420_U = getTestU();
+  private static final ByteBuffer TEST_I420_V = getTestV();
+
+  private static ByteBuffer getTestY() {
+    final ByteBuffer testY = ByteBuffer.allocateDirect(TEST_HEIGHT * TEST_I420_STRIDE_Y);
+    testY.put(new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9});
+    return testY;
+  }
+
+  private static ByteBuffer getTestU() {
+    final ByteBuffer testU = ByteBuffer.allocateDirect(TEST_CHROMA_HEIGHT * TEST_I420_STRIDE_V);
+    testU.put(new byte[] {51, 52, 53, 54});
+    return testU;
+  }
+
+  private static ByteBuffer getTestV() {
+    final ByteBuffer testV = ByteBuffer.allocateDirect(TEST_CHROMA_HEIGHT * TEST_I420_STRIDE_U);
+    testV.put(new byte[] {101, 102, 103, 104, 105, 106, 107, 108});
+    return testV;
+  }
+
+  @Before
+  public void setUp() {
+    NativeLibrary.initialize(new NativeLibrary.DefaultLoader());
+  }
+
+  @SmallTest
+  @Test
+  public void testI420Copy() {
+    final int dstStrideY = TEST_WIDTH;
+    final int dstStrideU = TEST_CHROMA_WIDTH;
+    final int dstStrideV = TEST_CHROMA_WIDTH;
+    final ByteBuffer dstY = ByteBuffer.allocateDirect(TEST_HEIGHT * dstStrideY);
+    final ByteBuffer dstU = ByteBuffer.allocateDirect(TEST_CHROMA_HEIGHT * dstStrideU);
+    final ByteBuffer dstV = ByteBuffer.allocateDirect(TEST_CHROMA_HEIGHT * dstStrideV);
+
+    YuvHelper.I420Copy(TEST_I420_Y, TEST_I420_STRIDE_Y, TEST_I420_U, TEST_I420_STRIDE_V,
+        TEST_I420_V, TEST_I420_STRIDE_U, dstY, dstStrideY, dstU, dstStrideU, dstV, dstStrideV,
+        TEST_WIDTH, TEST_HEIGHT);
+
+    assertByteBufferContentEquals(new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9}, dstY);
+    assertByteBufferContentEquals(new byte[] {51, 52, 53, 54}, dstU);
+    assertByteBufferContentEquals(new byte[] {101, 102, 105, 106}, dstV);
+  }
+
+  @SmallTest
+  @Test
+  public void testI420CopyTight() {
+    final ByteBuffer dst = ByteBuffer.allocateDirect(
+        TEST_WIDTH * TEST_HEIGHT + TEST_CHROMA_WIDTH * TEST_CHROMA_HEIGHT * 2);
+
+    YuvHelper.I420Copy(TEST_I420_Y, TEST_I420_STRIDE_Y, TEST_I420_U, TEST_I420_STRIDE_V,
+        TEST_I420_V, TEST_I420_STRIDE_U, dst, TEST_WIDTH, TEST_HEIGHT);
+
+    assertByteBufferContentEquals(
+        new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 51, 52, 53, 54, 101, 102, 105, 106}, dst);
+  }
+
+  @SmallTest
+  @Test
+  public void testI420ToNV12() {
+    final int dstStrideY = TEST_WIDTH;
+    final int dstStrideUV = TEST_CHROMA_WIDTH * 2;
+    final ByteBuffer dstY = ByteBuffer.allocateDirect(TEST_HEIGHT * dstStrideY);
+    final ByteBuffer dstUV = ByteBuffer.allocateDirect(2 * TEST_CHROMA_HEIGHT * dstStrideUV);
+
+    YuvHelper.I420ToNV12(TEST_I420_Y, TEST_I420_STRIDE_Y, TEST_I420_U, TEST_I420_STRIDE_V,
+        TEST_I420_V, TEST_I420_STRIDE_U, dstY, dstStrideY, dstUV, dstStrideUV, TEST_WIDTH,
+        TEST_HEIGHT);
+
+    assertByteBufferContentEquals(new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9}, dstY);
+    assertByteBufferContentEquals(new byte[] {51, 101, 52, 102, 53, 105, 54, 106}, dstUV);
+  }
+
+  @SmallTest
+  @Test
+  public void testI420ToNV12Tight() {
+    final int dstStrideY = TEST_WIDTH;
+    final int dstStrideUV = TEST_CHROMA_WIDTH * 2;
+    final ByteBuffer dst = ByteBuffer.allocateDirect(
+        TEST_WIDTH * TEST_HEIGHT + TEST_CHROMA_WIDTH * TEST_CHROMA_HEIGHT * 2);
+
+    YuvHelper.I420ToNV12(TEST_I420_Y, TEST_I420_STRIDE_Y, TEST_I420_U, TEST_I420_STRIDE_V,
+        TEST_I420_V, TEST_I420_STRIDE_U, dst, TEST_WIDTH, TEST_HEIGHT);
+
+    assertByteBufferContentEquals(
+        new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 51, 101, 52, 102, 53, 105, 54, 106}, dst);
+  }
+
+  private static void assertByteBufferContentEquals(byte[] expected, ByteBuffer test) {
+    assertTrue(
+        "ByteBuffer is too small. Expected " + expected.length + " but was " + test.capacity(),
+        test.capacity() >= expected.length);
+    for (int i = 0; i < expected.length; i++) {
+      assertEquals("Unexpected ByteBuffer contents at index: " + i, expected[i], test.get(i));
+    }
+  }
+}

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

@@ -567,36 +567,27 @@ class HardwareVideoEncoder implements VideoEncoder {
   /**
    * Enumeration of supported YUV color formats used for MediaCodec's input.
    */
-  private static enum YuvFormat {
+  private enum YuvFormat {
     I420 {
       @Override
-      void fillBuffer(ByteBuffer inputBuffer, VideoFrame.Buffer buffer) {
+      void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer) {
-        VideoFrame.I420Buffer i420 = buffer.toI420();
+        VideoFrame.I420Buffer i420 = srcBuffer.toI420();
-        inputBuffer.put(i420.getDataY());
+        YuvHelper.I420Copy(i420.getDataY(), i420.getStrideY(), i420.getDataU(), i420.getStrideU(),
-        inputBuffer.put(i420.getDataU());
+            i420.getDataV(), i420.getStrideV(), dstBuffer, i420.getWidth(), i420.getHeight());
-        inputBuffer.put(i420.getDataV());
         i420.release();
       }
     },
     NV12 {
       @Override
-      void fillBuffer(ByteBuffer inputBuffer, VideoFrame.Buffer buffer) {
+      void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer) {
-        VideoFrame.I420Buffer i420 = buffer.toI420();
+        VideoFrame.I420Buffer i420 = srcBuffer.toI420();
-        inputBuffer.put(i420.getDataY());
+        YuvHelper.I420ToNV12(i420.getDataY(), i420.getStrideY(), i420.getDataU(), i420.getStrideU(),
-
+            i420.getDataV(), i420.getStrideV(), dstBuffer, i420.getWidth(), i420.getHeight());
-        // Interleave the bytes from the U and V portions, starting with U.
-        ByteBuffer u = i420.getDataU();
-        ByteBuffer v = i420.getDataV();
-        int i = 0;
-        while (u.hasRemaining() && v.hasRemaining()) {
-          inputBuffer.put(u.get());
-          inputBuffer.put(v.get());
-        }
         i420.release();
       }
     };
 
-    abstract void fillBuffer(ByteBuffer inputBuffer, VideoFrame.Buffer buffer);
+    abstract void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer);
 
     static YuvFormat valueOf(int colorFormat) {
       switch (colorFormat) {

sdk/android/src/jni/yuvhelper.cc

@@ -0,0 +1,84 @@
+/*
+ *  Copyright 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <jni.h>
+
+#include "sdk/android/src/jni/jni_helpers.h"
+#include "third_party/libyuv/include/libyuv/convert.h"
+
+namespace webrtc {
+namespace jni {
+
+JNI_FUNCTION_DECLARATION(void,
+                         YuvHelper_I420Copy,
+                         JNIEnv* jni,
+                         jclass,
+                         jobject j_src_y,
+                         jint src_stride_y,
+                         jobject j_src_u,
+                         jint src_stride_u,
+                         jobject j_src_v,
+                         jint src_stride_v,
+                         jobject j_dst_y,
+                         jint dst_stride_y,
+                         jobject j_dst_u,
+                         jint dst_stride_u,
+                         jobject j_dst_v,
+                         jint dst_stride_v,
+                         jint width,
+                         jint height) {
+  const uint8_t* src_y =
+      static_cast<const uint8_t*>(jni->GetDirectBufferAddress(j_src_y));
+  const uint8_t* src_u =
+      static_cast<const uint8_t*>(jni->GetDirectBufferAddress(j_src_u));
+  const uint8_t* src_v =
+      static_cast<const uint8_t*>(jni->GetDirectBufferAddress(j_src_v));
+  uint8_t* dst_y = static_cast<uint8_t*>(jni->GetDirectBufferAddress(j_dst_y));
+  uint8_t* dst_u = static_cast<uint8_t*>(jni->GetDirectBufferAddress(j_dst_u));
+  uint8_t* dst_v = static_cast<uint8_t*>(jni->GetDirectBufferAddress(j_dst_v));
+
+  libyuv::I420Copy(src_y, src_stride_y, src_u, src_stride_u, src_v,
+                   src_stride_v, dst_y, dst_stride_y, dst_u, dst_stride_u,
+                   dst_v, dst_stride_v, width, height);
+}
+
+JNI_FUNCTION_DECLARATION(void,
+                         YuvHelper_I420ToNV12,
+                         JNIEnv* jni,
+                         jclass,
+                         jobject j_src_y,
+                         jint src_stride_y,
+                         jobject j_src_u,
+                         jint src_stride_u,
+                         jobject j_src_v,
+                         jint src_stride_v,
+                         jobject j_dst_y,
+                         jint dst_stride_y,
+                         jobject j_dst_uv,
+                         jint dst_stride_uv,
+                         jint width,
+                         jint height) {
+  const uint8_t* src_y =
+      static_cast<const uint8_t*>(jni->GetDirectBufferAddress(j_src_y));
+  const uint8_t* src_u =
+      static_cast<const uint8_t*>(jni->GetDirectBufferAddress(j_src_u));
+  const uint8_t* src_v =
+      static_cast<const uint8_t*>(jni->GetDirectBufferAddress(j_src_v));
+  uint8_t* dst_y = static_cast<uint8_t*>(jni->GetDirectBufferAddress(j_dst_y));
+  uint8_t* dst_uv =
+      static_cast<uint8_t*>(jni->GetDirectBufferAddress(j_dst_uv));
+
+  libyuv::I420ToNV12(src_y, src_stride_y, src_u, src_stride_u, src_v,
+                     src_stride_v, dst_y, dst_stride_y, dst_uv, dst_stride_uv,
+                     width, height);
+}
+
+}  // namespace jni
+}  // namespace webrtc