From 0b06552ab332ca5e1473b6193abd90d6527ebd21 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Fri, 18 Feb 2022 09:52:11 +0900 Subject: [PATCH] Android: Respect input buffer layout of MediaFormat On Android, MediaCodec can request a specific layout of the input buffer. One can use the stride and slice height to calculate the layout from the Encoder's MediaFormat. The current code assumes a specific layout, which is a problematic in Android 12. Fix this by honoring the stride and slice-height. Bug: webrtc:13427 Change-Id: I2d3e429309e3add3ae668e0390460b51e6a49eb9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/240680 Reviewed-by: Niels Moller Reviewed-by: Harald Alvestrand Commit-Queue: Daniel.L (Byoungchan) Lee Cr-Commit-Position: refs/heads/main@{#36033} --- sdk/android/api/org/webrtc/YuvHelper.java | 123 +++++++++++++----- .../src/org/webrtc/YuvHelperTest.java | 35 +++++ .../java/org/webrtc/HardwareVideoEncoder.java | 52 +++++++- .../java/org/webrtc/MediaCodecWrapper.java | 2 + .../webrtc/MediaCodecWrapperFactoryImpl.java | 5 + .../org/webrtc/AndroidVideoDecoderTest.java | 3 +- .../src/org/webrtc/FakeMediaCodecWrapper.java | 9 +- .../org/webrtc/HardwareVideoEncoderTest.java | 3 +- 8 files changed, 187 insertions(+), 45 deletions(-) diff --git a/sdk/android/api/org/webrtc/YuvHelper.java b/sdk/android/api/org/webrtc/YuvHelper.java index 83bd7dcbb4..afb8e837d1 100644 --- a/sdk/android/api/org/webrtc/YuvHelper.java +++ b/sdk/android/api/org/webrtc/YuvHelper.java @@ -14,55 +14,93 @@ 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. */ + /** + * Copy I420 Buffer to a contiguously allocated buffer. + *

In Android, MediaCodec can request a buffer of a specific layout with the stride and + * slice-height (or plane height), and this function is used in this case. + *

For more information, see + * https://cs.android.com/android/platform/superproject/+/64fea7e5726daebc40f46890100837c01091100d:frameworks/base/media/java/android/media/MediaFormat.java;l=568 + * @param dstStrideY the stride of output buffers' Y plane. + * @param dstSliceHeightY the slice-height of output buffer's Y plane. + * @param dstStrideU the stride of output buffers' U (and V) plane. + * @param dstSliceHeightU the slice-height of output buffer's U (and V) plane + */ 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; + ByteBuffer srcV, int srcStrideV, ByteBuffer dst, int dstWidth, int dstHeight, int dstStrideY, + int dstSliceHeightY, int dstStrideU, int dstSliceHeightU) { + final int chromaWidth = (dstWidth + 1) / 2; + final int chromaHeight = (dstHeight + 1) / 2; - final int minSize = width * height + chromaWidth * chromaHeight * 2; - if (dst.capacity() < minSize) { + final int dstStartY = 0; + final int dstEndY = dstStartY + dstStrideY * dstHeight; + final int dstStartU = dstStartY + dstStrideY * dstSliceHeightY; + final int dstEndU = dstStartU + dstStrideU * chromaHeight; + final int dstStartV = dstStartU + dstStrideU * dstSliceHeightU; + // The last line doesn't need any padding, so use chromaWidth + // to calculate the exact end position. + final int dstEndV = dstStartV + dstStrideU * (chromaHeight - 1) + chromaWidth; + if (dst.capacity() < dstEndV) { throw new IllegalArgumentException("Expected destination buffer capacity to be at least " - + minSize + " was " + dst.capacity()); + + dstEndV + " was " + dst.capacity()); } - final int startY = 0; - final int startU = height * width; - final int startV = startU + chromaHeight * chromaWidth; - - dst.position(startY); + dst.limit(dstEndY); + dst.position(dstStartY); final ByteBuffer dstY = dst.slice(); - dst.position(startU); + dst.limit(dstEndU); + dst.position(dstStartU); final ByteBuffer dstU = dst.slice(); - dst.position(startV); + dst.limit(dstEndV); + dst.position(dstStartV); final ByteBuffer dstV = dst.slice(); - nativeI420Copy(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, width, dstU, - chromaWidth, dstV, chromaWidth, width, height); + I420Copy(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, dstStrideY, dstU, + dstStrideU, dstV, dstStrideU, dstWidth, dstHeight); + } + + /** 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 dstWidth, int dstHeight) { + I420Copy(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dst, dstWidth, dstHeight, + dstWidth, dstHeight, (dstWidth + 1) / 2, (dstHeight + 1) / 2); + } + + /** + * Copy I420 Buffer to a contiguously allocated buffer. + * @param dstStrideY the stride of output buffers' Y plane. + * @param dstSliceHeightY the slice-height of output buffer's Y plane. + */ + public static void I420ToNV12(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU, int srcStrideU, + ByteBuffer srcV, int srcStrideV, ByteBuffer dst, int dstWidth, int dstHeight, int dstStrideY, + int dstSliceHeightY) { + final int chromaHeight = (dstHeight + 1) / 2; + final int chromaWidth = (dstWidth + 1) / 2; + + final int dstStartY = 0; + final int dstEndY = dstStartY + dstStrideY * dstHeight; + final int dstStartUV = dstStartY + dstStrideY * dstSliceHeightY; + final int dstEndUV = dstStartUV + chromaWidth * chromaHeight * 2; + if (dst.capacity() < dstEndUV) { + throw new IllegalArgumentException("Expected destination buffer capacity to be at least " + + dstEndUV + " was " + dst.capacity()); + } + + dst.limit(dstEndY); + dst.position(dstStartY); + final ByteBuffer dstY = dst.slice(); + dst.limit(dstEndUV); + dst.position(dstStartUV); + final ByteBuffer dstUV = dst.slice(); + + I420ToNV12(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, dstStrideY, dstUV, + chromaWidth * 2, dstWidth, dstHeight); } /** 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(); - - nativeI420ToNV12(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, width, dstUV, - chromaWidth * 2, width, height); + ByteBuffer srcV, int srcStrideV, ByteBuffer dst, int dstWidth, int dstHeight) { + I420ToNV12(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dst, dstWidth, dstHeight, + dstWidth, dstHeight); } /** Helper method for rotating I420 to tightly packed destination buffer. */ @@ -109,9 +147,18 @@ public class YuvHelper { src, srcStride, dstY, dstStrideY, dstU, dstStrideU, dstV, dstStrideV, width, height); } + /** + * Copies I420 to the I420 dst buffer. + *

