fix: size YUY2, UYVY and NV12 frames of odd width in whole pixel pairs

docs/tools/video/video-buffer-converter.md

@@ -31,17 +31,19 @@ The target/source pixel layout is selected with `dev.onvoid.webrtc.media.FourCC`
 - `FourCC.RGBA` – 4 bytes per pixel, RGBA order (commonly used with BufferedImage TYPE_4BYTE_ABGR interop, see example below)
 - `FourCC.ARGB`, `FourCC.ABGR`, `FourCC.BGRA` – other 32-bit packed variants
 - `FourCC.NV12`, `FourCC.NV21` – 4:2:0 semi-planar YUV formats
+- `FourCC.YUY2`, `FourCC.UYVY` – 4:2:2 packed YUV formats, 4 bytes per pair of pixels
 
 Consult the FourCC enum in your version for the complete list.
 
 ## Buffer sizing
 
-You must allocate destination buffers large enough for the chosen format:
-- For 32-bit RGBA-like formats: `width * height * 4` bytes
-- For NV12/NV21: `width * height * 3 / 2` bytes
-- For other layouts, compute according to their specification
+Frames are stored without row padding, except where a format groups pixels in pairs: a YUY2 or UYVY row, and an NV12 or NV21 chroma row, always ends with a whole pair, so an odd width is rounded up to the next pair. A buffer must hold at least:
+- `ARGB`, `BGRA`, `ABGR`, `RGBA`: `width * height * 4` bytes
+- `BG24`: `width * height * 3` bytes
+- `I420`, `NV12`, `NV21`: `width * height + 2 * ((width + 1) / 2) * ((height + 1) / 2)` bytes
+- `YUY2`, `UYVY`: `4 * ((width + 1) / 2) * height` bytes, `4 * ((width + 1) / 2)` per row
 
-Attempting to convert into undersized buffers will result in an error.
+Both directions use the same layout, so a frame written by `convertFromI420` reads back with `convertToI420`. Converting into or from a smaller buffer throws, and touches no byte of it.
 
 ## Example: Convert VideoFrame to BufferedImage
 

webrtc-jni/src/main/cpp/src/JNI_VideoBufferConverter.cpp

@@ -23,44 +23,111 @@
 #include "libyuv/convert.h"
 #include "libyuv/video_common.h"
 
+#include <climits>
+#include <cstdint>
 #include <memory>
 
