diff --git a/docs/tools/audio/audio-converter.md b/docs/tools/audio/audio-converter.md index 98269c25..5603aa27 100644 --- a/docs/tools/audio/audio-converter.md +++ b/docs/tools/audio/audio-converter.md @@ -95,6 +95,7 @@ finally { ## Error handling and caveats +- Sample rates must be between 100 Hz and 384 kHz and channel counts between 1 and 8, otherwise the constructor throws `IllegalArgumentException`. Remixing at high rates is bounded too: one 10 ms frame of the larger channel count must fit 7680 samples (e.g. 8 channels up to 96 kHz). - Frame length must be exactly 10 ms. If `src` has fewer samples than required, `convert` throws `IllegalArgumentException`. - Ensure `dst` is at least `getTargetBufferSize()` bytes. Otherwise, `IllegalArgumentException` is thrown. - Audio is assumed to be 16‑bit PCM. Do not pass float or 24‑bit samples. diff --git a/webrtc-jni/src/main/cpp/include/media/audio/AudioConverter.h b/webrtc-jni/src/main/cpp/include/media/audio/AudioConverter.h index fa496ecf..ac7dd322 100644 --- a/webrtc-jni/src/main/cpp/include/media/audio/AudioConverter.h +++ b/webrtc-jni/src/main/cpp/include/media/audio/AudioConverter.h @@ -30,10 +30,13 @@ namespace jni AudioConverter(const AudioConverter&) = delete; AudioConverter& operator=(const AudioConverter&) = delete; + // Frames are samples per channel in 10 ms. Throws std::invalid_argument for a format WebRTC cannot convert. static std::unique_ptr create(size_t srcFrames, size_t srcChannels, size_t dstFrames, size_t dstChannels); virtual ~AudioConverter() = default; + // Both sizes count interleaved samples over all channels: srcSize must be srcFrames * srcChannels, + // dstSize, the capacity of dst, at least dstFrames * dstChannels. Throws std::invalid_argument otherwise. virtual void convert(const int16_t * src, size_t srcSize, int16_t * dst, size_t dstSize) = 0; size_t getSrcChannels() const { return srcChannels; } @@ -42,7 +45,6 @@ namespace jni size_t getDstFrames() const { return dstFrames; } protected: - AudioConverter(); AudioConverter(size_t srcFrames, size_t srcChannels, size_t dstFrames, size_t dstChannels); void checkSizes(size_t srcSize, size_t dstCapacity) const; diff --git a/webrtc-jni/src/main/cpp/src/JNI_AudioConverter.cpp b/webrtc-jni/src/main/cpp/src/JNI_AudioConverter.cpp index 2db13ac3..48ea4b58 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_AudioConverter.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_AudioConverter.cpp @@ -20,14 +20,29 @@ #include "JavaUtils.h" #include +#include JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioConverter_convertInternal (JNIEnv * env, jobject caller, jbyteArray src, jint nSrcSamples, jbyteArray dst, jint nDstSamples) { - try { - jni::AudioConverter * converter = GetHandle(env, caller); - CHECK_HANDLE(converter); + jni::AudioConverter * converter = GetHandle(env, caller); + CHECK_HANDLE(converter); + + if (src == nullptr || dst == nullptr) { + ThrowJavaException(env, "java/lang/NullPointerException", "Sample arrays must not be null"); + return; + } + + // The counts are samples over all channels. The converter checks them against its format, this the arrays. + const jlong bytesPerSample = sizeof(int16_t); + + if (nSrcSamples < 0 || nDstSamples < 0 || + env->GetArrayLength(src) < nSrcSamples * bytesPerSample || env->GetArrayLength(dst) < nDstSamples * bytesPerSample) { + ThrowJavaException(env, "java/lang/IllegalArgumentException", "Sample arrays too short for the requested samples"); + return; + } + try { auto releaseSrc = [env, src](jbyte * bytes) { env->ReleaseByteArrayElements(src, bytes, JNI_ABORT); }; @@ -47,6 +62,9 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioConverter_convert converter->convert(reinterpret_cast(srcPtr.get()), nSrcSamples, reinterpret_cast(dstPtr.get()), nDstSamples); } + catch (const std::invalid_argument & e) { + ThrowJavaException(env, "java/lang/IllegalArgumentException", e.what()); + } catch (...) { ThrowCxxJavaException(env); } @@ -70,7 +88,15 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioConverter_initial size_t srcFrames = webrtc::AudioProcessing::GetFrameSize(srcSampleRate); size_t dstFrames = webrtc::AudioProcessing::GetFrameSize(dstSampleRate); - jni::AudioConverter * converter = jni::AudioConverter::create(srcFrames, srcChannels, dstFrames, dstChannels).release(); + try { + jni::AudioConverter * converter = jni::AudioConverter::create(srcFrames, srcChannels, dstFrames, dstChannels).release(); - SetHandle(env, caller, converter); + SetHandle(env, caller, converter); + } + catch (const std::invalid_argument & e) { + ThrowJavaException(env, "java/lang/IllegalArgumentException", e.what()); + } + catch (...) { + ThrowCxxJavaException(env); + } } \ No newline at end of file diff --git a/webrtc-jni/src/main/cpp/src/media/audio/AudioConverter.cpp b/webrtc-jni/src/main/cpp/src/media/audio/AudioConverter.cpp index 7014a44b..b7b2d0b0 100644 --- a/webrtc-jni/src/main/cpp/src/media/audio/AudioConverter.cpp +++ b/webrtc-jni/src/main/cpp/src/media/audio/AudioConverter.cpp @@ -16,9 +16,13 @@ #include "media/audio/AudioConverter.h" +#include #include +#include +#include #include "audio/utility/channel_mixer.h" +#include "common_audio/include/audio_util.h" #include "common_audio/resampler/include/push_resampler.h" namespace jni @@ -49,14 +53,19 @@ namespace jni ChannelConverter(size_t srcFrames, size_t srcChannels, size_t dstFrames, size_t dstChannels) : AudioConverter(srcFrames, srcChannels, dstFrames, dstChannels) { + RTC_CHECK_EQ(srcFrames, dstFrames); + + // The mixer works in place in an AudioFrame, which must hold both layouts. + if (std::max(srcChannels, dstChannels) * srcFrames > webrtc::AudioFrame::kMaxDataSizeSamples) { + throw std::invalid_argument("Too many samples to remix: " + std::to_string(srcFrames) + " per channel, " + + std::to_string(std::max(srcChannels, dstChannels)) + " channels"); + } + webrtc::ChannelLayout srcLayout = webrtc::GuessChannelLayout(static_cast(srcChannels)); webrtc::ChannelLayout dstLayout = webrtc::GuessChannelLayout(static_cast(dstChannels)); mixer = std::make_unique(srcLayout, dstLayout); frame = std::make_unique(); - - frame->samples_per_channel_ = srcFrames; - frame->num_channels_ = srcChannels; } ~ChannelConverter() override @@ -66,7 +75,8 @@ namespace jni void convert(const int16_t * src, size_t srcSize, int16_t * dst, size_t dstSize) override { checkSizes(srcSize, dstSize); - std::memcpy(frame->mutable_data(), src, srcFrames * srcChannels * sizeof(int16_t)); + // Transform() leaves the frame in the output layout, so the input layout is set on every call. + std::memcpy(frame->mutable_data(srcFrames, srcChannels).data().data(), src, srcFrames * srcChannels * sizeof(int16_t)); mixer->Transform(frame.get()); @@ -84,6 +94,8 @@ namespace jni ResampleConverter(size_t srcFrames, size_t srcChannels, size_t dstFrames, size_t dstChannels) : AudioConverter(srcFrames, srcChannels, dstFrames, dstChannels) { + RTC_CHECK_EQ(srcChannels, dstChannels); + resampler = std::make_unique>(srcFrames, dstFrames, srcChannels); } @@ -91,9 +103,12 @@ namespace jni { } - void convert(const int16_t * src, size_t srcSamplesPerChannel, int16_t * dst, size_t dstSamplesPerChannel) override { - webrtc::InterleavedView srcView(src, srcSamplesPerChannel, srcChannels); - webrtc::InterleavedView dstView(dst, dstSamplesPerChannel, srcChannels); + void convert(const int16_t * src, size_t srcSize, int16_t * dst, size_t dstSize) override { + checkSizes(srcSize, dstSize); + + // The views count samples per channel, the sizes count all channels. + webrtc::InterleavedView srcView(src, srcFrames, srcChannels); + webrtc::InterleavedView dstView(dst, dstFrames, dstChannels); resampler->Resample(srcView, dstView); } @@ -106,7 +121,9 @@ namespace jni class CompositionConverter : public AudioConverter { public: explicit CompositionConverter(std::vector> converters_) - : converters(std::move(converters_)) + : AudioConverter(converters_.front()->getSrcFrames(), converters_.front()->getSrcChannels(), + converters_.back()->getDstFrames(), converters_.back()->getDstChannels()), + converters(std::move(converters_)) { RTC_CHECK_GE(converters.size(), 2); @@ -121,11 +138,14 @@ namespace jni } void convert(const int16_t * src, size_t srcSize, int16_t * dst, size_t dstSize) override { + checkSizes(srcSize, dstSize); + converters.front()->convert(src, srcSize, buffers.front().data(), buffers.front().size()); - for (size_t i = 2; i < converters.size(); ++i) { - auto & src_buffer = buffers[i - 2]; - auto & dst_buffer = buffers[i - 1]; + // Converter i reads the buffer of converter i - 1. + for (size_t i = 1; i < converters.size() - 1; ++i) { + auto & src_buffer = buffers[i - 1]; + auto & dst_buffer = buffers[i]; converters[i]->convert(src_buffer.data(), src_buffer.size(), dst_buffer.data(), dst_buffer.size()); } @@ -141,6 +161,20 @@ namespace jni std::unique_ptr AudioConverter::create(size_t srcFrames, size_t srcChannels, size_t dstFrames, size_t dstChannels) { + auto checkFormat = [](size_t frames, size_t channels, const char * name) { + if (frames == 0 || frames > webrtc::kMaxSamplesPerChannel10ms) { + throw std::invalid_argument(std::string("Invalid number of ") + name + " samples per channel: " + + std::to_string(frames) + ", must be between 1 and " + std::to_string(webrtc::kMaxSamplesPerChannel10ms)); + } + if (channels == 0 || channels > static_cast(webrtc::kMaxConcurrentChannels)) { + throw std::invalid_argument(std::string("Invalid number of ") + name + " channels: " + + std::to_string(channels) + ", must be between 1 and " + std::to_string(webrtc::kMaxConcurrentChannels)); + } + }; + + checkFormat(srcFrames, srcChannels, "source"); + checkFormat(dstFrames, dstChannels, "destination"); + std::unique_ptr converter; if (srcChannels > dstChannels) { @@ -177,14 +211,6 @@ namespace jni return converter; } - AudioConverter::AudioConverter() : - srcFrames(0), - srcChannels(0), - dstFrames(0), - dstChannels(0) - { - } - AudioConverter::AudioConverter(size_t srcFrames, size_t srcChannels, size_t dstFrames, size_t dstChannels) : srcFrames(srcFrames), srcChannels(srcChannels), @@ -194,7 +220,13 @@ namespace jni } void AudioConverter::checkSizes(size_t srcSize, size_t dstCapacity) const { - RTC_CHECK_EQ(srcSize, srcChannels * srcFrames); - RTC_CHECK_GE(dstCapacity, dstChannels * dstFrames); + if (srcSize != srcChannels * srcFrames) { + throw std::invalid_argument("Source must hold " + std::to_string(srcChannels * srcFrames) + + " samples, but has " + std::to_string(srcSize)); + } + if (dstCapacity < dstChannels * dstFrames) { + throw std::invalid_argument("Destination must hold " + std::to_string(dstChannels * dstFrames) + + " samples, but has room for " + std::to_string(dstCapacity)); + } } } \ No newline at end of file diff --git a/webrtc/src/main/java/dev/onvoid/webrtc/media/audio/AudioConverter.java b/webrtc/src/main/java/dev/onvoid/webrtc/media/audio/AudioConverter.java index 95080e6a..d3bde6f3 100644 --- a/webrtc/src/main/java/dev/onvoid/webrtc/media/audio/AudioConverter.java +++ b/webrtc/src/main/java/dev/onvoid/webrtc/media/audio/AudioConverter.java @@ -16,6 +16,8 @@ package dev.onvoid.webrtc.media.audio; +import static java.util.Objects.requireNonNull; + import dev.onvoid.webrtc.internal.DisposableNativeObject; /** @@ -27,15 +29,18 @@ */ public class AudioConverter extends DisposableNativeObject { - /** The number of samples in a 10 ms frame for the input audio. */ + /** The highest sampling frequency that WebRTC converts. */ + private static final int MAX_SAMPLE_RATE = 384000; + + /** The most channels that WebRTC converts. */ + private static final int MAX_CHANNELS = 8; + + /** The number of samples over all channels in a 10 ms frame for the input audio. */ private final int srcSamples; - /** The number of samples in a 10 ms frame for the output audio. */ + /** The number of samples over all channels in a 10 ms frame for the output audio. */ private final int dstSamples; - /** The number of output samples after conversion in a 10 ms frame. */ - private final int dstSamplesOut; - /** * Creates a new {@code AudioConverter} with specified sampling frequency @@ -45,12 +50,20 @@ public class AudioConverter extends DisposableNativeObject { * @param srcChannels The number of audio channels of the input signal. * @param dstSampleRate The sampling frequency of the output signal. * @param dstChannels The number of audio channels of the output signal. + * + * @throws IllegalArgumentException if a sampling frequency is not between + * 100 Hz and 384 kHz, or a channel count + * not between 1 and 8. */ public AudioConverter(int srcSampleRate, int srcChannels, int dstSampleRate, int dstChannels) { + checkSampleRate(srcSampleRate, "source"); + checkSampleRate(dstSampleRate, "destination"); + checkChannels(srcChannels, "source"); + checkChannels(dstChannels, "destination"); + this.srcSamples = srcSampleRate / 100 * srcChannels; this.dstSamples = dstSampleRate / 100 * dstChannels; - this.dstSamplesOut = (dstSampleRate / 100) * dstChannels; initialize(srcSampleRate, srcChannels, dstSampleRate, dstChannels); } @@ -74,12 +87,15 @@ public int getTargetBufferSize() { * @param src The audio samples to convert. * @param dst The output buffer for converted audio samples. * - * @return The number of converted samples. + * @return The number of converted samples over all channels. * * @throws IllegalArgumentException if the buffer sizes do not match the * frame sizes. */ public int convert(byte[] src, byte[] dst) { + requireNonNull(src); + requireNonNull(dst); + if (src.length / 2 < srcSamples) { throw new IllegalArgumentException(String.format( "Insufficient samples input length: %d vs. %d", @@ -93,7 +109,7 @@ public int convert(byte[] src, byte[] dst) { convertInternal(src, srcSamples, dst, dstSamples); - return dstSamplesOut; + return dstSamples; } @Override @@ -101,6 +117,20 @@ public int convert(byte[] src, byte[] dst) { private native void initialize(int srcSampleRate, int srcChannels, int dstSampleRate, int dstChannels); - public native void convertInternal(byte[] src, int nSrcSamples, byte[] dst, int nDstSamples); + private native void convertInternal(byte[] src, int nSrcSamples, byte[] dst, int nDstSamples); + + private static void checkSampleRate(int sampleRate, String name) { + if (sampleRate < 100 || sampleRate > MAX_SAMPLE_RATE) { + throw new IllegalArgumentException("Invalid " + name + " sampling frequency: " + + sampleRate + ", must be between 100 and " + MAX_SAMPLE_RATE); + } + } + + private static void checkChannels(int channels, String name) { + if (channels <= 0 || channels > MAX_CHANNELS) { + throw new IllegalArgumentException("Invalid number of " + name + " channels: " + + channels + ", must be between 1 and " + MAX_CHANNELS); + } + } } diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioConverterTest.java b/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioConverterTest.java index c00e0b05..4b39e4be 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioConverterTest.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioConverterTest.java @@ -16,12 +16,21 @@ package dev.onvoid.webrtc.media.audio; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Arrays; import org.junit.jupiter.api.Test; class AudioConverterTest { + private static final byte UNTOUCHED = 0x55; + @Test void downSample() { convert(new ProcessBuffer(48000, 44100, 1, 1)); @@ -72,6 +81,130 @@ void targetBufferUnderflow() { }); } + @Test + void stereoDownSample() { + // 320 bytes per output frame, the converter used to write 640. + assertBounded(16000, 2, 8000, 2); + } + + @Test + void stereoUpSample() { + assertBounded(8000, 2, 16000, 2); + } + + @Test + void downMixResampleStereo() { + // Remixes to stereo, then resamples stereo. + assertBounded(16000, 4, 8000, 2); + } + + @Test + void resampleStereoUpMix() { + // Resamples stereo, then remixes it. + assertBounded(8000, 2, 16000, 4); + } + + @Test + void stereoKeepsChannels() { + AudioConverter converter = new AudioConverter(48000, 2, 16000, 2); + byte[] src = new byte[480 * 2 * 2]; + byte[] dst = new byte[converter.getTargetBufferSize()]; + + // A loud left and a silent right channel. + for (int i = 0; i < 480; i++) { + src[i * 4 + 1] = 0x40; + } + + // The resampler has a delay, so the signal shows only after a few frames. + for (int i = 0; i < 5; i++) { + converter.convert(src, dst); + } + + converter.dispose(); + + int last = 160 - 1; + + assertNotEquals(0, sample(dst, last * 2)); + assertEquals(0, sample(dst, last * 2 + 1)); + } + + @Test + void invalidFormats() { + assertThrows(IllegalArgumentException.class, () -> new AudioConverter(0, 1, 16000, 1)); + assertThrows(IllegalArgumentException.class, () -> new AudioConverter(48000, 1, 400000, 1)); + assertThrows(IllegalArgumentException.class, () -> new AudioConverter(48000, 0, 16000, 1)); + assertThrows(IllegalArgumentException.class, () -> new AudioConverter(48000, 1, 16000, 9)); + + // 1920 samples of 8 channels exceed the frame the channels are remixed in. + assertThrows(IllegalArgumentException.class, () -> new AudioConverter(192000, 8, 192000, 1)); + } + + @Test + void nativeBoundaryRejectsInvalidSizes() throws Exception { + // Bypass Java validation to exercise the checks protecting the native buffers. + Method convert = AudioConverter.class.getDeclaredMethod("convertInternal", + byte[].class, int.class, byte[].class, int.class); + convert.setAccessible(true); + + AudioConverter converter = new AudioConverter(16000, 2, 8000, 2); + int srcSamples = 160 * 2; + int dstSamples = 80 * 2; + byte[] src = new byte[srcSamples * 2]; + byte[] dst = new byte[dstSamples * 2]; + + try { + Object[][] invalid = { + { src, srcSamples - 1, dst, dstSamples }, + { new byte[src.length + 2], srcSamples + 1, dst, dstSamples }, + { src, srcSamples, dst, dstSamples - 1 }, + { new byte[src.length - 2], srcSamples, dst, dstSamples }, + { src, srcSamples, new byte[dst.length - 2], dstSamples }, + { src, -1, dst, dstSamples }, + { src, srcSamples, dst, -1 }, + }; + + for (Object[] args : invalid) { + InvocationTargetException error = assertThrows(InvocationTargetException.class, + () -> convert.invoke(converter, args)); + assertTrue(error.getCause() instanceof IllegalArgumentException, String.valueOf(error.getCause())); + } + + InvocationTargetException nullError = assertThrows(InvocationTargetException.class, + () -> convert.invoke(converter, null, srcSamples, dst, dstSamples)); + assertTrue(nullError.getCause() instanceof NullPointerException); + } + finally { + converter.dispose(); + } + } + + private static void assertBounded(int srcRate, int srcChannels, int dstRate, int dstChannels) { + AudioConverter converter = new AudioConverter(srcRate, srcChannels, dstRate, dstChannels); + int frameSizeOut = dstRate / 100 * dstChannels * 2; + + byte[] src = new byte[srcRate / 100 * srcChannels * 2]; + // As many guard bytes behind the frame as the frame has. + byte[] dst = new byte[frameSizeOut * 2]; + + Arrays.fill(dst, UNTOUCHED); + + int result = converter.convert(src, dst); + + converter.dispose(); + + assertEquals(frameSizeOut, converter.getTargetBufferSize()); + assertEquals(frameSizeOut / 2, result); + + for (int i = frameSizeOut; i < dst.length; i++) { + assertEquals(UNTOUCHED, dst[i], "Byte " + i + " was written"); + } + } + + private static short sample(byte[] samples, int index) { + // Little-endian 16-bit PCM. + return (short) ((samples[index * 2] & 0xFF) | (samples[index * 2 + 1] << 8)); + } + private static void convert(ProcessBuffer buffer) { AudioConverter converter = new AudioConverter(buffer.sampleRateIn, buffer.channelsIn, buffer.sampleRateOut, buffer.channelsOut);