From f6515cd0e3a11d3893242cbc8313b0fc902aedcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20Kalliom=C3=A4ki?= Date: Thu, 2 Nov 2017 11:25:58 +0100 Subject: [PATCH] Fix and optimize input buffer filling in HardwareVideoEncoder. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Commit-Queue: Sami Kalliomäki Cr-Commit-Position: refs/heads/master@{#20550} --- sdk/android/BUILD.gn | 9 +- sdk/android/api/org/webrtc/YuvHelper.java | 74 ++++++++++ .../org/webrtc/HardwareVideoDecoderTest.java | 2 + .../org/webrtc/HardwareVideoEncoderTest.java | 2 + .../src/org/webrtc/YuvHelperTest.java | 132 ++++++++++++++++++ .../java/org/webrtc/HardwareVideoEncoder.java | 29 ++-- sdk/android/src/jni/yuvhelper.cc | 84 +++++++++++ 7 files changed, 310 insertions(+), 22 deletions(-) create mode 100644 sdk/android/api/org/webrtc/YuvHelper.java create mode 100644 sdk/android/instrumentationtests/src/org/webrtc/YuvHelperTest.java create mode 100644 sdk/android/src/jni/yuvhelper.cc diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index 8d3d58787c..19eb00b637 100644 --- a/sdk/android/BUILD.gn +++ b/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 = [ diff --git a/sdk/android/api/org/webrtc/YuvHelper.java b/sdk/android/api/org/webrtc/YuvHelper.java new file mode 100644 index 0000000000..344d5d962a --- /dev/null +++ b/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); +} diff --git a/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoDecoderTest.java b/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoDecoderTest.java index 340d184cfe..4260397ecf 100644 --- a/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoDecoderTest.java +++ b/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(); diff --git a/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoEncoderTest.java b/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoEncoderTest.java index 49b601ec1c..5b3c04f90e 100644 --- a/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoEncoderTest.java +++ b/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(); diff --git a/sdk/android/instrumentationtests/src/org/webrtc/YuvHelperTest.java b/sdk/android/instrumentationtests/src/org/webrtc/YuvHelperTest.java new file mode 100644 index 0000000000..a6ded5cc7a --- /dev/null +++ b/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)); + } + } +} diff --git a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java index e0cd6b1ca6..61cf9a9887 100644 --- a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java +++ b/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) { - VideoFrame.I420Buffer i420 = buffer.toI420(); - inputBuffer.put(i420.getDataY()); - inputBuffer.put(i420.getDataU()); - inputBuffer.put(i420.getDataV()); + void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer) { + VideoFrame.I420Buffer i420 = srcBuffer.toI420(); + YuvHelper.I420Copy(i420.getDataY(), i420.getStrideY(), i420.getDataU(), i420.getStrideU(), + i420.getDataV(), i420.getStrideV(), dstBuffer, i420.getWidth(), i420.getHeight()); i420.release(); } }, NV12 { @Override - void fillBuffer(ByteBuffer inputBuffer, VideoFrame.Buffer buffer) { - VideoFrame.I420Buffer i420 = buffer.toI420(); - inputBuffer.put(i420.getDataY()); - - // 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()); - } + void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer) { + VideoFrame.I420Buffer i420 = srcBuffer.toI420(); + YuvHelper.I420ToNV12(i420.getDataY(), i420.getStrideY(), i420.getDataU(), i420.getStrideU(), + i420.getDataV(), i420.getStrideV(), dstBuffer, i420.getWidth(), i420.getHeight()); 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) { diff --git a/sdk/android/src/jni/yuvhelper.cc b/sdk/android/src/jni/yuvhelper.cc new file mode 100644 index 0000000000..bdf5375c4f --- /dev/null +++ b/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 + +#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(jni->GetDirectBufferAddress(j_src_y)); + const uint8_t* src_u = + static_cast(jni->GetDirectBufferAddress(j_src_u)); + const uint8_t* src_v = + static_cast(jni->GetDirectBufferAddress(j_src_v)); + uint8_t* dst_y = static_cast(jni->GetDirectBufferAddress(j_dst_y)); + uint8_t* dst_u = static_cast(jni->GetDirectBufferAddress(j_dst_u)); + uint8_t* dst_v = static_cast(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(jni->GetDirectBufferAddress(j_src_y)); + const uint8_t* src_u = + static_cast(jni->GetDirectBufferAddress(j_src_u)); + const uint8_t* src_v = + static_cast(jni->GetDirectBufferAddress(j_src_v)); + uint8_t* dst_y = static_cast(jni->GetDirectBufferAddress(j_dst_y)); + uint8_t* dst_uv = + static_cast(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