From f58afb066f4d5924a9688d52c34f57030d0c94a0 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Thu, 22 Oct 2026 18:51:55 +0200 Subject: [PATCH] fix: convert only the window between position and limit of a ByteBuffer VideoBufferConverter passes direct buffers as slices and heap buffers with offset and length, so slices and buffers with a position no longer read or write outside their window. The position stays as it is. --- docs/tools/video/video-buffer-converter.md | 1 + .../cpp/include/JNI_VideoBufferConverter.h | 8 +- .../main/cpp/src/JNI_VideoBufferConverter.cpp | 46 ++-- .../media/video/VideoBufferConverter.java | 58 ++--- .../media/video/VideoBufferConverterTest.java | 203 ++++++++++++++++++ 5 files changed, 276 insertions(+), 40 deletions(-) diff --git a/docs/tools/video/video-buffer-converter.md b/docs/tools/video/video-buffer-converter.md index bda9068a4..51d4837c7 100644 --- a/docs/tools/video/video-buffer-converter.md +++ b/docs/tools/video/video-buffer-converter.md @@ -135,6 +135,7 @@ public void onImage(BufferedImage image) { ## Error handling and edge cases - All methods throw `NullPointerException` if src/dst is null; ensure proper checks. - `ByteBuffer` destinations must be writable (not read-only) for `convertFromI420`. +- A `ByteBuffer` is read or written between its position and its limit only, also when it is a slice or a view of a larger buffer. The window must hold the whole frame; the position and limit stay as they are. - Ensure the correct FourCC is used for the actual memory layout you pass/expect. - Beware of frame rotation metadata; conversions do not rotate pixels. Handle `VideoFrame.rotation` separately if your renderer requires upright images. diff --git a/webrtc-jni/src/main/cpp/include/JNI_VideoBufferConverter.h b/webrtc-jni/src/main/cpp/include/JNI_VideoBufferConverter.h index 8373597a7..49ad06fa5 100644 --- a/webrtc-jni/src/main/cpp/include/JNI_VideoBufferConverter.h +++ b/webrtc-jni/src/main/cpp/include/JNI_VideoBufferConverter.h @@ -10,10 +10,10 @@ extern "C" { /* * Class: dev_onvoid_webrtc_media_video_VideoBufferConverter * Method: I420toByteArray - * Signature: (Ljava/nio/ByteBuffer;ILjava/nio/ByteBuffer;ILjava/nio/ByteBuffer;I[BIII)V + * Signature: (Ljava/nio/ByteBuffer;ILjava/nio/ByteBuffer;ILjava/nio/ByteBuffer;I[BIIIII)V */ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_I420toByteArray - (JNIEnv *, jclass, jobject, jint, jobject, jint, jobject, jint, jbyteArray, jint, jint, jint); + (JNIEnv *, jclass, jobject, jint, jobject, jint, jobject, jint, jbyteArray, jint, jint, jint, jint, jint); /* * Class: dev_onvoid_webrtc_media_video_VideoBufferConverter @@ -26,10 +26,10 @@ extern "C" { /* * Class: dev_onvoid_webrtc_media_video_VideoBufferConverter * Method: byteArrayToI420 - * Signature: ([BIILjava/nio/ByteBuffer;ILjava/nio/ByteBuffer;ILjava/nio/ByteBuffer;II)V + * Signature: ([BIIIILjava/nio/ByteBuffer;ILjava/nio/ByteBuffer;ILjava/nio/ByteBuffer;II)V */ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_byteArrayToI420 - (JNIEnv *, jclass, jbyteArray, jint, jint, jobject, jint, jobject, jint, jobject, jint, jint); + (JNIEnv *, jclass, jbyteArray, jint, jint, jint, jint, jobject, jint, jobject, jint, jobject, jint, jint); /* * Class: dev_onvoid_webrtc_media_video_VideoBufferConverter diff --git a/webrtc-jni/src/main/cpp/src/JNI_VideoBufferConverter.cpp b/webrtc-jni/src/main/cpp/src/JNI_VideoBufferConverter.cpp index 91af80794..9ee67037f 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_VideoBufferConverter.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_VideoBufferConverter.cpp @@ -63,17 +63,35 @@ size_t CalcBufferSize(int width, int height, int fourCC) { return bufferSize; } +// Throws and returns false unless [offset, offset + length) lies within the array. +static bool CheckArrayWindow(JNIEnv * env, jbyteArray array, jint offset, jint length) +{ + jsize arrayLength = env->GetArrayLength(array); + + if (offset < 0 || length < 0 || offset > arrayLength - length) { + env->Throw(jni::JavaRuntimeException(env, "Invalid array window [offset %d, length %d, array %d]", + offset, length, arrayLength)); + return false; + } + + return true; +} + JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_I420toByteArray (JNIEnv * env, jclass cls, jobject jSrcY, jint srcStrideY, jobject jSrcU, jint srcStrideU, - jobject jSrcV, jint srcStrideV, jbyteArray dst, jint width, jint height, jint fourCC) + jobject jSrcV, jint srcStrideV, jbyteArray dst, jint dstOffset, jint dstLength, + jint width, jint height, jint fourCC) { try { - jsize arrayLength = env->GetArrayLength(dst); + if (!CheckArrayWindow(env, dst, dstOffset, dstLength)) { + return; + } + size_t requiredSize = CalcBufferSize(width, height, fourCC); - if (arrayLength < requiredSize) { + if (static_cast(dstLength) < requiredSize) { env->Throw(jni::JavaRuntimeException(env, "Insufficient buffer size [has %d, need %zd]", - arrayLength, requiredSize)); + dstLength, requiredSize)); return; } @@ -89,7 +107,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_I if (!arrayPtr) { return; } - uint8_t * dstPtr = reinterpret_cast(arrayPtr.get()); + uint8_t * dstPtr = reinterpret_cast(arrayPtr.get()) + dstOffset; const int conversionResult = libyuv::ConvertFromI420(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstPtr, 0, width, height, static_cast(fourCC)); @@ -138,16 +156,20 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_I } JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_byteArrayToI420 -(JNIEnv * env, jclass cls, jbyteArray src, jint width, jint height, jobject jDstY, jint dstStrideY, - jobject jDstU, jint dstStrideU, jobject jDstV, jint dstStrideV, jint fourCC) +(JNIEnv * env, jclass cls, jbyteArray src, jint srcOffset, jint srcLength, jint width, jint height, + jobject jDstY, jint dstStrideY, jobject jDstU, jint dstStrideU, jobject jDstV, jint dstStrideV, + jint fourCC) { try { - jsize arrayLength = env->GetArrayLength(src); + if (!CheckArrayWindow(env, src, srcOffset, srcLength)) { + return; + } + size_t requiredSize = CalcBufferSize(width, height, fourCC); - if (arrayLength < requiredSize) { + if (static_cast(srcLength) < requiredSize) { env->Throw(jni::JavaRuntimeException(env, "Insufficient buffer size [has %d, need %zd]", - arrayLength, requiredSize)); + srcLength, requiredSize)); return; } @@ -162,10 +184,10 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_b if (!arrayPtr) { return; } - const uint8_t * srcPtr = reinterpret_cast(arrayPtr.get()); + const uint8_t * srcPtr = reinterpret_cast(arrayPtr.get()) + srcOffset; const int conversionResult = libyuv::ConvertToI420( - srcPtr, arrayLength, + srcPtr, srcLength, dstY, dstStrideY, dstU, dstStrideU, dstV, dstStrideV, diff --git a/webrtc/src/main/java/dev/onvoid/webrtc/media/video/VideoBufferConverter.java b/webrtc/src/main/java/dev/onvoid/webrtc/media/video/VideoBufferConverter.java index 1b8694f3e..1253c1053 100644 --- a/webrtc/src/main/java/dev/onvoid/webrtc/media/video/VideoBufferConverter.java +++ b/webrtc/src/main/java/dev/onvoid/webrtc/media/video/VideoBufferConverter.java @@ -58,7 +58,7 @@ public static void convertFromI420(VideoFrameBuffer src, byte[] dst, FourCC four i420.getDataY(), i420.getStrideY(), i420.getDataU(), i420.getStrideU(), i420.getDataV(), i420.getStrideV(), - dst, + dst, 0, dst.length, i420.getWidth(), i420.getHeight(), fourCC.value()); } @@ -67,10 +67,15 @@ public static void convertFromI420(VideoFrameBuffer src, byte[] dst, FourCC four * Convert an arbitrary {@link VideoFrameBuffer} to the specified pixel format * and write the result into a {@link ByteBuffer}. If the destination buffer * is a direct buffer, a native direct path is used; otherwise its backing - * array (or a temporary array) is employed. + * array is employed. + *

+ * The result is written to the bytes between the buffer's position and its + * limit, starting at the position. The position and limit stay as they are, + * and no byte outside of them is touched. * * @param src Source video frame buffer. - * @param dst Writable destination {@link ByteBuffer}. Must not be read-only. + * @param dst Writable destination {@link ByteBuffer}. Must not be read-only, + * and must have enough bytes remaining for the converted frame. * @param fourCC Target FourCC format identifier. * * @throws NullPointerException if {@code src} or {@code dst} is {@code null}. @@ -92,30 +97,23 @@ public static void convertFromI420(VideoFrameBuffer src, ByteBuffer dst, FourCC I420Buffer i420 = src.toI420(); if (dst.isDirect()) { + // The slice starts at the position and ends at the limit, which + // the native code sees as the buffer's address and capacity. I420toDirectBuffer( i420.getDataY(), i420.getStrideY(), i420.getDataU(), i420.getStrideU(), i420.getDataV(), i420.getStrideV(), - dst, + dst.slice(), i420.getWidth(), i420.getHeight(), fourCC.value()); } else { - byte[] arrayBuffer; - - if (dst.hasArray()) { - arrayBuffer = dst.array(); - } - else { - arrayBuffer = new byte[dst.remaining()]; - dst.get(arrayBuffer); - } - + // A writable heap buffer always has an accessible array. I420toByteArray( i420.getDataY(), i420.getStrideY(), i420.getDataU(), i420.getStrideU(), i420.getDataV(), i420.getStrideV(), - arrayBuffer, + dst.array(), dst.arrayOffset() + dst.position(), dst.remaining(), i420.getWidth(), i420.getHeight(), fourCC.value()); } @@ -142,7 +140,7 @@ public static void convertToI420(byte[] src, I420Buffer dst, FourCC fourCC) } byteArrayToI420( - src, + src, 0, src.length, dst.getWidth(), dst.getHeight(), dst.getDataY(), dst.getStrideY(), dst.getDataU(), dst.getStrideU(), @@ -155,6 +153,10 @@ public static void convertToI420(byte[] src, I420Buffer dst, FourCC fourCC) * specified by {@code fourCC}) to I420 and write the result into the provided * {@link I420Buffer}. Uses a direct native path for direct buffers; otherwise * reads from the backing or a temporary array. + *

+ * The source frame is read from the bytes between the buffer's position and + * its limit, starting at the position. The position and limit stay as they + * are. * * @param src Source pixel data buffer. * @param dst Destination I420 buffer (pre-allocated). @@ -173,8 +175,10 @@ public static void convertToI420(ByteBuffer src, I420Buffer dst, FourCC fourCC) } if (src.isDirect()) { + // The slice starts at the position and ends at the limit, which + // the native code sees as the buffer's address and capacity. directBufferToI420( - src, + src.slice(), dst.getWidth(), dst.getHeight(), dst.getDataY(), dst.getStrideY(), dst.getDataU(), dst.getStrideU(), @@ -182,18 +186,24 @@ public static void convertToI420(ByteBuffer src, I420Buffer dst, FourCC fourCC) fourCC.value()); } else { - byte[] arrayBuffer; + byte[] array; + int offset; + int length = src.remaining(); if (src.hasArray()) { - arrayBuffer = src.array(); + array = src.array(); + offset = src.arrayOffset() + src.position(); } else { - arrayBuffer = new byte[src.remaining()]; - src.get(arrayBuffer); + // Read-only, so the array is not accessible. Copied through a + // duplicate, which leaves the position of the buffer as it is. + array = new byte[length]; + offset = 0; + src.duplicate().get(array); } byteArrayToI420( - arrayBuffer, + array, offset, length, dst.getWidth(), dst.getHeight(), dst.getDataY(), dst.getStrideY(), dst.getDataU(), dst.getStrideU(), @@ -206,7 +216,7 @@ private native static void I420toByteArray( ByteBuffer srcY, int srcStrideY, ByteBuffer srcU, int srcStrideU, ByteBuffer srcV, int srcStrideV, - byte[] dst, + byte[] dst, int dstOffset, int dstLength, int width, int height, int fourCC) throws Exception; @@ -219,7 +229,7 @@ private native static void I420toDirectBuffer( int fourCC) throws Exception; private native static void byteArrayToI420( - byte[] src, + byte[] src, int srcOffset, int srcLength, int width, int height, ByteBuffer dstY, int dstStrideY, ByteBuffer dstU, int dstStrideU, diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/media/video/VideoBufferConverterTest.java b/webrtc/src/test/java/dev/onvoid/webrtc/media/video/VideoBufferConverterTest.java index 943ff540d..285db9acb 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/media/video/VideoBufferConverterTest.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/media/video/VideoBufferConverterTest.java @@ -16,6 +16,7 @@ package dev.onvoid.webrtc.media.video; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -31,6 +32,7 @@ class VideoBufferConverterTest extends TestBase { private static final int WIDTH = 32; private static final int HEIGHT = 8; + private static final int RGBA_SIZE = 4 * WIDTH * HEIGHT; @Test void convertFromI420ToByteArray() throws Exception { @@ -198,6 +200,117 @@ void convertFromSmallNonDirectBufferToI420() throws Exception { }); } + @Test + void convertFromI420ToDirectWindow() throws Exception { + convertFromI420ToWindow(new Window(true, RGBA_SIZE)); + } + + @Test + void convertFromI420ToNonDirectWindow() throws Exception { + convertFromI420ToWindow(new Window(false, RGBA_SIZE)); + } + + @Test + void convertFromI420ToSmallDirectWindow() throws Exception { + convertFromI420ToSmallWindow(new Window(true, RGBA_SIZE - 1)); + } + + @Test + void convertFromI420ToSmallNonDirectWindow() throws Exception { + convertFromI420ToSmallWindow(new Window(false, RGBA_SIZE - 1)); + } + + @Test + void convertFromDirectWindowToI420() throws Exception { + Window window = new Window(true, RGBA_SIZE); + convertFromWindowToI420(window, window.buffer); + } + + @Test + void convertFromNonDirectWindowToI420() throws Exception { + Window window = new Window(false, RGBA_SIZE); + convertFromWindowToI420(window, window.buffer); + } + + @Test + void convertFromReadOnlyDirectWindowToI420() throws Exception { + Window window = new Window(true, RGBA_SIZE); + convertFromWindowToI420(window, window.buffer.asReadOnlyBuffer()); + } + + @Test + void convertFromReadOnlyNonDirectWindowToI420() throws Exception { + Window window = new Window(false, RGBA_SIZE); + convertFromWindowToI420(window, window.buffer.asReadOnlyBuffer()); + } + + @Test + void convertFromSmallDirectWindowToI420() throws Exception { + convertFromSmallWindowToI420(new Window(true, RGBA_SIZE - 1)); + } + + @Test + void convertFromSmallNonDirectWindowToI420() throws Exception { + convertFromSmallWindowToI420(new Window(false, RGBA_SIZE - 1)); + } + + private void convertFromI420ToWindow(Window window) throws Exception { + NativeI420Buffer i420 = NativeI420Buffer.allocate(WIDTH, HEIGHT); + initializeI420Buffer(i420); + byte[] expected = new byte[RGBA_SIZE]; + VideoBufferConverter.convertFromI420(i420, expected, FourCC.RGBA); + + VideoBufferConverter.convertFromI420(i420, window.buffer, FourCC.RGBA); + + window.verifyUnmoved(); + window.verifyGuards(); + assertArrayEquals(expected, window.contents()); + } + + private void convertFromI420ToSmallWindow(Window window) throws Exception { + // The buffer's capacity would hold the frame, the window does not. + NativeI420Buffer i420 = NativeI420Buffer.allocate(WIDTH, HEIGHT); + initializeI420Buffer(i420); + assertTrue(window.buffer.capacity() >= RGBA_SIZE); + + assertThrows(RuntimeException.class, () -> { + VideoBufferConverter.convertFromI420(i420, window.buffer, FourCC.RGBA); + }); + + window.verifyUnmoved(); + window.verifyGuards(); + window.verifyUntouched(); + } + + private void convertFromWindowToI420(Window window, ByteBuffer src) throws Exception { + byte[] rgba = new byte[RGBA_SIZE]; + initializeRGBAArray(rgba); + NativeI420Buffer expected = NativeI420Buffer.allocate(WIDTH, HEIGHT); + VideoBufferConverter.convertToI420(rgba, expected, FourCC.RGBA); + + window.fill(rgba); + NativeI420Buffer i420 = NativeI420Buffer.allocate(WIDTH, HEIGHT); + VideoBufferConverter.convertToI420(src, i420, FourCC.RGBA); + + assertEquals(Window.GUARD, src.position()); + assertEquals(Window.GUARD + RGBA_SIZE, src.limit()); + verifyI420Equals(expected, i420); + } + + private void convertFromSmallWindowToI420(Window window) throws Exception { + // The bytes after the window would make up a frame, the window does not. + byte[] rgba = new byte[RGBA_SIZE - 1]; + initializeRGBAArray(rgba); + window.fill(rgba); + NativeI420Buffer i420 = NativeI420Buffer.allocate(WIDTH, HEIGHT); + + assertThrows(RuntimeException.class, () -> { + VideoBufferConverter.convertToI420(window.buffer, i420, FourCC.RGBA); + }); + + window.verifyUnmoved(); + } + private void initializeI420Buffer(I420Buffer i420) { ByteBuffer dataY = i420.getDataY(); int strideY = i420.getStrideY(); @@ -344,4 +457,94 @@ private void verifyRGBABuffer(ByteBuffer rgba, boolean expectingChanges) { } } + private void verifyI420Equals(I420Buffer expected, I420Buffer actual) { + for (int y = 0; y < HEIGHT; y++) { + for (int x = 0; x < WIDTH; x++) { + assertEquals(expected.getDataY().get(y * expected.getStrideY() + x), + actual.getDataY().get(y * actual.getStrideY() + x)); + } + } + for (int y = 0; y < HEIGHT / 2; y++) { + for (int x = 0; x < WIDTH / 2; x++) { + assertEquals(expected.getDataU().get(y * expected.getStrideU() + x), + actual.getDataU().get(y * actual.getStrideU() + x)); + assertEquals(expected.getDataV().get(y * expected.getStrideV() + x), + actual.getDataV().get(y * actual.getStrideV() + x)); + } + } + } + + + + /** + * A window between position and limit of a buffer that is a slice of a + * larger one, so that the buffer has an offset of its own as well as a + * position. Guard bytes surround the window on both sides. + */ + private static class Window { + + static final int GUARD = 16; + + static final byte GUARD_VALUE = (byte) 0x5A; + + /** The whole memory, of which the window is a part. */ + final ByteBuffer whole; + + final ByteBuffer buffer; + + final int length; + + + Window(boolean direct, int length) { + this.length = length; + + int size = GUARD + GUARD + length + GUARD; + whole = direct ? ByteBuffer.allocateDirect(size) : ByteBuffer.allocate(size); + + for (int i = 0; i < size; i++) { + whole.put(i, GUARD_VALUE); + } + + whole.position(GUARD); + buffer = whole.slice(); + whole.position(0); + + buffer.position(GUARD); + buffer.limit(GUARD + length); + } + + void fill(byte[] data) { + for (int i = 0; i < data.length; i++) { + buffer.put(GUARD + i, data[i]); + } + } + + byte[] contents() { + byte[] contents = new byte[length]; + buffer.duplicate().get(contents); + return contents; + } + + void verifyUnmoved() { + assertEquals(GUARD, buffer.position()); + assertEquals(GUARD + length, buffer.limit()); + } + + void verifyGuards() { + int start = GUARD + GUARD; + + for (int i = 0; i < whole.capacity(); i++) { + if (i < start || i >= start + length) { + assertEquals(GUARD_VALUE, whole.get(i), "Guard byte " + i + " was overwritten"); + } + } + } + + void verifyUntouched() { + for (byte value : contents()) { + assertEquals(GUARD_VALUE, value); + } + } + } + } \ No newline at end of file