fix: bound AudioResampler buffers and keep cropped I420 buffers alive

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

@@ -68,6 +68,16 @@ JNIEXPORT jint JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_resampl
 		webrtc::PushResampler<int16_t> * resampler = GetHandle<webrtc::PushResampler<int16_t>>(env, caller);
 		CHECK_HANDLEV(resampler, -1);
 
+		// Java checks these too, but the views below must never reach past an array.
+		const jlong srcBytes = static_cast<jlong>(srcSamplesPerChannel) * channels * sizeof(int16_t);
+		const jlong dstBytes = static_cast<jlong>(dstSamplesPerChannel) * channels * sizeof(int16_t);
+
+		if (srcSamplesPerChannel <= 0 || dstSamplesPerChannel <= 0 || channels <= 0 ||
+			env->GetArrayLength(samplesIn) < srcBytes || env->GetArrayLength(samplesOut) < dstBytes) {
+			ThrowJavaException(env, "java/lang/IllegalArgumentException", "Sample arrays too short for the requested samples");
+			return -1;
+		}
+
 		auto releaseSrc = [env, samplesIn](jbyte * bytes) {
 			env->ReleaseByteArrayElements(samplesIn, bytes, JNI_ABORT);
 		};

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

@@ -20,6 +20,7 @@
 #include "JavaUtils.h"
 
 #include "api/video/i420_buffer.h"
+#include "common_video/include/video_frame_buffer.h"
 #include "libyuv/scale.h"
 
 JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_media_video_NativeI420Buffer_allocate
@@ -64,6 +65,34 @@ JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_media_video_NativeI420Buffer_co
 	}
 }
 
+JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_media_video_NativeI420Buffer_crop
+(JNIEnv * env, jobject caller, jint cropX, jint cropY, jint cropW, jint cropH)
+{
+	webrtc::I420BufferInterface * buffer = GetHandle<webrtc::I420BufferInterface>(env, caller);
+	CHECK_HANDLEV(buffer, nullptr);
+
+	try {
+		webrtc::scoped_refptr<webrtc::I420BufferInterface> source(buffer);
+
+		// The cropped buffer points into the source, which it keeps alive until it is gone itself.
+		webrtc::scoped_refptr<webrtc::I420BufferInterface> cropped = webrtc::WrapI420Buffer(cropW, cropH,
+			source->DataY() + cropX + cropY * source->StrideY(), source->StrideY(),
+			source->DataU() + cropX / 2 + cropY / 2 * source->StrideU(), source->StrideU(),
+			source->DataV() + cropX / 2 + cropY / 2 * source->StrideV(), source->StrideV(),
+			[source]() {});
+
+		jni::JavaLocalRef<jobject> jBuffer = jni::I420Buffer::toJava(env, cropped);
+
+		cropped->AddRef();
+
+		return jBuffer.release();
+	}
+	catch (...) {
+		ThrowCxxJavaException(env);
+		return nullptr;
+	}
+}
+
 JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_NativeI420Buffer_cropAndScale
 (JNIEnv * env, jclass caller, jobject jSrcY, jint srcStrideY, jobject jSrcU, jint srcStrideU,
 	jobject jSrcV, jint srcStrideV, jint cropX, jint cropY, jint cropW, jint cropH,

webrtc-jni/src/main/cpp/src/api/VideoFrame.cpp

@@ -17,6 +17,8 @@
 #include "api/VideoFrame.h"
 #include "JavaObject.h"
 #include "JavaUtils.h"
+#include "JavaRuntimeException.h"
+#include "JavaWrappedException.h"
 #include "JNI_WebRTC.h"
 
 #include "api/video/i420_buffer.h"
@@ -62,6 +64,13 @@ namespace jni
 			*/
 
 			webrtc::I420BufferInterface * i420Buffer = GetHandle<webrtc::I420BufferInterface>(env, jBuffer);
+
+			// A buffer that has been released for good has no pixels left.
+			if (i420Buffer == nullptr) {
+				throw JavaWrappedException(JavaLocalRef<jthrowable>(env,
+					JavaRuntimeException(env, "The buffer of the VideoFrame has been released")));
+			}
+
 			webrtc::scoped_refptr<webrtc::I420BufferInterface> buffer(i420Buffer);
 
 			return webrtc::VideoFrame::Builder()
@@ -107,6 +116,11 @@ namespace jni
 			if (env->IsInstanceOf(javaBuffer, nativeClass->cls)) {
 				webrtc::I420BufferInterface * buffer = GetHandle<webrtc::I420BufferInterface>(env, javaBuffer);
 
+				// Released for good, so there is nothing to read.
+				if (buffer == nullptr) {
+					return nullptr;
+				}
+
 				return webrtc::scoped_refptr<webrtc::VideoFrameBuffer>(buffer);
 			}
 
@@ -154,9 +168,14 @@ namespace jni
 		{
 			const auto javaClass = JavaClasses::get<JavaNativeI420BufferClass>(env);
 
-			jobject yBuffer = env->NewDirectByteBuffer(const_cast<uint8_t *>(buffer->DataY()), static_cast<jlong>(buffer->StrideY()) * buffer->height());
-			jobject uBuffer = env->NewDirectByteBuffer(const_cast<uint8_t *>(buffer->DataU()), static_cast<jlong>(buffer->StrideU()) * buffer->ChromaHeight());
-			jobject vBuffer = env->NewDirectByteBuffer(const_cast<uint8_t *>(buffer->DataV()), static_cast<jlong>(buffer->StrideV()) * buffer->ChromaHeight());
+			// The last row ends at the width: a cropped buffer has no padding behind it that belongs to it.
+			auto planeSize = [](int stride, int width, int height) {
+				return static_cast<jlong>(stride) * (height - 1) + width;
+			};
+
+			jobject yBuffer = env->NewDirectByteBuffer(const_cast<uint8_t *>(buffer->DataY()), planeSize(buffer->StrideY(), buffer->width(), buffer->height()));
+			jobject uBuffer = env->NewDirectByteBuffer(const_cast<uint8_t *>(buffer->DataU()), planeSize(buffer->StrideU(), buffer->ChromaWidth(), buffer->ChromaHeight()));
+			jobject vBuffer = env->NewDirectByteBuffer(const_cast<uint8_t *>(buffer->DataV()), planeSize(buffer->StrideV(), buffer->ChromaWidth(), buffer->ChromaHeight()));
 
 			jobject jBuffer = env->NewObject(javaClass->cls, javaClass->ctor, buffer->width(), buffer->height(),
 				yBuffer, buffer->StrideY(), uBuffer, buffer->StrideU(), vBuffer, buffer->StrideV());

webrtc/src/main/java/dev/onvoid/webrtc/media/audio/AudioResampler.java

@@ -19,6 +19,7 @@
 import static java.util.Objects.requireNonNull;
 
 import dev.onvoid.webrtc.internal.DisposableNativeObject;
+import dev.onvoid.webrtc.internal.NativeLoader;
 
 /**
  * Audio sampling rate converter. This resampler operates on audio frames of 10
@@ -28,6 +29,24 @@
  */
 public class AudioResampler extends DisposableNativeObject {
 
+	/** The most samples of one channel in 10 ms that WebRTC resamples, at 384 kHz. */
+	private static final int MAX_SAMPLES_PER_CHANNEL = 3840;
+
+	/** The most channels that WebRTC resamples at once. */
+	private static final int MAX_CHANNELS = 8;
+
+	/** Bytes of a 16-bit PCM sample. */
+	private static final int BYTES_PER_SAMPLE = 2;
+
+	static {
+		try {
+			NativeLoader.loadLibrary("webrtc-java");
+		}
+		catch (Exception e) {
+			throw new RuntimeException("Load library 'webrtc-java' failed", e);
+		}
+	}
+
 	/**
 	 * Creates a new {@code AudioResampler} without sampling parameters.
 	 */
@@ -44,7 +63,14 @@ public AudioResampler() {
 	 * @param channels         The number of audio channels to resample.
 	 */
 	public AudioResampler(int sourceSampleRate, int targetSampleRate, int channels) {
-		initialize(getSamplesPerChannel(sourceSampleRate), getSamplesPerChannel(targetSampleRate), channels);
+		int srcSamplesPerChannel = getSamplesPerChannel(sourceSampleRate);
+		int dstSamplesPerChannel = getSamplesPerChannel(targetSampleRate);
+
+		checkSamplesPerChannel(srcSamplesPerChannel, "source");
+		checkSamplesPerChannel(dstSamplesPerChannel, "destination");
+		checkChannels(channels);
+
+		initialize(srcSamplesPerChannel, dstSamplesPerChannel, channels);
 	}
 
 	/**
@@ -59,20 +85,24 @@ public AudioResampler(int sourceSampleRate, int targetSampleRate, int channels)
 	 * @param dstSamplesPerChannel The number of samples per destination channel.
 	 * @param channels             The number of audio channels to resample.
 	 *
-	 * @return The number of converted audio samples.
+	 * @return The number of converted audio samples per channel.
+	 *
+	 * @throws IllegalArgumentException if a sample count or the channel count is out of range, or if
+	 *                                  an array holds fewer than {@code samplesPerChannel * channels}
+	 *                                  samples.
 	 */
 	public int resample(byte[] samplesIn, int srcSamplesPerChannel, byte[] samplesOut, int dstSamplesPerChannel,
 						int channels) {
 		requireNonNull(samplesIn);
 		requireNonNull(samplesOut);
 
-		final int maxSamplesOut = samplesOut.length / 2; // 16-bit PCM sample
-
-		if (dstSamplesPerChannel > maxSamplesOut) {
-			throw new IllegalArgumentException("Insufficient samples output length");
-		}
+		checkSamplesPerChannel(srcSamplesPerChannel, "source");
+		checkSamplesPerChannel(dstSamplesPerChannel, "destination");
+		checkChannels(channels);
+		checkLength(samplesIn, srcSamplesPerChannel, channels, "input");
+		checkLength(samplesOut, dstSamplesPerChannel, channels, "output");
 
-		return resampleInternal(samplesIn, srcSamplesPerChannel, samplesOut, maxSamplesOut, channels);
+		return resampleInternal(samplesIn, srcSamplesPerChannel, samplesOut, dstSamplesPerChannel, channels);
 	}
 
 	@Override
@@ -95,4 +125,28 @@ private native int resampleInternal(byte[] samplesIn, int srcSamplesPerChannel,
 	public static int getSamplesPerChannel(int sampleRate) {
 		return sampleRate / 100;
 	}
+
+	private static void checkSamplesPerChannel(int samplesPerChannel, String name) {
+		if (samplesPerChannel <= 0 || samplesPerChannel > MAX_SAMPLES_PER_CHANNEL) {
+			throw new IllegalArgumentException("Invalid number of " + name + " samples per channel: "
+					+ samplesPerChannel + ", must be between 1 and " + MAX_SAMPLES_PER_CHANNEL);
+		}
+	}
+
+	private static void checkChannels(int channels) {
+		if (channels <= 0 || channels > MAX_CHANNELS) {
+			throw new IllegalArgumentException("Invalid number of channels: " + channels
+					+ ", must be between 1 and " + MAX_CHANNELS);
+		}
+	}
+
+	private static void checkLength(byte[] samples, int samplesPerChannel, int channels, String name) {
+		// Both counts are bounded, so this cannot overflow.
+		final int required = samplesPerChannel * channels * BYTES_PER_SAMPLE;
+
+		if (samples.length < required) {
+			throw new IllegalArgumentException("Insufficient samples " + name + " length: need "
+					+ required + " bytes, but was " + samples.length);
+		}
+	}
 }

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

@@ -131,49 +131,26 @@ private static native NativeI420Buffer copy(int width, int height, ByteBuffer da
 												int strideU, ByteBuffer dataV, int strideV);
 
 	/**
-	 * Wraps existing ByteBuffers into NativeI420Buffer object without copying the
-	 * contents.
+	 * Crops this buffer without copying. The cropped buffer shares the pixels of this one and holds a
+	 * reference to it, so it stays valid when this buffer is released.
 	 */
-	private static I420Buffer wrap(int width, int height, ByteBuffer dataY, int strideY, ByteBuffer dataU,
-			int strideU, ByteBuffer dataV, int strideV) {
-		if (dataY == null || dataU == null || dataV == null) {
-			throw new IllegalArgumentException("Data buffers cannot be null");
+	private native NativeI420Buffer crop(int cropX, int cropY, int cropWidth, int cropHeight);
+
+	private static VideoFrameBuffer cropAndScale(final NativeI420Buffer buffer, int cropX, int cropY,
+			int cropWidth, int cropHeight, int scaleWidth, int scaleHeight) {
+		if (cropX < 0 || cropY < 0 || cropWidth <= 0 || cropHeight <= 0
+				|| cropWidth > buffer.getWidth() - cropX || cropHeight > buffer.getHeight() - cropY) {
+			throw new IllegalArgumentException("Crop rectangle " + cropWidth + "x" + cropHeight + " at ("
+					+ cropX + ", " + cropY + ") does not fit the " + buffer.getWidth() + "x"
+					+ buffer.getHeight() + " buffer");
 		}
-		if (!dataY.isDirect() || !dataU.isDirect() || !dataV.isDirect()) {
-			throw new IllegalArgumentException("Data buffers must be direct byte buffers");
+		if (scaleWidth <= 0 || scaleHeight <= 0) {
+			throw new IllegalArgumentException("Invalid scale size " + scaleWidth + "x" + scaleHeight);
 		}
 
-		// Slice the buffers to prevent external modifications to the position / limit
-		// of the buffer.
-		// Note that this doesn't protect the contents of the buffers from modifications.
-		dataY = dataY.slice();
-		dataU = dataU.slice();
-		dataV = dataV.slice();
-
-		final int chromaWidth = (width + 1) / 2;
-		final int chromaHeight = (height + 1) / 2;
-
-		checkCapacity(dataY, width, height, strideY);
-		checkCapacity(dataU, chromaWidth, chromaHeight, strideU);
-		checkCapacity(dataV, chromaWidth, chromaHeight, strideV);
-
-		return new NativeI420Buffer(width, height, dataY, strideY, dataU, strideU, dataV, strideV);
-	}
-
-	private static VideoFrameBuffer cropAndScale(final I420Buffer buffer, int cropX, int cropY,
-			int cropWidth, int cropHeight, int scaleWidth, int scaleHeight) {
 		if (cropWidth == scaleWidth && cropHeight == scaleHeight) {
 			// No scaling.
-			ByteBuffer dataY = buffer.getDataY();
-			ByteBuffer dataU = buffer.getDataU();
-			ByteBuffer dataV = buffer.getDataV();
-
-			dataY.position(cropX + cropY * buffer.getStrideY());
-			dataU.position(cropX / 2 + cropY / 2 * buffer.getStrideU());
-			dataV.position(cropX / 2 + cropY / 2 * buffer.getStrideV());
-
-			return wrap(scaleWidth, scaleHeight, dataY.slice(), buffer.getStrideY(), dataU.slice(),
-					buffer.getStrideU(), dataV.slice(), buffer.getStrideV());
+			return buffer.crop(cropX, cropY, cropWidth, cropHeight);
 		}
 
 		I420Buffer newBuffer = allocate(scaleWidth, scaleHeight);
@@ -186,16 +163,6 @@ private static VideoFrameBuffer cropAndScale(final I420Buffer buffer, int cropX,
 		return newBuffer;
 	}
 
-	private static void checkCapacity(ByteBuffer data, int width, int height, int stride) {
-		// The last row does not necessarily need padding.
-		final int minCapacity = stride * (height - 1) + width;
-
-		if (data.capacity() < minCapacity) {
-			throw new IllegalArgumentException("Buffer must be at least " + minCapacity +
-					" bytes, but was " + data.capacity());
-		}
-	}
-
 	private static native void cropAndScale(ByteBuffer srcY, int srcStrideY,
 		      ByteBuffer srcU, int srcStrideU, ByteBuffer srcV, int srcStrideV, int cropX, int cropY,
 		      int cropWidth, int cropHeight, ByteBuffer dstY, int dstStrideY, ByteBuffer dstU,