Unlike `libyuv::I420Copy`, this function checks if the height <= 0, so flipping is not + * supported. + */ public static 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) { + if (srcY == null || srcU == null || srcV == null || dstY == null || dstU == null || dstV == null + || width <= 0 || height <= 0) { + throw new IllegalArgumentException("Invalid I420Copy input arguments"); + } nativeI420Copy(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, dstStrideY, dstU, dstStrideU, dstV, dstStrideV, width, height); } @@ -119,6 +166,10 @@ public class YuvHelper { public static 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) { + if (srcY == null || srcU == null || srcV == null || dstY == null || dstUV == null || width <= 0 + || height <= 0) { + throw new IllegalArgumentException("Invalid I420ToNV12 input arguments"); + } nativeI420ToNV12(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, dstStrideY, dstUV, dstStrideUV, width, height); } diff --git a/sdk/android/instrumentationtests/src/org/webrtc/YuvHelperTest.java b/sdk/android/instrumentationtests/src/org/webrtc/YuvHelperTest.java index 693bdd4a79..0a63bd7634 100644 --- a/sdk/android/instrumentationtests/src/org/webrtc/YuvHelperTest.java +++ b/sdk/android/instrumentationtests/src/org/webrtc/YuvHelperTest.java @@ -101,6 +101,25 @@ public class YuvHelperTest { new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 51, 52, 53, 54, 101, 102, 105, 106}, dst); } + @SmallTest + @Test + public void testI420CopyStride() { + final int dstStrideY = 4; + final int dstSliceHeightY = 4; + final int dstStrideU = dstStrideY / 2; + final int dstSliceHeightU = dstSliceHeightY / 2; + final int dstSize = dstStrideY * dstStrideY * 3 / 2; + + final ByteBuffer dst = ByteBuffer.allocateDirect(dstSize); + 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, dstStrideY, dstSliceHeightY, + dstStrideU, dstSliceHeightU); + + assertByteBufferContentEquals(new byte[] {1, 2, 3, 0, 4, 5, 6, 0, 7, 8, 9, 0, 0, 0, 0, 0, 51, + 52, 53, 54, 101, 102, 105, 106}, + dst); + } + @SmallTest @Test public void testI420ToNV12() { @@ -132,6 +151,22 @@ public class YuvHelperTest { new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 51, 101, 52, 102, 53, 105, 54, 106}, dst); } + @SmallTest + @Test + public void testI420ToNV12Stride() { + final int dstStrideY = 4; + final int dstSliceHeightY = 4; + final int dstSize = dstStrideY * dstStrideY * 3 / 2; + + final ByteBuffer dst = ByteBuffer.allocateDirect(dstSize); + 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, dstStrideY, dstSliceHeightY); + + assertByteBufferContentEquals(new byte[] {1, 2, 3, 0, 4, 5, 6, 0, 7, 8, 9, 0, 0, 0, 0, 0, 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(), diff --git a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java index e5062629c7..0825c1e742 100644 --- a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java +++ b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java @@ -15,6 +15,7 @@ import android.media.MediaCodec; import android.media.MediaCodecInfo; import android.media.MediaFormat; import android.opengl.GLES20; +import android.os.Build; import android.os.Bundle; import android.view.Surface; import androidx.annotation.Nullable; @@ -149,6 +150,10 @@ class HardwareVideoEncoder implements VideoEncoder { private int width; private int height; + // Y-plane strides in the encoder's input + private int stride; + // Y-plane slice-height in the encoder's input + private int sliceHeight; private boolean useSurfaceMode; // --- Only accessed from the encoding thread. @@ -280,6 +285,10 @@ class HardwareVideoEncoder implements VideoEncoder { textureEglBase.makeCurrent(); } + MediaFormat inputFormat = codec.getInputFormat(); + stride = getStride(inputFormat, width); + sliceHeight = getSliceHeight(inputFormat, height); + codec.start(); outputBuffers = codec.getOutputBuffers(); } catch (IllegalStateException e) { @@ -686,9 +695,25 @@ class HardwareVideoEncoder implements VideoEncoder { return sharedContext != null && surfaceColorFormat != null; } + private static int getStride(MediaFormat inputFormat, int width) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && inputFormat != null + && inputFormat.containsKey(MediaFormat.KEY_STRIDE)) { + return inputFormat.getInteger(MediaFormat.KEY_STRIDE); + } + return width; + } + + private static int getSliceHeight(MediaFormat inputFormat, int height) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && inputFormat != null + && inputFormat.containsKey(MediaFormat.KEY_SLICE_HEIGHT)) { + return inputFormat.getInteger(MediaFormat.KEY_SLICE_HEIGHT); + } + return height; + } + // Visible for testing. protected void fillInputBuffer(ByteBuffer buffer, VideoFrame.Buffer videoFrameBuffer) { - yuvFormat.fillBuffer(buffer, videoFrameBuffer); + yuvFormat.fillBuffer(buffer, videoFrameBuffer, stride, sliceHeight); } /** @@ -697,24 +722,39 @@ class HardwareVideoEncoder implements VideoEncoder { private enum YuvFormat { I420 { @Override - void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer) { + void fillBuffer( + ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer, int dstStrideY, int dstSliceHeightY) { + /* + * According to the docs in Android MediaCodec, the stride of the U and V planes can be + * calculated based on the color format, though it is generally undefined and depends on the + * device and release. + *

Assuming the width and height, dstStrideY and dstSliceHeightY are + * even, it works fine when we define the stride and slice-height of the dst U/V plane to be + * half of the dst Y plane. + */ + int dstStrideU = dstStrideY / 2; + int dstSliceHeight = dstSliceHeightY / 2; 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.getDataV(), i420.getStrideV(), dstBuffer, i420.getWidth(), i420.getHeight(), + dstStrideY, dstSliceHeightY, dstStrideU, dstSliceHeight); i420.release(); } }, NV12 { @Override - void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer) { + void fillBuffer( + ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer, int dstStrideY, int dstSliceHeightY) { 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.getDataV(), i420.getStrideV(), dstBuffer, i420.getWidth(), i420.getHeight(), + dstStrideY, dstSliceHeightY); i420.release(); } }; - abstract void fillBuffer(ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer); + abstract void fillBuffer( + ByteBuffer dstBuffer, VideoFrame.Buffer srcBuffer, int dstStrideY, int dstSliceHeightY); static YuvFormat valueOf(int colorFormat) { switch (colorFormat) { diff --git a/sdk/android/src/java/org/webrtc/MediaCodecWrapper.java b/sdk/android/src/java/org/webrtc/MediaCodecWrapper.java index bb67d1f4b9..89f392d3bd 100644 --- a/sdk/android/src/java/org/webrtc/MediaCodecWrapper.java +++ b/sdk/android/src/java/org/webrtc/MediaCodecWrapper.java @@ -41,6 +41,8 @@ interface MediaCodecWrapper { void releaseOutputBuffer(int index, boolean render); + MediaFormat getInputFormat(); + MediaFormat getOutputFormat(); ByteBuffer[] getInputBuffers(); diff --git a/sdk/android/src/java/org/webrtc/MediaCodecWrapperFactoryImpl.java b/sdk/android/src/java/org/webrtc/MediaCodecWrapperFactoryImpl.java index 544d6ebe4f..29edf6ef0c 100644 --- a/sdk/android/src/java/org/webrtc/MediaCodecWrapperFactoryImpl.java +++ b/sdk/android/src/java/org/webrtc/MediaCodecWrapperFactoryImpl.java @@ -78,6 +78,11 @@ class MediaCodecWrapperFactoryImpl implements MediaCodecWrapperFactory { mediaCodec.releaseOutputBuffer(index, render); } + @Override + public MediaFormat getInputFormat() { + return mediaCodec.getInputFormat(); + } + @Override public MediaFormat getOutputFormat() { return mediaCodec.getOutputFormat(); diff --git a/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java b/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java index 644b24b1b3..04891e2563 100644 --- a/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java +++ b/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java @@ -201,9 +201,10 @@ public class AndroidVideoDecoderTest { MockitoAnnotations.initMocks(this); when(mockSurfaceTextureHelper.getSurfaceTexture()) .thenReturn(new SurfaceTexture(/*texName=*/0)); + MediaFormat inputFormat = new MediaFormat(); MediaFormat outputFormat = new MediaFormat(); // TODO(sakal): Add more details to output format as needed. - fakeMediaCodecWrapper = spy(new FakeMediaCodecWrapper(outputFormat)); + fakeMediaCodecWrapper = spy(new FakeMediaCodecWrapper(inputFormat, outputFormat)); fakeDecoderCallback = new FakeDecoderCallback(); } diff --git a/sdk/android/tests/src/org/webrtc/FakeMediaCodecWrapper.java b/sdk/android/tests/src/org/webrtc/FakeMediaCodecWrapper.java index 990f7b3aca..c5e69498e0 100644 --- a/sdk/android/tests/src/org/webrtc/FakeMediaCodecWrapper.java +++ b/sdk/android/tests/src/org/webrtc/FakeMediaCodecWrapper.java @@ -104,6 +104,7 @@ public class FakeMediaCodecWrapper implements MediaCodecWrapper { private State state = State.STOPPED_UNINITIALIZED; private @Nullable MediaFormat configuredFormat; private int configuredFlags; + private final MediaFormat inputFormat; private final MediaFormat outputFormat; private final ByteBuffer[] inputBuffers = new ByteBuffer[NUM_INPUT_BUFFERS]; private final ByteBuffer[] outputBuffers = new ByteBuffer[NUM_OUTPUT_BUFFERS]; @@ -111,7 +112,8 @@ public class FakeMediaCodecWrapper implements MediaCodecWrapper { private final boolean[] outputBufferReserved = new boolean[NUM_OUTPUT_BUFFERS]; private final List queuedOutputBuffers = new ArrayList<>(); - public FakeMediaCodecWrapper(MediaFormat outputFormat) { + public FakeMediaCodecWrapper(MediaFormat inputFormat, MediaFormat outputFormat) { + this.inputFormat = inputFormat; this.outputFormat = outputFormat; } @@ -299,6 +301,11 @@ public class FakeMediaCodecWrapper implements MediaCodecWrapper { return outputBuffers; } + @Override + public MediaFormat getInputFormat() { + return inputFormat; + } + @Override public MediaFormat getOutputFormat() { return outputFormat; diff --git a/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java b/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java index 145c0cc29a..08f7fb055f 100644 --- a/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java +++ b/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java @@ -161,9 +161,10 @@ public class HardwareVideoEncoderTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); + MediaFormat inputFormat = new MediaFormat(); MediaFormat outputFormat = new MediaFormat(); // TODO(sakal): Add more details to output format as needed. - fakeMediaCodecWrapper = spy(new FakeMediaCodecWrapper(outputFormat)); + fakeMediaCodecWrapper = spy(new FakeMediaCodecWrapper(inputFormat, outputFormat)); } @Test