-size_t CalcBufferSize(int width, int height, int fourCC) {
-	size_t bufferSize = 0;
+// The size of a frame in the layout libyuv::ConvertToI420 reads, which
+// ConvertFromI420 below writes. YUY2 and UYVY rows, and NV12 and NV21 chroma
+// rows, hold whole pixel pairs, so an odd width is rounded up to the next pair.
+// Returns false for an empty frame, an unsupported format, or a width beyond
+// the INT_MAX / 4 that ConvertToI420 accepts. With that width, no row of 4
+// bytes per pixel overflows an int, and no frame size overflows 64 bits.
+static bool CalcBufferSize(jint width, jint height, jint fourCC, uint64_t & size)
+{
+	if (width <= 0 || height <= 0 || width > INT_MAX / 4) {
+		return false;
+	}
+
+	const uint64_t pixels = static_cast<uint64_t>(width) * height;
+	const uint64_t pairs = (static_cast<uint64_t>(width) + 1) / 2;
+	const uint64_t chromaRows = (static_cast<uint64_t>(height) + 1) / 2;
 
 	switch (fourCC) {
 		case libyuv::FOURCC_I420:
 		case libyuv::FOURCC_NV12:
 		case libyuv::FOURCC_NV21:
 		case libyuv::FOURCC_IYUV:
 		case libyuv::FOURCC_YV12:
-			bufferSize = width * height + ((width + 1) >> 1) * ((height + 1) >> 1) * 2;
-			break;
+			size = pixels + pairs * chromaRows * 2;
+			return true;
 
 		case libyuv::FOURCC_R444:
 		case libyuv::FOURCC_RGBP:
 		case libyuv::FOURCC_RGBO:
+			size = pixels * 2;
+			return true;
+
 		case libyuv::FOURCC_YUY2:
 		case libyuv::FOURCC_UYVY:
-			bufferSize = width * height * 2;
-			break;
+			size = pairs * 4 * height;
+			return true;
 
 		case libyuv::FOURCC_24BG:
-			bufferSize = width * height * 3;
-			break;
+			size = pixels * 3;
+			return true;
 
 		case libyuv::FOURCC_ARGB:
 		case libyuv::FOURCC_ABGR:
 		case libyuv::FOURCC_BGRA:
 		case libyuv::FOURCC_RGBA:
-			bufferSize = width * height * 4;
-			break;
+			size = pixels * 4;
+			return true;
 
 		default:
-			break;
+			return false;
+	}
+}
+
+// Throws and returns false unless a frame of the given size and format fits into length bytes.
+static bool CheckBufferSize(JNIEnv * env, jint width, jint height, jint fourCC, uint64_t length)
+{
+	uint64_t requiredSize;
+
+	if (!CalcBufferSize(width, height, fourCC, requiredSize)) {
+		env->Throw(jni::JavaRuntimeException(env, "Invalid frame [width %d, height %d, format 0x%08x]",
+			width, height, static_cast<unsigned>(fourCC)));
+		return false;
+	}
+	if (length < requiredSize) {
+		env->Throw(jni::JavaRuntimeException(env, "Insufficient buffer size [has %llu, need %llu]",
+			static_cast<unsigned long long>(length), static_cast<unsigned long long>(requiredSize)));
+		return false;
 	}
 
-	return bufferSize;
+	return true;
+}
+
+// libyuv::ConvertFromI420 with the strides of CalcBufferSize. On its own it
+// writes YUY2 and UYVY rows width * 2 bytes apart, and NV12 and NV21 chroma
+// rows width bytes apart, though each of these rows ends with a whole pair.
+static int ConvertFromI420(const uint8_t * srcY, int srcStrideY, const uint8_t * srcU, int srcStrideU,
+	const uint8_t * srcV, int srcStrideV, uint8_t * dst, int width, int height, uint32_t fourCC)
+{
+	const int alignedWidth = (width + 1) & ~1;
+	const ptrdiff_t lumaSize = static_cast<ptrdiff_t>(width) * height;
+
+	switch (fourCC) {
+		case libyuv::FOURCC_YUY2:
+			return libyuv::I420ToYUY2(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV,
+				dst, alignedWidth * 2, width, height);
+
+		case libyuv::FOURCC_UYVY:
+			return libyuv::I420ToUYVY(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV,
+				dst, alignedWidth * 2, width, height);
+
+		case libyuv::FOURCC_NV12:
+			return libyuv::I420ToNV12(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV,
+				dst, width, dst + lumaSize, alignedWidth, width, height);
+
+		case libyuv::FOURCC_NV21:
+			return libyuv::I420ToNV21(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV,
+				dst, width, dst + lumaSize, alignedWidth, width, height);
+
+		default:
+			return libyuv::ConvertFromI420(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV,
+				dst, 0, width, height, fourCC);
+	}
 }
 
 // Throws and returns false unless [offset, offset + length) lies within the array.
@@ -87,11 +154,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_I
 			return;
 		}
 
-		size_t requiredSize = CalcBufferSize(width, height, fourCC);
-
-		if (static_cast<size_t>(dstLength) < requiredSize) {
-			env->Throw(jni::JavaRuntimeException(env, "Insufficient buffer size [has %d, need %zd]",
-				dstLength, requiredSize));
+		if (!CheckBufferSize(env, width, height, fourCC, static_cast<uint64_t>(dstLength))) {
 			return;
 		}
 
@@ -109,8 +172,8 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_I
 		}
 		uint8_t * dstPtr = reinterpret_cast<uint8_t *>(arrayPtr.get()) + dstOffset;
 
-		const int conversionResult = libyuv::ConvertFromI420(srcY, srcStrideY, srcU, srcStrideU,
-			srcV, srcStrideV, dstPtr, 0, width, height, static_cast<uint32_t>(fourCC));
+		const int conversionResult = ConvertFromI420(srcY, srcStrideY, srcU, srcStrideU,
+			srcV, srcStrideV, dstPtr, width, height, static_cast<uint32_t>(fourCC));
 
 		if (conversionResult < 0) {
 			env->Throw(jni::JavaRuntimeException(env, "Failed to convert buffer to I420: %d",
@@ -133,17 +196,14 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_I
 	uint8_t * address = static_cast<uint8_t *>(env->GetDirectBufferAddress(dst));
 
 	if (address != NULL) {
-		size_t bufferLength = env->GetDirectBufferCapacity(dst);
-		size_t requiredSize = CalcBufferSize(width, height, fourCC);
+		jlong bufferLength = env->GetDirectBufferCapacity(dst);
 
-		if (bufferLength < requiredSize) {
-			env->Throw(jni::JavaRuntimeException(env, "Insufficient buffer size [has %zd, need %zd]",
-				bufferLength, requiredSize));
+		if (!CheckBufferSize(env, width, height, fourCC, static_cast<uint64_t>(bufferLength))) {
 			return;
 		}
 
-		const int conversionResult = libyuv::ConvertFromI420(srcY, srcStrideY, srcU, srcStrideU,
-			srcV, srcStrideV, address, 0, width, height, static_cast<uint32_t>(fourCC));
+		const int conversionResult = ConvertFromI420(srcY, srcStrideY, srcU, srcStrideU,
+			srcV, srcStrideV, address, width, height, static_cast<uint32_t>(fourCC));
 
 		if (conversionResult < 0) {
 			env->Throw(jni::JavaRuntimeException(env, "Failed to convert buffer to I420: %d",
@@ -165,11 +225,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_b
 			return;
 		}
 
-		size_t requiredSize = CalcBufferSize(width, height, fourCC);
-
-		if (static_cast<size_t>(srcLength) < requiredSize) {
-			env->Throw(jni::JavaRuntimeException(env, "Insufficient buffer size [has %d, need %zd]",
-				srcLength, requiredSize));
+		if (!CheckBufferSize(env, width, height, fourCC, static_cast<uint64_t>(srcLength))) {
 			return;
 		}
 
@@ -215,12 +271,9 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoBufferConverter_d
 	const uint8_t * address = static_cast<uint8_t *>(env->GetDirectBufferAddress(src));
 
 	if (address != NULL) {
-		size_t bufferLength = env->GetDirectBufferCapacity(src);
-		size_t requiredSize = CalcBufferSize(width, height, fourCC);
+		jlong bufferLength = env->GetDirectBufferCapacity(src);
 
-		if (bufferLength < requiredSize) {
-			env->Throw(jni::JavaRuntimeException(env, "Insufficient buffer size [has %zd, need %zd]",
-				bufferLength, requiredSize));
+		if (!CheckBufferSize(env, width, height, fourCC, static_cast<uint64_t>(bufferLength))) {
 			return;
 		}
 

webrtc/src/main/java/dev/onvoid/webrtc/media/video/VideoBufferConverter.java

@@ -25,6 +25,14 @@
  * Utility methods to convert between I420 video frame buffers and other pixel
  * formats represented by a FourCC. Delegates actual conversion work to native
  * methods for performance.
+ * <p>
+ * A frame is stored without row padding, except where a format groups pixels
+ * in pairs: a {@link FourCC#YUY2} or {@link FourCC#UYVY} row, and an
+ * {@link FourCC#NV12} or {@link FourCC#NV21} chroma row, always ends with a
+ * whole pair. With an odd width, a YUY2 or UYVY row takes
+ * {@code 4 * ((width + 1) / 2)} bytes, and an NV12 or NV21 chroma row
+ * {@code 2 * ((width + 1) / 2)}. Conversions in both directions use this
+ * layout, and fail when the buffer is smaller than the frame.
  *
  * @author Alex Andres
  */