From 7607cf36e6be1849900e61ce50ea324a4b757252 Mon Sep 17 00:00:00 2001 From: WentTheFox Date: Thu, 10 Sep 2026 12:36:38 +0200 Subject: [PATCH 1/9] fix(android/ios): cache AudioBuffer copy instead of recopying on every setBuffer AudioBufferSourceNodeHostObject::setBuffer() deep-copied the entire AudioBuffer on every `.buffer = x` reassignment, even when reassigning the exact same underlying buffer. That's exactly what seeking is. A live AudioBufferSourceNode can't be repositioned, so the standard pattern is to stop/disconnect it and create a fresh source node wrapping the already-decoded buffer at a new offset. Every seek therefore triggered a full, unnecessary copy of the whole track's PCM data. On a 4-minute stereo buffer this is a real full-size copy (~85MB) per seek. The old copy isn't freed until its replacement's scheduled audio event actually runs, so repeated rapid seeking accumulates full-size buffers faster than the audio thread can free the previous ones. The growing native heap eventually crashes the app via a Hermes GC OOM once it can no longer find room to grow the JS heap. This caches the defensive copy on the JS-visible AudioBufferHostObject and reuses it across repeated reassignments of the same buffer. The cache is invalidated only when the buffer's data could actually have been mutated: copyToChannel, or a live getChannelData() view having escaped to JS, since JS could write through that view at any later time. This keeps the exact copy-on-first-touch semantics the original code needed for pitch-correction and mutation safety, while making the "same buffer, many source nodes" pattern free after the first copy instead of paying for a fresh copy on every single reassignment. The caching logic lives in a new, standalone ImmutableBufferCache utility rather than directly in AudioBufferHostObject, because HostObjects/*.cpp is excluded from this project's C++ test suite (see common/cpp/test/CMakeLists.txt) and a plain utility class can be unit tested without a jsi::Runtime. Also documents this in the best-practices guide. The existing "reuse the same AudioBuffer across nodes" guidance was already correct, but silently expensive before this fix; it is now actually cheap as advertised. Verified: - The library's own C++ test suite still passes in full, plus 4 new unit tests for ImmutableBufferCache covering reuse, distinct-copy identity, and both invalidation paths (420/420 total). - A minimal repro app (creating a fresh AudioBufferSourceNode/GainNode pair wrapping the same AudioBuffer every ~150ms, simulating rapid seeking) went from ~71MB leaked per seek (visibly crashing within ~60 iterations) to no measurable per-seek growth across three consecutive 60-iteration runs, measured via `adb shell dumpsys meminfo` before/after/+30s-settled. - A real app using this pattern for playback seeking held native heap flat (Android, Samsung Galaxy S24+) across ~200 rapid seeks that previously crashed within a similar span. Fixes #1263 --- .../docs/fundamentals/best-practices.mdx | 3 + .../sources/AudioBufferHostObject.cpp | 10 +++ .../sources/AudioBufferHostObject.h | 17 ++++ .../AudioBufferSourceNodeHostObject.cpp | 12 ++- .../sources/AudioBufferSourceNodeHostObject.h | 4 +- .../audioapi/utils/ImmutableBufferCache.hpp | 59 ++++++++++++++ .../src/utils/ImmutableBufferCacheTest.cpp | 81 +++++++++++++++++++ 7 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp create mode 100644 packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp diff --git a/packages/audiodocs/docs/fundamentals/best-practices.mdx b/packages/audiodocs/docs/fundamentals/best-practices.mdx index 273c64896..fb9eda409 100644 --- a/packages/audiodocs/docs/fundamentals/best-practices.mdx +++ b/packages/audiodocs/docs/fundamentals/best-practices.mdx @@ -53,6 +53,8 @@ user experience, and maintainability. Here are some key best practices to consid - **Scheduled source nodes are single-use**: [`AudioBufferSourceNode`](../sources/audio-buffer-source-node.mdx), [`OscillatorNode`](../sources/oscillator-node.mdx), and other [`AudioScheduledSourceNode`](../sources/audio-scheduled-source-node.mdx) subclasses can be [`start()`](../sources/audio-scheduled-source-node.mdx#start)ed only once. Create a new node to replay a sound, but reuse the same [`AudioBuffer`](../sources/audio-buffer.mdx) — nodes are inexpensive to create. +- **Seeking**: since a started `AudioBufferSourceNode` can't be repositioned, seeking means stopping/disconnecting it and creating a fresh node with the same `AudioBuffer` at a new `start()` offset. Reassigning the same buffer this way is cheap. The underlying PCM data is copied once per `AudioBuffer`, not once per node, so recreating the source node on every seek doesn't re-copy it. + - **Use [`AudioBufferQueueSourceNode`](../sources/audio-buffer-queue-source-node.mdx) for chunked playback**: When audio arrives in segments (streaming TTS, progressive download), enqueue buffers into a queue source node rather than recreating the entire graph per chunk. ## [**AudioParam**](../core/audio-param.mdx) changes @@ -87,4 +89,5 @@ Prefer logging plain values instead: ```tsx console.log({ channelCount: node.channelCount, numberOfInputs: node.numberOfInputs }); ``` + ::: diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp index a2276c050..544af91c7 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp @@ -43,6 +43,13 @@ JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, numberOfChannels) { } JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { + // The returned Float32Array is a live, JS-writable view straight into + // audioBuffer_'s storage, and the caller can hold onto it indefinitely. + // Any cached copy handed to a source node from here on can no longer be + // trusted to stay in sync, so stop caching for the rest of this buffer's + // lifetime. + immutableCopyCache_.markLiveViewEscaped(); + auto channel = static_cast(args[0].getNumber()); auto audioArrayBuffer = audioBuffer_->getSharedChannel(channel); auto arrayBuffer = jsi::ArrayBuffer(runtime, audioArrayBuffer); @@ -76,6 +83,9 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, copyFromChannel) { } JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, copyToChannel) { + // Mutates audioBuffer_ in place, so any previously cached copy is now stale. + immutableCopyCache_.invalidate(); + auto arrayBuffer = args[0].getObject(runtime).getPropertyAsObject(runtime, "buffer").getArrayBuffer(runtime); auto *source = reinterpret_cast(arrayBuffer.data(runtime)); diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h index 04a463fb0..b7b552b86 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -34,6 +35,19 @@ class AudioBufferHostObject : public HostObject { return audioBuffer_->getSize() * audioBuffer_->getNumberOfChannels() * sizeof(float) * 2; } + /// @brief Returns a defensive copy of `audioBuffer_` suitable for handing to an + /// `AudioBufferSourceNode`, reusing a cached copy across repeated `.buffer = x` + /// reassignments of this same JS-visible buffer (e.g. seeking, which recreates the + /// source node but keeps reusing the already-decoded buffer). Without this, every + /// reassignment allocated a brand-new full-size copy, which is where + /// https://github.com/software-mansion/react-native-audio-api/issues/1263 came from. + /// @note The cache is invalidated whenever the buffer's data could have been mutated + /// from JS (`copyToChannel`, or ever having handed out a live `getChannelData` view), + /// since a cached copy must never be shared while its source can still be written to. + [[nodiscard]] std::shared_ptr getOrCreateImmutableCopy() { + return immutableCopyCache_.getOrCreate(audioBuffer_); + } + JSI_PROPERTY_GETTER_DECL(sampleRate); JSI_PROPERTY_GETTER_DECL(length); JSI_PROPERTY_GETTER_DECL(duration); @@ -42,5 +56,8 @@ class AudioBufferHostObject : public HostObject { JSI_HOST_FUNCTION_DECL(getChannelData); JSI_HOST_FUNCTION_DECL(copyFromChannel); JSI_HOST_FUNCTION_DECL(copyToChannel); + + private: + utils::ImmutableBufferCache immutableCopyCache_; }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp index 4bd5826c6..26d7980a5 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp @@ -147,13 +147,15 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, setBuffer) { thisValue.asObject(runtime).setExternalMemoryPressure( runtime, getMemoryPressure() + bufferHostObject->getSizeInBytes()); - setBuffer(bufferHostObject->audioBuffer_); + setBuffer(bufferHostObject->audioBuffer_, bufferHostObject); } return jsi::Value::undefined(); } -void AudioBufferSourceNodeHostObject::setBuffer(const std::shared_ptr &buffer) { +void AudioBufferSourceNodeHostObject::setBuffer( + const std::shared_ptr &buffer, + const std::shared_ptr &bufferHostObject) { // TODO: add optimized memory management for buffer changes, e.g. // when the same buffer is reused across threads and // buffer modification is not allowed on JS thread @@ -180,6 +182,12 @@ void AudioBufferSourceNodeHostObject::setBuffer(const std::shared_ptrgetNumberOfChannels(), buffer->getSampleRate()); copiedBuffer->copy(*buffer, 0, 0, buffer->getSize()); copiedBuffer->zero(buffer->getSize(), extraTailFrames); + } else if (bufferHostObject != nullptr) { + // Reuse a cached copy across repeated `.buffer = x` reassignments of the same + // JS-visible buffer (e.g. seeking, which recreates the source node but keeps + // reusing the already-decoded buffer) instead of deep-copying every time. + // See https://github.com/software-mansion/react-native-audio-api/issues/1263. + copiedBuffer = bufferHostObject->getOrCreateImmutableCopy(); } else { copiedBuffer = std::make_shared(*buffer); } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h index b3b480576..caacab3b1 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h @@ -51,7 +51,9 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje double loopStart_; double loopEnd_; - void setBuffer(const std::shared_ptr &buffer); + void setBuffer( + const std::shared_ptr &buffer, + const std::shared_ptr &bufferHostObject = nullptr); }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp b/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp new file mode 100644 index 000000000..d9a8139ae --- /dev/null +++ b/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp @@ -0,0 +1,59 @@ +#pragma once + +#include + +#include + +namespace audioapi::utils { + +/// @brief Caches a defensive copy of an `AudioBuffer` so repeated requests for +/// "an immutable copy suitable for handing to a source node" reuse the same +/// copy instead of allocating a fresh one every time. Used by +/// `AudioBufferHostObject` to avoid deep-copying the entire buffer on every +/// `.buffer = x` reassignment of the same underlying buffer. That reassignment +/// is what seeking requires, since a live `AudioBufferSourceNode` can't be +/// repositioned. See https://github.com/software-mansion/react-native-audio-api/issues/1263. +/// @note The two invalidation methods must be called whenever the source +/// buffer's data could have been mutated from JS. This class has no way to +/// observe that on its own. +class ImmutableBufferCache { + public: + /// @brief Returns a defensive copy of `source`, reusing the last copy + /// produced as long as neither `invalidate()` nor `markLiveViewEscaped()` + /// has been called since. + [[nodiscard]] std::shared_ptr getOrCreate( + const std::shared_ptr &source) { + if (liveViewEscaped_) { + // A live, JS-writable view into source's data has escaped. A write + // through it could happen at any later time, so we can no longer prove + // a cached copy won't go stale. Fall back to copying every time. + return std::make_shared(*source); + } + + if (cached_ == nullptr) { + cached_ = std::make_shared(*source); + } + return cached_; + } + + /// @brief Call when the source buffer's data has just been mutated in place + /// (e.g. `copyToChannel`). The current cached copy is now stale. + void invalidate() { + cached_ = nullptr; + } + + /// @brief Call when a live, JS-writable view into the source buffer's data + /// has been handed out (e.g. `getChannelData`). This permanently stops + /// caching, since the view can be written through at any later time, not + /// just at the moment it was retrieved. + void markLiveViewEscaped() { + liveViewEscaped_ = true; + cached_ = nullptr; + } + + private: + std::shared_ptr cached_; + bool liveViewEscaped_ = false; +}; + +} // namespace audioapi::utils diff --git a/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp b/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp new file mode 100644 index 000000000..e9c8429cc --- /dev/null +++ b/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp @@ -0,0 +1,81 @@ +#include +#include +#include +#include + +using namespace audioapi; +using namespace audioapi::utils; + +// NOLINTBEGIN + +namespace { + +constexpr size_t FRAME_COUNT = 1024; +constexpr int CHANNELS = 2; +constexpr float SAMPLE_RATE = 44100.0f; + +std::shared_ptr makeBuffer() { + return std::make_shared(FRAME_COUNT, CHANNELS, SAMPLE_RATE); +} + +} // namespace + +// The core of https://github.com/software-mansion/react-native-audio-api/issues/1263: +// repeated `.buffer = x` reassignment of the same underlying buffer (the seek pattern) +// must not allocate a fresh full-size copy every time. +TEST(ImmutableBufferCacheTest, ReusesCachedCopyAcrossRepeatedCalls) { + ImmutableBufferCache cache; + auto source = makeBuffer(); + + auto first = cache.getOrCreate(source); + auto second = cache.getOrCreate(source); + auto third = cache.getOrCreate(source); + + EXPECT_EQ(first, second) << "Second call should reuse the cached copy, not allocate a new one."; + EXPECT_EQ(second, third) << "Third call should reuse the same cached copy as well."; +} + +TEST(ImmutableBufferCacheTest, CachedCopyIsAnActualCopyNotTheOriginal) { + ImmutableBufferCache cache; + auto source = makeBuffer(); + + auto copy = cache.getOrCreate(source); + + EXPECT_NE(copy, source) << "The cached copy must be a distinct AudioBuffer instance. Sharing " + "the original directly would let a future mutation of it race with " + "a node concurrently reading the \"copy\"."; +} + +TEST(ImmutableBufferCacheTest, InvalidateForcesAFreshCopyOnce) { + ImmutableBufferCache cache; + auto source = makeBuffer(); + + auto first = cache.getOrCreate(source); + cache.invalidate(); + auto second = cache.getOrCreate(source); + auto third = cache.getOrCreate(source); + + EXPECT_NE(first, second) << "invalidate() means the source was just mutated in place, so the " + "previously cached copy is stale and must not be reused."; + EXPECT_EQ(second, third) << "After producing one fresh copy, subsequent calls should resume " + "caching normally rather than copying every time."; +} + +TEST(ImmutableBufferCacheTest, MarkLiveViewEscapedDisablesCachingPermanently) { + ImmutableBufferCache cache; + auto source = makeBuffer(); + + auto first = cache.getOrCreate(source); + cache.markLiveViewEscaped(); + auto second = cache.getOrCreate(source); + auto third = cache.getOrCreate(source); + + EXPECT_NE(first, second) << "A live, JS-writable view escaped, so the pre-existing cache entry " + "must be dropped since we can no longer prove it stayed in sync."; + EXPECT_NE(second, third) + << "Once a live view has ever escaped, every future call must fall back to a fresh, " + "uncached copy indefinitely. A write through that view could happen at any later " + "time, not just at the moment it was retrieved."; +} + +// NOLINTEND From 00f51c5f4fe65144ee23f78c5f456b71b692f288 Mon Sep 17 00:00:00 2001 From: michal Date: Fri, 11 Sep 2026 11:57:39 +0200 Subject: [PATCH 2/9] feat: acquire the content in absn --- .claude/skills/host-objects/SKILL.md | 2 + .../sources/AudioBufferHostObject.cpp | 51 ++++++++-- .../sources/AudioBufferHostObject.h | 33 ++++++- .../AudioBufferSourceNodeHostObject.cpp | 99 ++++++++++++------- .../sources/AudioBufferSourceNodeHostObject.h | 20 ++++ .../core/sources/AudioBufferSourceNode.cpp | 38 ++++--- .../core/sources/AudioBufferSourceNode.h | 16 +++ .../common/cpp/audioapi/utils/AudioBuffer.hpp | 8 ++ .../cpp/audioapi/utils/ImmutableBufferCache.h | 44 +++++++++ .../audioapi/utils/ImmutableBufferCache.hpp | 59 ----------- .../cpp/test/src/utils/AudioBufferTest.cpp | 21 ++++ .../src/utils/ImmutableBufferCacheTest.cpp | 19 +--- .../src/core/AudioBuffer.ts | 12 +++ .../src/core/AudioBufferSourceNode.ts | 9 ++ 14 files changed, 296 insertions(+), 135 deletions(-) create mode 100644 packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h delete mode 100644 packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp diff --git a/.claude/skills/host-objects/SKILL.md b/.claude/skills/host-objects/SKILL.md index 27d27bd66..7b56606f4 100644 --- a/.claude/skills/host-objects/SKILL.md +++ b/.claude/skills/host-objects/SKILL.md @@ -353,6 +353,8 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { } ``` +The view aliases native memory for as long as JS keeps it, and the `shared_ptr` inside the `jsi::ArrayBuffer` keeps that memory alive on its own. When the native side later needs the view to stop aliasing (Web Audio's "acquire the content" on `AudioBufferSourceNode.start()`), it must retain the returned object and neutralise it afterwards — see `detachReturnedChannelData` in the real `AudioBufferHostObject`. + ### External memory pressure Call `setExternalMemoryPressure` whenever returning a HostObject or typed array that wraps a large native buffer. This lets the JS GC schedule collection correctly: diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp index 544af91c7..787838d37 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace audioapi { @@ -24,7 +25,43 @@ AudioBufferHostObject::AudioBufferHostObject(const std::shared_ptr } AudioBufferHostObject::AudioBufferHostObject(AudioBufferHostObject &&other) noexcept - : HostObject(std::move(other)), audioBuffer_(std::move(other.audioBuffer_)) {} + : HostObject(std::move(other)), + audioBuffer_(std::move(other.audioBuffer_)), + immutableCopyCache_(std::move(other.immutableCopyCache_)), + returnedChannelDataArrays_(std::move(other.returnedChannelDataArrays_)) {} + +void AudioBufferHostObject::detachReturnedChannelData(jsi::Runtime &runtime) { + if (returnedChannelDataArrays_.empty()) { + return; + } + + auto defineProperty = runtime.global() + .getPropertyAsObject(runtime, "Object") + .getPropertyAsFunction(runtime, "defineProperty"); + auto zeroDescriptor = jsi::Object(runtime); + zeroDescriptor.setProperty(runtime, "value", 0); + + std::vector channelDetached(audioBuffer_->getNumberOfChannels(), false); + + for (const auto &returned : returnedChannelDataArrays_) { + auto array = returned.array.lock(runtime); + if (array.isObject()) { + for (const auto *sizeProperty : {"length", "byteLength", "byteOffset"}) { + defineProperty.call(runtime, array, sizeProperty, zeroDescriptor); + } + } + + if (!channelDetached[returned.channel]) { + audioBuffer_->detachSharedChannel(returned.channel); + channelDetached[returned.channel] = true; + } + } + + returnedChannelDataArrays_.clear(); + // A view could have been written through right up until now, i.e. after the cached + // copy was taken. + immutableCopyCache_.invalidate(); +} JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, sampleRate) { return {audioBuffer_->getSampleRate()}; @@ -44,13 +81,11 @@ JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, numberOfChannels) { JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { // The returned Float32Array is a live, JS-writable view straight into - // audioBuffer_'s storage, and the caller can hold onto it indefinitely. - // Any cached copy handed to a source node from here on can no longer be - // trusted to stay in sync, so stop caching for the rest of this buffer's - // lifetime. - immutableCopyCache_.markLiveViewEscaped(); + // audioBuffer_'s storage, so a copy cached before now can no longer be trusted. + // Caching resumes once the view is neutralised by detachReturnedChannelData(). + immutableCopyCache_.invalidate(); - auto channel = static_cast(args[0].getNumber()); + auto channel = static_cast(args[0].getNumber()); auto audioArrayBuffer = audioBuffer_->getSharedChannel(channel); auto arrayBuffer = jsi::ArrayBuffer(runtime, audioArrayBuffer); @@ -58,6 +93,8 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { auto float32Array = float32ArrayCtor.callAsConstructor(runtime, arrayBuffer).getObject(runtime); float32Array.setExternalMemoryPressure(runtime, audioArrayBuffer->size()); + returnedChannelDataArrays_.push_back( + {.channel = channel, .array = jsi::WeakObject(runtime, float32Array)}); return float32Array; } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h index b7b552b86..bcaaa36b2 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h @@ -2,12 +2,13 @@ #include #include -#include +#include #include #include #include #include +#include namespace audioapi { using namespace facebook; @@ -24,6 +25,8 @@ class AudioBufferHostObject : public HostObject { if (this != &other) { HostObject::operator=(std::move(other)); audioBuffer_ = std::move(other.audioBuffer_); + immutableCopyCache_ = std::move(other.immutableCopyCache_); + returnedChannelDataArrays_ = std::move(other.returnedChannelDataArrays_); } return *this; } @@ -41,13 +44,27 @@ class AudioBufferHostObject : public HostObject { /// source node but keeps reusing the already-decoded buffer). Without this, every /// reassignment allocated a brand-new full-size copy, which is where /// https://github.com/software-mansion/react-native-audio-api/issues/1263 came from. - /// @note The cache is invalidated whenever the buffer's data could have been mutated - /// from JS (`copyToChannel`, or ever having handed out a live `getChannelData` view), - /// since a cached copy must never be shared while its source can still be written to. + /// @note The cache is dropped whenever `audioBuffer_` may have diverged from it: + /// `copyToChannel` mutates in place, `getChannelData` hands out a live JS-writable + /// view, and `detachReturnedChannelData` is the last moment such a view could have + /// been written through. [[nodiscard]] std::shared_ptr getOrCreateImmutableCopy() { return immutableCopyCache_.getOrCreate(audioBuffer_); } + /// @brief Web Audio's "acquire the content" step for the views handed out by + /// `getChannelData`. Call once playback of this buffer has been scheduled. Every + /// previously returned Float32Array stops aliasing `audioBuffer_` and, if JS still + /// holds it, reads as zero-length; the next `getChannelData` call hands out a fresh + /// view, mirroring what a browser does when it detaches those ArrayBuffers. + void detachReturnedChannelData(jsi::Runtime &runtime); + + /// @brief Whether any `getChannelData` view is live, i.e. handed out since the last + /// `detachReturnedChannelData`. + [[nodiscard]] bool hasReturnedChannelData() const { + return !returnedChannelDataArrays_.empty(); + } + JSI_PROPERTY_GETTER_DECL(sampleRate); JSI_PROPERTY_GETTER_DECL(length); JSI_PROPERTY_GETTER_DECL(duration); @@ -58,6 +75,14 @@ class AudioBufferHostObject : public HostObject { JSI_HOST_FUNCTION_DECL(copyToChannel); private: + struct ReturnedChannelDataArray { + size_t channel; + jsi::WeakObject array; + }; + utils::ImmutableBufferCache immutableCopyCache_; + /// Float32Array views handed out by `getChannelData` since the last + /// `detachReturnedChannelData`, kept so they can be neutralised then. + std::vector returnedChannelDataArrays_; }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp index 26d7980a5..eb2de3581 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp @@ -126,6 +126,9 @@ JSI_PROPERTY_SETTER_IMPL(AudioBufferSourceNodeHostObject, onLoopEnded) { } JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, start) { + hasBeenStarted_ = true; + acquireBufferContent(runtime); + auto handle = node_->handle; auto event = [handle, node = audioBufferSourceNode_, @@ -139,6 +142,22 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, start) { return jsi::Value::undefined(); } +void AudioBufferSourceNodeHostObject::acquireBufferContent(jsi::Runtime &runtime) { + if (bufferHostObject_ == nullptr || !bufferHostObject_->hasReturnedChannelData()) { + return; + } + + bufferHostObject_->detachReturnedChannelData(runtime); + // The copy handed to the node in setBuffer() predates any writes made through those + // views since, so hand it the post-write content that has now been fenced off. + auto buffers = prepareNodeBuffers(bufferHostObject_->audioBuffer_, bufferHostObject_); + auto event = + [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { + node->replaceBufferContent(buffers.copiedBuffer, buffers.audioBuffer); + }; + audioBufferSourceNode_->scheduleAudioEvent(std::move(event)); +} + JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, setBuffer) { if (args[0].isNull()) { setBuffer(nullptr); @@ -150,61 +169,73 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, setBuffer) { setBuffer(bufferHostObject->audioBuffer_, bufferHostObject); } + // Per Web Audio, assigning a buffer to an already-started source acquires its + // content right away, because start() had nothing to acquire back then. + if (hasBeenStarted_) { + acquireBufferContent(runtime); + } + return jsi::Value::undefined(); } -void AudioBufferSourceNodeHostObject::setBuffer( +AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::prepareNodeBuffers( const std::shared_ptr &buffer, const std::shared_ptr &bufferHostObject) { // TODO: add optimized memory management for buffer changes, e.g. // when the same buffer is reused across threads and // buffer modification is not allowed on JS thread - auto handle = node_->handle; - - std::shared_ptr copiedBuffer; - std::shared_ptr audioBuffer; - const size_t newChannelCount = buffer == nullptr ? AudioBufferSourceOptions::kDefaultChannelCount - : buffer->getNumberOfChannels(); + NodeBuffers buffers; if (buffer == nullptr) { - copiedBuffer = nullptr; - audioBuffer = std::make_shared( + buffers.copiedBuffer = nullptr; + buffers.audioBuffer = std::make_shared( RENDER_QUANTUM_SIZE, AudioBufferSourceOptions::kDefaultChannelCount, audioBufferSourceNode_->getContextSampleRate()); + return buffers; + } + + if (pitchCorrection_) { + initStretch(static_cast(buffer->getNumberOfChannels()), buffer->getSampleRate()); + auto extraTailFrames = + static_cast((inputLatency_ + outputLatency_) * buffer->getSampleRate()); + size_t totalSize = buffer->getSize() + extraTailFrames; + buffers.copiedBuffer = std::make_shared( + totalSize, buffer->getNumberOfChannels(), buffer->getSampleRate()); + buffers.copiedBuffer->copy(*buffer, 0, 0, buffer->getSize()); + buffers.copiedBuffer->zero(buffer->getSize(), extraTailFrames); + } else if (bufferHostObject != nullptr) { + // Reuse a cached copy across repeated `.buffer = x` reassignments of the same + // JS-visible buffer (e.g. seeking, which recreates the source node but keeps + // reusing the already-decoded buffer) instead of deep-copying every time. + // See https://github.com/software-mansion/react-native-audio-api/issues/1263. + buffers.copiedBuffer = bufferHostObject->getOrCreateImmutableCopy(); } else { - if (pitchCorrection_) { - initStretch(static_cast(buffer->getNumberOfChannels()), buffer->getSampleRate()); - auto extraTailFrames = - static_cast((inputLatency_ + outputLatency_) * buffer->getSampleRate()); - size_t totalSize = buffer->getSize() + extraTailFrames; - copiedBuffer = std::make_shared( - totalSize, buffer->getNumberOfChannels(), buffer->getSampleRate()); - copiedBuffer->copy(*buffer, 0, 0, buffer->getSize()); - copiedBuffer->zero(buffer->getSize(), extraTailFrames); - } else if (bufferHostObject != nullptr) { - // Reuse a cached copy across repeated `.buffer = x` reassignments of the same - // JS-visible buffer (e.g. seeking, which recreates the source node but keeps - // reusing the already-decoded buffer) instead of deep-copying every time. - // See https://github.com/software-mansion/react-native-audio-api/issues/1263. - copiedBuffer = bufferHostObject->getOrCreateImmutableCopy(); - } else { - copiedBuffer = std::make_shared(*buffer); - } - - audioBuffer = std::make_shared( - RENDER_QUANTUM_SIZE, - copiedBuffer->getNumberOfChannels(), - audioBufferSourceNode_->getContextSampleRate()); + buffers.copiedBuffer = std::make_shared(*buffer); } + buffers.audioBuffer = std::make_shared( + RENDER_QUANTUM_SIZE, + buffers.copiedBuffer->getNumberOfChannels(), + audioBufferSourceNode_->getContextSampleRate()); + return buffers; +} + +void AudioBufferSourceNodeHostObject::setBuffer( + const std::shared_ptr &buffer, + const std::shared_ptr &bufferHostObject) { + bufferHostObject_ = bufferHostObject; + auto buffers = prepareNodeBuffers(buffer, bufferHostObject); + // Update channelCount on the host thread before renegotiation so MAX / // CLAMPED_MAX downstream nodes see the new width immediately. + const size_t newChannelCount = buffer == nullptr ? AudioBufferSourceOptions::kDefaultChannelCount + : buffer->getNumberOfChannels(); updateChannelCount(newChannelCount); auto event = - [handle, node = audioBufferSourceNode_, copiedBuffer, audioBuffer](BaseAudioContext &) { - node->setBuffer(copiedBuffer, audioBuffer); + [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { + node->setBuffer(buffers.copiedBuffer, buffers.audioBuffer); }; audioBufferSourceNode_->scheduleAudioEvent(std::move(event)); } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h index caacab3b1..a4d2fec5f 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h @@ -51,9 +51,29 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje double loopStart_; double loopEnd_; + /// The JS-visible buffer behind the last `setBuffer`, kept so the "acquire the + /// content" step can run on it. Null when the buffer came from options or was cleared. + std::shared_ptr bufferHostObject_; + bool hasBeenStarted_ = false; + + struct NodeBuffers { + std::shared_ptr copiedBuffer; + std::shared_ptr audioBuffer; + }; + + NodeBuffers prepareNodeBuffers( + const std::shared_ptr &buffer, + const std::shared_ptr &bufferHostObject); + void setBuffer( const std::shared_ptr &buffer, const std::shared_ptr &bufferHostObject = nullptr); + + /// Web Audio's "acquire the content" step: runs on start() when a buffer is set, and on + /// setBuffer() once already started. Cuts off every live getChannelData() view and + /// re-hands the node the fenced-off content. + /// https://webaudio.github.io/web-audio-api/#acquire-the-content + void acquireBufferContent(jsi::Runtime &runtime); }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp index 24dbf20f6..978eeeb21 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp @@ -55,10 +55,33 @@ void AudioBufferSourceNode::setLoopEnd(double loopEnd) { void AudioBufferSourceNode::setBuffer( const std::shared_ptr &buffer, const std::shared_ptr &audioBuffer) { + if (!swapBuffers(buffer, audioBuffer)) { + return; + } + + if (buffer_ == nullptr) { + loopEnd_ = 0; + channelCount_ = AudioBufferSourceOptions::kDefaultChannelCount; + return; + } + + channelCount_ = static_cast(buffer_->getNumberOfChannels()); + loopEnd_ = buffer_->getDuration(); +} + +void AudioBufferSourceNode::replaceBufferContent( + const std::shared_ptr &buffer, + const std::shared_ptr &audioBuffer) { + swapBuffers(buffer, audioBuffer); +} + +bool AudioBufferSourceNode::swapBuffers( + const std::shared_ptr &buffer, + const std::shared_ptr &audioBuffer) { std::shared_ptr context = context_.lock(); if (context == nullptr) { - return; + return false; } if (buffer_ != nullptr) { @@ -69,21 +92,10 @@ void AudioBufferSourceNode::setBuffer( context->getDisposer()->dispose(std::move(audioBuffer_)); } - if (buffer == nullptr) { - loopEnd_ = 0; - channelCount_ = AudioBufferSourceOptions::kDefaultChannelCount; - - buffer_ = nullptr; - processor_->setBuffer(nullptr); - audioBuffer_ = audioBuffer; - return; - } - buffer_ = buffer; audioBuffer_ = audioBuffer; - channelCount_ = static_cast(buffer_->getNumberOfChannels()); - loopEnd_ = buffer_->getDuration(); processor_->setBuffer(buffer_); + return true; } void AudioBufferSourceNode::start(double when, double offset, double duration) { diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h index 37ae6c803..380c853cd 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h @@ -29,12 +29,22 @@ class AudioBufferSourceNode : public AudioBufferBaseSourceNode { /// @note Audio Thread only void setLoopEnd(double loopEnd); + [[nodiscard]] double getLoopEnd() const { + return loopEnd_; + } /// @note Audio Thread only void setBuffer( const std::shared_ptr &buffer, const std::shared_ptr &audioBuffer); + /// @brief Swaps in a buffer holding the same frames, channels and sample rate as the + /// current one, keeping loop bounds and channel count untouched. This is the "acquire + /// the content" refresh: the samples may have changed since setBuffer(), the shape has not. + void replaceBufferContent( + const std::shared_ptr &buffer, + const std::shared_ptr &audioBuffer); + using AudioScheduledSourceNode::start; /// @note Audio Thread only void start(double when, double offset, double duration = -1); @@ -73,6 +83,12 @@ class AudioBufferSourceNode : public AudioBufferBaseSourceNode { double getVirtualEndFrame(float sampleRate); std::unique_ptr processor_; + + /// Hands the old buffers to the disposer and installs the new ones. Returns false when + /// the context is already gone. + bool swapBuffers( + const std::shared_ptr &buffer, + const std::shared_ptr &audioBuffer); }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp index 8f32141fa..95279272b 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp @@ -141,6 +141,14 @@ class AlignedAudioBuffer { return channels_[index]; } + /// @brief Gives channel @p index fresh storage holding a copy of its current samples. + /// Every handle previously obtained through getSharedChannel() keeps the old storage + /// alive but no longer aliases this buffer, so writes through it can't reach us anymore. + /// This is how a JS `getChannelData` view gets cut off once playback acquires the buffer. + void detachSharedChannel(size_t index) { + channels_[index] = std::make_shared>(*channels_[index]); + } + AlignedAudioArray &operator[](size_t index) { return *channels_[index]; } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h b/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h new file mode 100644 index 000000000..b6207fcf5 --- /dev/null +++ b/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h @@ -0,0 +1,44 @@ +#pragma once + +#include + +#include + +namespace audioapi::utils { + +/// @brief Caches a defensive copy of an `AudioBuffer` so repeated requests for +/// "an immutable copy suitable for handing to a source node" reuse the same +/// copy instead of allocating a fresh one every time. Used by +/// `AudioBufferHostObject` to avoid deep-copying the entire buffer on every +/// `.buffer = x` reassignment of the same underlying buffer. That reassignment +/// is what seeking requires, since a live `AudioBufferSourceNode` can't be +/// repositioned. See https://github.com/software-mansion/react-native-audio-api/issues/1263. +/// @note `invalidate()` must be called whenever the source buffer's data could +/// have diverged from the cached copy. This class has no way to observe that on +/// its own. +class ImmutableBufferCache { + public: + /// @brief Returns a defensive copy of `source`, reusing the last copy + /// produced as long as `invalidate()` has not been called since. + [[nodiscard]] std::shared_ptr getOrCreate( + const std::shared_ptr &source) { + if (cached_ == nullptr) { + cached_ = std::make_shared(*source); + } + return cached_; + } + + /// @brief Call when the source buffer's data may no longer match the cached + /// copy: it was mutated in place (`copyToChannel`), a live JS-writable view + /// into it was handed out (`getChannelData`), or such a view has just been + /// cut off after possibly being written through. The next `getOrCreate` + /// produces a fresh copy and caching resumes from there. + void invalidate() { + cached_ = nullptr; + } + + private: + std::shared_ptr cached_; +}; + +} // namespace audioapi::utils diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp b/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp deleted file mode 100644 index d9a8139ae..000000000 --- a/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.hpp +++ /dev/null @@ -1,59 +0,0 @@ -#pragma once - -#include - -#include - -namespace audioapi::utils { - -/// @brief Caches a defensive copy of an `AudioBuffer` so repeated requests for -/// "an immutable copy suitable for handing to a source node" reuse the same -/// copy instead of allocating a fresh one every time. Used by -/// `AudioBufferHostObject` to avoid deep-copying the entire buffer on every -/// `.buffer = x` reassignment of the same underlying buffer. That reassignment -/// is what seeking requires, since a live `AudioBufferSourceNode` can't be -/// repositioned. See https://github.com/software-mansion/react-native-audio-api/issues/1263. -/// @note The two invalidation methods must be called whenever the source -/// buffer's data could have been mutated from JS. This class has no way to -/// observe that on its own. -class ImmutableBufferCache { - public: - /// @brief Returns a defensive copy of `source`, reusing the last copy - /// produced as long as neither `invalidate()` nor `markLiveViewEscaped()` - /// has been called since. - [[nodiscard]] std::shared_ptr getOrCreate( - const std::shared_ptr &source) { - if (liveViewEscaped_) { - // A live, JS-writable view into source's data has escaped. A write - // through it could happen at any later time, so we can no longer prove - // a cached copy won't go stale. Fall back to copying every time. - return std::make_shared(*source); - } - - if (cached_ == nullptr) { - cached_ = std::make_shared(*source); - } - return cached_; - } - - /// @brief Call when the source buffer's data has just been mutated in place - /// (e.g. `copyToChannel`). The current cached copy is now stale. - void invalidate() { - cached_ = nullptr; - } - - /// @brief Call when a live, JS-writable view into the source buffer's data - /// has been handed out (e.g. `getChannelData`). This permanently stops - /// caching, since the view can be written through at any later time, not - /// just at the moment it was retrieved. - void markLiveViewEscaped() { - liveViewEscaped_ = true; - cached_ = nullptr; - } - - private: - std::shared_ptr cached_; - bool liveViewEscaped_ = false; -}; - -} // namespace audioapi::utils diff --git a/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp b/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp index 6f458af68..300ff9ca7 100644 --- a/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp +++ b/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp @@ -634,4 +634,25 @@ TEST_F(AudioBufferTest, DeinterleaveZeroFramesIsNoop) { expectChannel(buf, 0, 42.0f); } +TEST_F(AudioBufferTest, DetachSharedChannelCutsOffEscapedHandleAndKeepsSamples) { + AudioBuffer buf(BUF_SIZE, 2, SR); + fillChannel(buf, 0, 0.5f); + fillChannel(buf, 1, 0.25f); + auto escapedHandle = buf.getSharedChannel(0); + auto untouchedHandle = buf.getSharedChannel(1); + + buf.detachSharedChannel(0); + + EXPECT_NE(buf.getSharedChannel(0), escapedHandle) << "Detached channel must get fresh storage."; + EXPECT_EQ(buf.getSharedChannel(1), untouchedHandle) + << "Other channels keep their storage; only the escaped one is replaced."; + expectChannel(buf, 0, 0.5f); + + (*escapedHandle)[3] = 1.0f; + + expectChannel(buf, 0, 0.5f); + EXPECT_FLOAT_EQ((*escapedHandle)[3], 1.0f) + << "The old handle stays alive and writable, it just no longer reaches the buffer."; +} + // NOLINTEND diff --git a/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp b/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp index e9c8429cc..070f89763 100644 --- a/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp +++ b/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include @@ -61,21 +61,4 @@ TEST(ImmutableBufferCacheTest, InvalidateForcesAFreshCopyOnce) { "caching normally rather than copying every time."; } -TEST(ImmutableBufferCacheTest, MarkLiveViewEscapedDisablesCachingPermanently) { - ImmutableBufferCache cache; - auto source = makeBuffer(); - - auto first = cache.getOrCreate(source); - cache.markLiveViewEscaped(); - auto second = cache.getOrCreate(source); - auto third = cache.getOrCreate(source); - - EXPECT_NE(first, second) << "A live, JS-writable view escaped, so the pre-existing cache entry " - "must be dropped since we can no longer prove it stayed in sync."; - EXPECT_NE(second, third) - << "Once a live view has ever escaped, every future call must fall back to a fresh, " - "uncached copy indefinitely. A write through that view could happen at any later " - "time, not just at the moment it was retrieved."; -} - // NOLINTEND diff --git a/packages/react-native-audio-api/src/core/AudioBuffer.ts b/packages/react-native-audio-api/src/core/AudioBuffer.ts index b73fcf441..385d61ee2 100644 --- a/packages/react-native-audio-api/src/core/AudioBuffer.ts +++ b/packages/react-native-audio-api/src/core/AudioBuffer.ts @@ -56,6 +56,18 @@ export default class AudioBuffer implements AudioBufferLike { return data; } + /** + * Forgets the cached channel views once the native side has cut them off from + * the buffer (a source node acquired this buffer's content), so the next + * `getChannelData` hands out a fresh view the way a browser does after it + * detaches the old ones. + * + * @internal + */ + public invalidateChannelDataCache(): void { + this.channelDataCache.length = 0; + } + public copyFromChannel( destination: Float32Array, channelNumber: number, diff --git a/packages/react-native-audio-api/src/core/AudioBufferSourceNode.ts b/packages/react-native-audio-api/src/core/AudioBufferSourceNode.ts index e855e7a5b..b70699b7a 100644 --- a/packages/react-native-audio-api/src/core/AudioBufferSourceNode.ts +++ b/packages/react-native-audio-api/src/core/AudioBufferSourceNode.ts @@ -53,6 +53,12 @@ export default class AudioBufferSourceNode extends AudioBufferBaseSourceNode { (this.node as IAudioBufferSourceNode).setBuffer(buffer.buffer); this._buffer = buffer; this.bufferHasBeenSet = true; + + if (this.hasBeenStarted) { + // Assigning a buffer to an already-started source acquires its content right + // away, so native has just cut off the views this buffer handed out. + buffer.invalidateChannelDataCache(); + } } public get loopSkip(): boolean { @@ -112,6 +118,9 @@ export default class AudioBufferSourceNode extends AudioBufferBaseSourceNode { this.hasBeenStarted = true; (this.node as IAudioBufferSourceNode).start(when, offset, duration); + // Native cut off every view handed out by getChannelData() while acquiring the + // buffer's content, so the wrapper must stop returning those dead views. + this._buffer?.invalidateChannelDataCache(); this.context.markRunningOnSourceStart(); } From 0ca46b8458b253957b73c36635326b9f84973d47 Mon Sep 17 00:00:00 2001 From: michal Date: Fri, 11 Sep 2026 13:25:41 +0200 Subject: [PATCH 3/9] refactor: new model of acquiring the buffer --- .../sources/AudioBufferHostObject.cpp | 42 ++++++++---- .../sources/AudioBufferHostObject.h | 48 +++++++++----- .../AudioBufferSourceNodeHostObject.cpp | 47 ++++++++------ .../sources/AudioBufferSourceNodeHostObject.h | 12 +++- .../common/cpp/audioapi/utils/AudioBuffer.hpp | 31 ++++++--- .../cpp/audioapi/utils/ImmutableBufferCache.h | 44 ------------- .../cpp/test/src/utils/AudioBufferTest.cpp | 24 +++++++ .../src/utils/ImmutableBufferCacheTest.cpp | 64 ------------------- 8 files changed, 145 insertions(+), 167 deletions(-) delete mode 100644 packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h delete mode 100644 packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp index 787838d37..2eef2ae6b 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp @@ -27,9 +27,30 @@ AudioBufferHostObject::AudioBufferHostObject(const std::shared_ptr AudioBufferHostObject::AudioBufferHostObject(AudioBufferHostObject &&other) noexcept : HostObject(std::move(other)), audioBuffer_(std::move(other.audioBuffer_)), - immutableCopyCache_(std::move(other.immutableCopyCache_)), + channelSharedWithNode_(std::move(other.channelSharedWithNode_)), + contentVersion_(other.contentVersion_), returnedChannelDataArrays_(std::move(other.returnedChannelDataArrays_)) {} +std::shared_ptr AudioBufferHostObject::shareForPlayback() { + channelSharedWithNode_.assign(audioBuffer_->getNumberOfChannels(), true); + return audioBuffer_->shareChannels(); +} + +void AudioBufferHostObject::makeChannelWritable(size_t channel) { + if (channel < channelSharedWithNode_.size() && channelSharedWithNode_[channel]) { + replaceChannelStorage(channel); + } +} + +void AudioBufferHostObject::replaceChannelStorage(size_t channel) { + // mark the channel as no longer shared with a node, so that future writes to it don't trigger another copy-on-write + if (channel < channelSharedWithNode_.size()) { + audioBuffer_->detachSharedChannel(channel); + channelSharedWithNode_[channel] = false; + ++contentVersion_; + } +} + void AudioBufferHostObject::detachReturnedChannelData(jsi::Runtime &runtime) { if (returnedChannelDataArrays_.empty()) { return; @@ -52,15 +73,12 @@ void AudioBufferHostObject::detachReturnedChannelData(jsi::Runtime &runtime) { } if (!channelDetached[returned.channel]) { - audioBuffer_->detachSharedChannel(returned.channel); + replaceChannelStorage(returned.channel); channelDetached[returned.channel] = true; } } returnedChannelDataArrays_.clear(); - // A view could have been written through right up until now, i.e. after the cached - // copy was taken. - immutableCopyCache_.invalidate(); } JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, sampleRate) { @@ -80,12 +98,11 @@ JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, numberOfChannels) { } JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { - // The returned Float32Array is a live, JS-writable view straight into - // audioBuffer_'s storage, so a copy cached before now can no longer be trusted. - // Caching resumes once the view is neutralised by detachReturnedChannelData(). - immutableCopyCache_.invalidate(); - auto channel = static_cast(args[0].getNumber()); + // The returned Float32Array is a live, JS-writable view straight into audioBuffer_'s + // storage, so that storage must not be one a node is playing. + makeChannelWritable(channel); + auto audioArrayBuffer = audioBuffer_->getSharedChannel(channel); auto arrayBuffer = jsi::ArrayBuffer(runtime, audioArrayBuffer); @@ -120,14 +137,13 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, copyFromChannel) { } JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, copyToChannel) { - // Mutates audioBuffer_ in place, so any previously cached copy is now stale. - immutableCopyCache_.invalidate(); - auto arrayBuffer = args[0].getObject(runtime).getPropertyAsObject(runtime, "buffer").getArrayBuffer(runtime); auto *source = reinterpret_cast(arrayBuffer.data(runtime)); auto sourceLength = arrayBuffer.size(runtime) / sizeof(float); auto channelNumber = static_cast(args[1].getNumber()); + // Mutates audioBuffer_ in place, so that channel must not be one a node is playing. + makeChannelWritable(static_cast(channelNumber)); auto rawStart = args[2].getNumber(); auto channelSize = audioBuffer_->getSize(); diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h index bcaaa36b2..6c4a787f2 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h @@ -2,10 +2,10 @@ #include #include -#include #include #include +#include #include #include #include @@ -13,6 +13,10 @@ namespace audioapi { using namespace facebook; +/// @brief JS-facing AudioBuffer. Source nodes play its channel storage directly instead of +/// deep-copying it (see `shareForPlayback`), so the one rule this class enforces is that the +/// JS thread never writes into storage a node may be reading: every write path first gives +/// the touched channel fresh storage (copy-on-write) and leaves the old one to the nodes. class AudioBufferHostObject : public HostObject { public: std::shared_ptr audioBuffer_; @@ -25,7 +29,8 @@ class AudioBufferHostObject : public HostObject { if (this != &other) { HostObject::operator=(std::move(other)); audioBuffer_ = std::move(other.audioBuffer_); - immutableCopyCache_ = std::move(other.immutableCopyCache_); + channelSharedWithNode_ = std::move(other.channelSharedWithNode_); + contentVersion_ = other.contentVersion_; returnedChannelDataArrays_ = std::move(other.returnedChannelDataArrays_); } return *this; @@ -34,22 +39,17 @@ class AudioBufferHostObject : public HostObject { ~AudioBufferHostObject() override = default; [[nodiscard]] size_t getSizeInBytes() const { - // *2 because every time buffer is passed we create a copy of it. - return audioBuffer_->getSize() * audioBuffer_->getNumberOfChannels() * sizeof(float) * 2; + return audioBuffer_->getSize() * audioBuffer_->getNumberOfChannels() * sizeof(float); } - /// @brief Returns a defensive copy of `audioBuffer_` suitable for handing to an - /// `AudioBufferSourceNode`, reusing a cached copy across repeated `.buffer = x` - /// reassignments of this same JS-visible buffer (e.g. seeking, which recreates the - /// source node but keeps reusing the already-decoded buffer). Without this, every - /// reassignment allocated a brand-new full-size copy, which is where - /// https://github.com/software-mansion/react-native-audio-api/issues/1263 came from. - /// @note The cache is dropped whenever `audioBuffer_` may have diverged from it: - /// `copyToChannel` mutates in place, `getChannelData` hands out a live JS-writable - /// view, and `detachReturnedChannelData` is the last moment such a view could have - /// been written through. - [[nodiscard]] std::shared_ptr getOrCreateImmutableCopy() { - return immutableCopyCache_.getOrCreate(audioBuffer_); + /// @brief from this call both the js and native side can read the same channel storage. + /// The JS side is copy-on-write, so it will get a fresh copy if it writes into it while a node is reading it. + [[nodiscard]] std::shared_ptr shareForPlayback(); + + /// @brief Bumped every time a channel's storage is replaced. A source node compares it + /// with the version it shared at to decide whether "acquire the content" must re-share. + [[nodiscard]] uint64_t getContentVersion() const { + return contentVersion_; } /// @brief Web Audio's "acquire the content" step for the views handed out by @@ -77,10 +77,24 @@ class AudioBufferHostObject : public HostObject { private: struct ReturnedChannelDataArray { size_t channel; + /// Weak on purpose: a view JS already dropped must not be kept alive (nor keep its + /// external-memory-pressure hint alive) until the next start(). The channel is still + /// recorded so its storage gets swapped, since wrappers over the same ArrayBuffer may + /// outlive this particular Float32Array object. jsi::WeakObject array; }; - utils::ImmutableBufferCache immutableCopyCache_; + /// Copy-on-write: call before exposing or mutating a channel from JS. If a node may be + /// reading that channel's storage, the buffer gets a private copy of it first. + void makeChannelWritable(size_t channel); + + /// Replaces the channel's storage with a copy and records that nothing shares it yet. + void replaceChannelStorage(size_t channel); + + /// One flag per channel: true while a node handed out by `shareForPlayback` may still + /// be reading that channel's current storage. + std::vector channelSharedWithNode_; + uint64_t contentVersion_ = 0; /// Float32Array views handed out by `getChannelData` since the last /// `detachReturnedChannelData`, kept so they can be neutralised then. std::vector returnedChannelDataArrays_; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp index eb2de3581..f49decd3d 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp @@ -143,17 +143,25 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, start) { } void AudioBufferSourceNodeHostObject::acquireBufferContent(jsi::Runtime &runtime) { - if (bufferHostObject_ == nullptr || !bufferHostObject_->hasReturnedChannelData()) { + if (bufferHostObject_ == nullptr) { return; } bufferHostObject_->detachReturnedChannelData(runtime); - // The copy handed to the node in setBuffer() predates any writes made through those - // views since, so hand it the post-write content that has now been fenced off. + + if (bufferHostObject_->getContentVersion() == sharedContentVersion_) { + // Nothing replaced the storage the node already reads, so it holds the content + // exactly as acquired. + return; + } + + // Some channel storage could be swapped since the node received its samples, either by + // the detach above or by an earlier copy-on-write. Re-hand it the current content, + // touching only the samples so loopEnd and friends survive. auto buffers = prepareNodeBuffers(bufferHostObject_->audioBuffer_, bufferHostObject_); auto event = [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { - node->replaceBufferContent(buffers.copiedBuffer, buffers.audioBuffer); + node->replaceBufferContent(buffers.nodeBuffer, buffers.audioBuffer); }; audioBufferSourceNode_->scheduleAudioEvent(std::move(event)); } @@ -181,13 +189,10 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, setBuffer) { AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::prepareNodeBuffers( const std::shared_ptr &buffer, const std::shared_ptr &bufferHostObject) { - // TODO: add optimized memory management for buffer changes, e.g. - // when the same buffer is reused across threads and - // buffer modification is not allowed on JS thread NodeBuffers buffers; if (buffer == nullptr) { - buffers.copiedBuffer = nullptr; + buffers.nodeBuffer = nullptr; buffers.audioBuffer = std::make_shared( RENDER_QUANTUM_SIZE, AudioBufferSourceOptions::kDefaultChannelCount, @@ -196,27 +201,31 @@ AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::pr } if (pitchCorrection_) { + // The stretcher needs tail padding past the end of the samples, so this path keeps + // its own padded copy. initStretch(static_cast(buffer->getNumberOfChannels()), buffer->getSampleRate()); auto extraTailFrames = static_cast((inputLatency_ + outputLatency_) * buffer->getSampleRate()); size_t totalSize = buffer->getSize() + extraTailFrames; - buffers.copiedBuffer = std::make_shared( + buffers.nodeBuffer = std::make_shared( totalSize, buffer->getNumberOfChannels(), buffer->getSampleRate()); - buffers.copiedBuffer->copy(*buffer, 0, 0, buffer->getSize()); - buffers.copiedBuffer->zero(buffer->getSize(), extraTailFrames); + buffers.nodeBuffer->copy(*buffer, 0, 0, buffer->getSize()); + buffers.nodeBuffer->zero(buffer->getSize(), extraTailFrames); } else if (bufferHostObject != nullptr) { - // Reuse a cached copy across repeated `.buffer = x` reassignments of the same - // JS-visible buffer (e.g. seeking, which recreates the source node but keeps - // reusing the already-decoded buffer) instead of deep-copying every time. - // See https://github.com/software-mansion/react-native-audio-api/issues/1263. - buffers.copiedBuffer = bufferHostObject->getOrCreateImmutableCopy(); + // stamp the buffer so from now on each call to getChannelData() returns a fresh view + // and the node can share the storage without risk of JS writing into it. + buffers.nodeBuffer = bufferHostObject->shareForPlayback(); } else { - buffers.copiedBuffer = std::make_shared(*buffer); + buffers.nodeBuffer = std::make_shared(*buffer); + } + + if (bufferHostObject != nullptr) { + sharedContentVersion_ = bufferHostObject->getContentVersion(); } buffers.audioBuffer = std::make_shared( RENDER_QUANTUM_SIZE, - buffers.copiedBuffer->getNumberOfChannels(), + buffers.nodeBuffer->getNumberOfChannels(), audioBufferSourceNode_->getContextSampleRate()); return buffers; } @@ -235,7 +244,7 @@ void AudioBufferSourceNodeHostObject::setBuffer( auto event = [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { - node->setBuffer(buffers.copiedBuffer, buffers.audioBuffer); + node->setBuffer(buffers.nodeBuffer, buffers.audioBuffer); }; audioBufferSourceNode_->scheduleAudioEvent(std::move(event)); } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h index a4d2fec5f..3ff446c90 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h @@ -4,6 +4,7 @@ #include #include +#include #include namespace audioapi { @@ -54,10 +55,16 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje /// The JS-visible buffer behind the last `setBuffer`, kept so the "acquire the /// content" step can run on it. Null when the buffer came from options or was cleared. std::shared_ptr bufferHostObject_; + /// `bufferHostObject_->getContentVersion()` at the moment the node last received its + /// samples. A newer version means JS replaced some channel storage since, so the node + /// is reading stale content and must be re-handed the buffer when it acquires it. + uint64_t sharedContentVersion_ = 0; bool hasBeenStarted_ = false; + /// The samples the node will read (shared with the JS-facing buffer when possible, a + /// padded private copy for pitch correction) plus its render-quantum scratch buffer. struct NodeBuffers { - std::shared_ptr copiedBuffer; + std::shared_ptr nodeBuffer; std::shared_ptr audioBuffer; }; @@ -71,7 +78,8 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje /// Web Audio's "acquire the content" step: runs on start() when a buffer is set, and on /// setBuffer() once already started. Cuts off every live getChannelData() view and - /// re-hands the node the fenced-off content. + /// if the JS-facing buffer's storage moved on since the node last received it, + /// re-hands the node the current content. /// https://webaudio.github.io/web-audio-api/#acquire-the-content void acquireBufferContent(jsi::Runtime &runtime); }; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp index 95279272b..1c562c407 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp @@ -141,10 +141,20 @@ class AlignedAudioBuffer { return channels_[index]; } + /// @brief Creates a buffer that shares this buffer's channel storage instead of copying + /// it. Both buffers read the same samples until one of them calls detachSharedChannel() + /// on a channel, which is how a source node can play a JS-visible buffer without a deep + /// copy while the JS side keeps copy-on-write semantics. + [[nodiscard]] std::shared_ptr shareChannels() const { + auto shared = std::make_shared(size_, numberOfChannels_, sampleRate_); + // share underlying data instead of copying it + shared->channels_ = channels_; + return shared; + } + /// @brief Gives channel @p index fresh storage holding a copy of its current samples. /// Every handle previously obtained through getSharedChannel() keeps the old storage /// alive but no longer aliases this buffer, so writes through it can't reach us anymore. - /// This is how a JS `getChannelData` view gets cut off once playback acquires the buffer. void detachSharedChannel(size_t index) { channels_[index] = std::make_shared>(*channels_[index]); } @@ -353,13 +363,18 @@ class AlignedAudioBuffer { {2, {ChannelLeft, ChannelRight}}, {4, {ChannelLeft, ChannelRight, ChannelSurroundLeft, ChannelSurroundRight}}, {5, {ChannelLeft, ChannelRight, ChannelCenter, ChannelSurroundLeft, ChannelSurroundRight}}, - {6, - {ChannelLeft, - ChannelRight, - ChannelCenter, - ChannelLFE, - ChannelSurroundLeft, - ChannelSurroundRight}}}; + { + 6, + { + ChannelLeft, + ChannelRight, + ChannelCenter, + ChannelLFE, + ChannelSurroundLeft, + ChannelSurroundRight, + }, + }, + }; template void discreteSum( diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h b/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h deleted file mode 100644 index b6207fcf5..000000000 --- a/packages/react-native-audio-api/common/cpp/audioapi/utils/ImmutableBufferCache.h +++ /dev/null @@ -1,44 +0,0 @@ -#pragma once - -#include - -#include - -namespace audioapi::utils { - -/// @brief Caches a defensive copy of an `AudioBuffer` so repeated requests for -/// "an immutable copy suitable for handing to a source node" reuse the same -/// copy instead of allocating a fresh one every time. Used by -/// `AudioBufferHostObject` to avoid deep-copying the entire buffer on every -/// `.buffer = x` reassignment of the same underlying buffer. That reassignment -/// is what seeking requires, since a live `AudioBufferSourceNode` can't be -/// repositioned. See https://github.com/software-mansion/react-native-audio-api/issues/1263. -/// @note `invalidate()` must be called whenever the source buffer's data could -/// have diverged from the cached copy. This class has no way to observe that on -/// its own. -class ImmutableBufferCache { - public: - /// @brief Returns a defensive copy of `source`, reusing the last copy - /// produced as long as `invalidate()` has not been called since. - [[nodiscard]] std::shared_ptr getOrCreate( - const std::shared_ptr &source) { - if (cached_ == nullptr) { - cached_ = std::make_shared(*source); - } - return cached_; - } - - /// @brief Call when the source buffer's data may no longer match the cached - /// copy: it was mutated in place (`copyToChannel`), a live JS-writable view - /// into it was handed out (`getChannelData`), or such a view has just been - /// cut off after possibly being written through. The next `getOrCreate` - /// produces a fresh copy and caching resumes from there. - void invalidate() { - cached_ = nullptr; - } - - private: - std::shared_ptr cached_; -}; - -} // namespace audioapi::utils diff --git a/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp b/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp index 300ff9ca7..bba6bd061 100644 --- a/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp +++ b/packages/react-native-audio-api/common/cpp/test/src/utils/AudioBufferTest.cpp @@ -655,4 +655,28 @@ TEST_F(AudioBufferTest, DetachSharedChannelCutsOffEscapedHandleAndKeepsSamples) << "The old handle stays alive and writable, it just no longer reaches the buffer."; } +// A source node plays a JS-visible buffer through shareChannels(). The JS side then relies +// on detachSharedChannel() as copy-on-write: its writes must never reach the node's view. +TEST_F(AudioBufferTest, ShareChannelsAliasesStorageUntilOneSideDetaches) { + AudioBuffer owner(BUF_SIZE, 2, SR); + fillChannel(owner, 0, 0.5f); + fillChannel(owner, 1, 0.25f); + + auto shared = owner.shareChannels(); + + ASSERT_EQ(shared->getNumberOfChannels(), 2u); + ASSERT_EQ(shared->getSize(), BUF_SIZE); + ASSERT_FLOAT_EQ(shared->getSampleRate(), SR); + EXPECT_EQ(shared->getSharedChannel(0), owner.getSharedChannel(0)) + << "shareChannels() must alias, not copy."; + + owner.detachSharedChannel(0); + fillChannel(owner, 0, 1.0f); + + expectChannel(*shared, 0, 0.5f); + expectChannel(owner, 0, 1.0f); + EXPECT_EQ(shared->getSharedChannel(1), owner.getSharedChannel(1)) + << "Detaching one channel must not touch the others."; +} + // NOLINTEND diff --git a/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp b/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp deleted file mode 100644 index 070f89763..000000000 --- a/packages/react-native-audio-api/common/cpp/test/src/utils/ImmutableBufferCacheTest.cpp +++ /dev/null @@ -1,64 +0,0 @@ -#include -#include -#include -#include - -using namespace audioapi; -using namespace audioapi::utils; - -// NOLINTBEGIN - -namespace { - -constexpr size_t FRAME_COUNT = 1024; -constexpr int CHANNELS = 2; -constexpr float SAMPLE_RATE = 44100.0f; - -std::shared_ptr makeBuffer() { - return std::make_shared(FRAME_COUNT, CHANNELS, SAMPLE_RATE); -} - -} // namespace - -// The core of https://github.com/software-mansion/react-native-audio-api/issues/1263: -// repeated `.buffer = x` reassignment of the same underlying buffer (the seek pattern) -// must not allocate a fresh full-size copy every time. -TEST(ImmutableBufferCacheTest, ReusesCachedCopyAcrossRepeatedCalls) { - ImmutableBufferCache cache; - auto source = makeBuffer(); - - auto first = cache.getOrCreate(source); - auto second = cache.getOrCreate(source); - auto third = cache.getOrCreate(source); - - EXPECT_EQ(first, second) << "Second call should reuse the cached copy, not allocate a new one."; - EXPECT_EQ(second, third) << "Third call should reuse the same cached copy as well."; -} - -TEST(ImmutableBufferCacheTest, CachedCopyIsAnActualCopyNotTheOriginal) { - ImmutableBufferCache cache; - auto source = makeBuffer(); - - auto copy = cache.getOrCreate(source); - - EXPECT_NE(copy, source) << "The cached copy must be a distinct AudioBuffer instance. Sharing " - "the original directly would let a future mutation of it race with " - "a node concurrently reading the \"copy\"."; -} - -TEST(ImmutableBufferCacheTest, InvalidateForcesAFreshCopyOnce) { - ImmutableBufferCache cache; - auto source = makeBuffer(); - - auto first = cache.getOrCreate(source); - cache.invalidate(); - auto second = cache.getOrCreate(source); - auto third = cache.getOrCreate(source); - - EXPECT_NE(first, second) << "invalidate() means the source was just mutated in place, so the " - "previously cached copy is stale and must not be reused."; - EXPECT_EQ(second, third) << "After producing one fresh copy, subsequent calls should resume " - "caching normally rather than copying every time."; -} - -// NOLINTEND From 017dcf8bc867bc7368a2602f94fa6d7471962dc4 Mon Sep 17 00:00:00 2001 From: michal Date: Fri, 11 Sep 2026 13:53:36 +0200 Subject: [PATCH 4/9] fix: remove unnecesary allocation in the constructor --- .../common/cpp/audioapi/utils/AudioBuffer.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp index 1c562c407..52b6753c7 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp @@ -146,8 +146,10 @@ class AlignedAudioBuffer { /// on a channel, which is how a source node can play a JS-visible buffer without a deep /// copy while the JS side keeps copy-on-write semantics. [[nodiscard]] std::shared_ptr shareChannels() const { - auto shared = std::make_shared(size_, numberOfChannels_, sampleRate_); - // share underlying data instead of copying it + auto shared = std::make_shared(); + shared->numberOfChannels_ = numberOfChannels_; + shared->sampleRate_ = sampleRate_; + shared->size_ = size_; shared->channels_ = channels_; return shared; } From 5bd8a0b2c77bd71950895a5c9dbf11ee5fb31c68 Mon Sep 17 00:00:00 2001 From: michal Date: Fri, 9 Oct 2026 14:18:05 +0200 Subject: [PATCH 5/9] refactor: cleanup of the implementation after review --- .../sources/AudioBufferHostObject.cpp | 46 +++++-------------- .../sources/AudioBufferHostObject.h | 27 ++++------- .../AudioBufferSourceNodeHostObject.cpp | 13 +++--- .../sources/AudioBufferSourceNodeHostObject.h | 4 +- .../core/sources/AudioBufferSourceNode.cpp | 34 ++++---------- .../core/sources/AudioBufferSourceNode.h | 17 ++----- .../common/cpp/audioapi/utils/AudioBuffer.hpp | 6 ++- 7 files changed, 45 insertions(+), 102 deletions(-) diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp index 2eef2ae6b..674ae4ee1 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp @@ -11,7 +11,7 @@ namespace audioapi { AudioBufferHostObject::AudioBufferHostObject(const std::shared_ptr &audioBuffer) - : audioBuffer_(audioBuffer) { + : audioBuffer_(audioBuffer), channelViewHandedOut_(audioBuffer->getNumberOfChannels(), false) { addGetters( JSI_EXPORT_PROPERTY_GETTER(AudioBufferHostObject, sampleRate), JSI_EXPORT_PROPERTY_GETTER(AudioBufferHostObject, length), @@ -29,7 +29,7 @@ AudioBufferHostObject::AudioBufferHostObject(AudioBufferHostObject &&other) noex audioBuffer_(std::move(other.audioBuffer_)), channelSharedWithNode_(std::move(other.channelSharedWithNode_)), contentVersion_(other.contentVersion_), - returnedChannelDataArrays_(std::move(other.returnedChannelDataArrays_)) {} + channelViewHandedOut_(std::move(other.channelViewHandedOut_)) {} std::shared_ptr AudioBufferHostObject::shareForPlayback() { channelSharedWithNode_.assign(audioBuffer_->getNumberOfChannels(), true); @@ -44,41 +44,18 @@ void AudioBufferHostObject::makeChannelWritable(size_t channel) { void AudioBufferHostObject::replaceChannelStorage(size_t channel) { // mark the channel as no longer shared with a node, so that future writes to it don't trigger another copy-on-write - if (channel < channelSharedWithNode_.size()) { - audioBuffer_->detachSharedChannel(channel); - channelSharedWithNode_[channel] = false; - ++contentVersion_; - } + audioBuffer_->detachSharedChannel(channel); + channelSharedWithNode_[channel] = false; + ++contentVersion_; } -void AudioBufferHostObject::detachReturnedChannelData(jsi::Runtime &runtime) { - if (returnedChannelDataArrays_.empty()) { - return; - } - - auto defineProperty = runtime.global() - .getPropertyAsObject(runtime, "Object") - .getPropertyAsFunction(runtime, "defineProperty"); - auto zeroDescriptor = jsi::Object(runtime); - zeroDescriptor.setProperty(runtime, "value", 0); - - std::vector channelDetached(audioBuffer_->getNumberOfChannels(), false); - - for (const auto &returned : returnedChannelDataArrays_) { - auto array = returned.array.lock(runtime); - if (array.isObject()) { - for (const auto *sizeProperty : {"length", "byteLength", "byteOffset"}) { - defineProperty.call(runtime, array, sizeProperty, zeroDescriptor); - } - } - - if (!channelDetached[returned.channel]) { - replaceChannelStorage(returned.channel); - channelDetached[returned.channel] = true; +void AudioBufferHostObject::detachReturnedChannelData() { + for (size_t channel = 0; channel < channelViewHandedOut_.size(); ++channel) { + if (channelViewHandedOut_[channel]) { + replaceChannelStorage(channel); + channelViewHandedOut_[channel] = false; } } - - returnedChannelDataArrays_.clear(); } JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, sampleRate) { @@ -110,8 +87,7 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { auto float32Array = float32ArrayCtor.callAsConstructor(runtime, arrayBuffer).getObject(runtime); float32Array.setExternalMemoryPressure(runtime, audioArrayBuffer->size()); - returnedChannelDataArrays_.push_back( - {.channel = channel, .array = jsi::WeakObject(runtime, float32Array)}); + channelViewHandedOut_[channel] = true; return float32Array; } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h index 6c4a787f2..6a7deb44c 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -31,7 +32,7 @@ class AudioBufferHostObject : public HostObject { audioBuffer_ = std::move(other.audioBuffer_); channelSharedWithNode_ = std::move(other.channelSharedWithNode_); contentVersion_ = other.contentVersion_; - returnedChannelDataArrays_ = std::move(other.returnedChannelDataArrays_); + channelViewHandedOut_ = std::move(other.channelViewHandedOut_); } return *this; } @@ -54,15 +55,14 @@ class AudioBufferHostObject : public HostObject { /// @brief Web Audio's "acquire the content" step for the views handed out by /// `getChannelData`. Call once playback of this buffer has been scheduled. Every - /// previously returned Float32Array stops aliasing `audioBuffer_` and, if JS still - /// holds it, reads as zero-length; the next `getChannelData` call hands out a fresh - /// view, mirroring what a browser does when it detaches those ArrayBuffers. - void detachReturnedChannelData(jsi::Runtime &runtime); + /// channel with a previously returned Float32Array gets fresh storage, so those views + /// stop pointing to `audioBuffer_` and the next `getChannelData` call hands out a fresh view. + void detachReturnedChannelData(); /// @brief Whether any `getChannelData` view is live, i.e. handed out since the last /// `detachReturnedChannelData`. [[nodiscard]] bool hasReturnedChannelData() const { - return !returnedChannelDataArrays_.empty(); + return std::ranges::any_of(channelViewHandedOut_, [](bool handedOut) { return handedOut; }); } JSI_PROPERTY_GETTER_DECL(sampleRate); @@ -75,15 +75,6 @@ class AudioBufferHostObject : public HostObject { JSI_HOST_FUNCTION_DECL(copyToChannel); private: - struct ReturnedChannelDataArray { - size_t channel; - /// Weak on purpose: a view JS already dropped must not be kept alive (nor keep its - /// external-memory-pressure hint alive) until the next start(). The channel is still - /// recorded so its storage gets swapped, since wrappers over the same ArrayBuffer may - /// outlive this particular Float32Array object. - jsi::WeakObject array; - }; - /// Copy-on-write: call before exposing or mutating a channel from JS. If a node may be /// reading that channel's storage, the buffer gets a private copy of it first. void makeChannelWritable(size_t channel); @@ -95,8 +86,8 @@ class AudioBufferHostObject : public HostObject { /// be reading that channel's current storage. std::vector channelSharedWithNode_; uint64_t contentVersion_ = 0; - /// Float32Array views handed out by `getChannelData` since the last - /// `detachReturnedChannelData`, kept so they can be neutralised then. - std::vector returnedChannelDataArrays_; + /// true once `getChannelData` handed out a view of that channel's + /// current storage, cleared by `detachReturnedChannelData`. + std::vector channelViewHandedOut_; }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp index 05781ceaf..f83d5a238 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp @@ -127,7 +127,7 @@ JSI_PROPERTY_SETTER_IMPL(AudioBufferSourceNodeHostObject, onloopended) { JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, start) { hasBeenStarted_ = true; - acquireBufferContent(runtime); + acquireBufferContent(); auto handle = node_->handle; auto event = [handle, @@ -142,12 +142,12 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, start) { return jsi::Value::undefined(); } -void AudioBufferSourceNodeHostObject::acquireBufferContent(jsi::Runtime &runtime) { +void AudioBufferSourceNodeHostObject::acquireBufferContent() { if (bufferHostObject_ == nullptr) { return; } - bufferHostObject_->detachReturnedChannelData(runtime); + bufferHostObject_->detachReturnedChannelData(); if (bufferHostObject_->getContentVersion() == sharedContentVersion_) { // Nothing replaced the storage the node already reads, so it holds the content @@ -156,12 +156,11 @@ void AudioBufferSourceNodeHostObject::acquireBufferContent(jsi::Runtime &runtime } // Some channel storage could be swapped since the node received its samples, either by - // the detach above or by an earlier copy-on-write. Re-hand it the current content, - // touching only the samples so loopEnd and friends survive. + // the detach above or by an earlier copy-on-write. Re-hand it the current content. auto buffers = prepareNodeBuffers(bufferHostObject_->audioBuffer_, bufferHostObject_); auto event = [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { - node->replaceBufferContent(buffers.nodeBuffer, buffers.audioBuffer); + node->setBuffer(buffers.nodeBuffer, buffers.audioBuffer); }; audioBufferSourceNode_->scheduleAudioEvent(std::move(event)); } @@ -180,7 +179,7 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, setBuffer) { // Per Web Audio, assigning a buffer to an already-started source acquires its // content right away, because start() had nothing to acquire back then. if (hasBeenStarted_) { - acquireBufferContent(runtime); + acquireBufferContent(); } return jsi::Value::undefined(); diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h index f7f1b9797..bed2ca136 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h @@ -34,7 +34,7 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje JSI_PROPERTY_SETTER_DECL(loopEnd); JSI_PROPERTY_SETTER_DECL(onloopended); - JSI_HOST_FUNCTION_DECL(start); + JSI_HOST_FUNCTION_DECL(start) override; JSI_HOST_FUNCTION_DECL(setBuffer); [[nodiscard]] size_t getMemoryPressure() const override { @@ -81,7 +81,7 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje /// if the JS-facing buffer's storage moved on since the node last received it, /// re-hands the node the current content. /// https://webaudio.github.io/web-audio-api/#acquire-the-content - void acquireBufferContent(jsi::Runtime &runtime); + void acquireBufferContent(); }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp index 98a4c1bb7..beea83eef 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp @@ -25,7 +25,7 @@ AudioBufferSourceNode::AudioBufferSourceNode( loopStart_(options.loopStart), loopEnd_(options.loopEnd), onLoopEndedEvent_(context->getAudioEventHandlerRegistry(), context->getAudioEventProducer()) { - auto onLoopEnded = [this]() { + auto onLoopEnded = [this] { sendOnLoopEndedEvent(); }; @@ -55,30 +55,14 @@ void AudioBufferSourceNode::setLoopEnd(double loopEnd) { void AudioBufferSourceNode::setBuffer( const std::shared_ptr &buffer, const std::shared_ptr &audioBuffer) { - if (!swapBuffers(buffer, audioBuffer)) { - return; - } - - loopEnd_ = buffer_ == nullptr ? 0 : buffer_->getDuration(); -} - -void AudioBufferSourceNode::replaceBufferContent( - const std::shared_ptr &buffer, - const std::shared_ptr &audioBuffer) { - swapBuffers(buffer, audioBuffer); -} - -bool AudioBufferSourceNode::swapBuffers( - const std::shared_ptr &buffer, - const std::shared_ptr &audioBuffer) { std::shared_ptr context = context_.lock(); if (context == nullptr) { - return false; + return; } if (isFinished()) { - return false; + return; } if (buffer_ != nullptr) { @@ -92,7 +76,6 @@ bool AudioBufferSourceNode::swapBuffers( buffer_ = buffer; audioBuffer_ = audioBuffer; processor_->setBuffer(buffer_); - return true; } void AudioBufferSourceNode::start(double when, double offset, double duration) { @@ -106,13 +89,12 @@ void AudioBufferSourceNode::start(double when, double offset, double duration) { return; } - offset = std::min(offset, static_cast(buffer_->getSize()) / buffer_->getSampleRate()); - - if (loop_) { - offset = std::min(offset, loopEnd_); - } + // Past the (effective) loop end, or past the buffer when not looping, playback starts + // at that boundary instead. + const auto sampleRate = buffer_->getSampleRate(); + offset = std::min(offset, getVirtualEndFrame(sampleRate) / sampleRate); - vReadIndex_ = static_cast(buffer_->getSampleRate() * offset); + vReadIndex_ = sampleRate * offset; } void AudioBufferSourceNode::disable() { diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h index 380c853cd..64fd9ab00 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h @@ -33,18 +33,15 @@ class AudioBufferSourceNode : public AudioBufferBaseSourceNode { return loopEnd_; } + /// @brief Hands the current buffers to the disposer and installs the new ones. Serves + /// both a JS `buffer` assignment and the "acquire the content" refresh on start(). + /// Loop bounds are left untouched: the effective loop end is resolved against the + /// buffer at render time, so a stale `loopEnd_` cannot outrun a shorter buffer. /// @note Audio Thread only void setBuffer( const std::shared_ptr &buffer, const std::shared_ptr &audioBuffer); - /// @brief Swaps in a buffer holding the same frames, channels and sample rate as the - /// current one, keeping loop bounds and channel count untouched. This is the "acquire - /// the content" refresh: the samples may have changed since setBuffer(), the shape has not. - void replaceBufferContent( - const std::shared_ptr &buffer, - const std::shared_ptr &audioBuffer); - using AudioScheduledSourceNode::start; /// @note Audio Thread only void start(double when, double offset, double duration = -1); @@ -83,12 +80,6 @@ class AudioBufferSourceNode : public AudioBufferBaseSourceNode { double getVirtualEndFrame(float sampleRate); std::unique_ptr processor_; - - /// Hands the old buffers to the disposer and installs the new ones. Returns false when - /// the context is already gone. - bool swapBuffers( - const std::shared_ptr &buffer, - const std::shared_ptr &audioBuffer); }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp index 52b6753c7..6ac1f1550 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioBuffer.hpp @@ -156,7 +156,7 @@ class AlignedAudioBuffer { /// @brief Gives channel @p index fresh storage holding a copy of its current samples. /// Every handle previously obtained through getSharedChannel() keeps the old storage - /// alive but no longer aliases this buffer, so writes through it can't reach us anymore. + /// alive but no longer points to this buffer, so writes through it can't reach here anymore. void detachSharedChannel(size_t index) { channels_[index] = std::make_shared>(*channels_[index]); } @@ -390,6 +390,8 @@ class AlignedAudioBuffer { } } + // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) + template void sumByUpMixing( const AlignedAudioBuffer &source, @@ -489,3 +491,5 @@ using AudioBuffer = AlignedAudioBuffer; using DSPAudioBuffer = AlignedAudioBuffer; } // namespace audioapi + +// NOLINTEND(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) From 51c8f02188a754ef181beaf9674c38e324d91e0c Mon Sep 17 00:00:00 2001 From: michal Date: Fri, 9 Oct 2026 16:26:49 +0200 Subject: [PATCH 6/9] refactor: corrected version, less copies --- .claude/skills/audio-nodes/SKILL.md | 1 + .claude/skills/host-objects/SKILL.md | 2 + .../sources/AudioBufferHostObject.cpp | 8 +- .../sources/AudioBufferHostObject.h | 8 -- .../AudioBufferSourceNodeHostObject.cpp | 71 +++--------- .../sources/AudioBufferSourceNodeHostObject.h | 35 +++--- .../HostObjects/utils/NodeOptionsParser.h | 9 -- .../sources/AudioBufferBaseSourceNode.cpp | 12 ++ .../core/sources/AudioBufferBaseSourceNode.h | 4 + .../cpp/audioapi/dsp/WsolaTimeStretcher.h | 10 +- .../common/cpp/audioapi/types/NodeOptions.h | 1 - .../sources/AudioBufferSourceNodeTest.cpp | 104 +++++++++++++++--- .../wpt_tests/wpt/wpt-results.mjs | 3 - 13 files changed, 152 insertions(+), 116 deletions(-) diff --git a/.claude/skills/audio-nodes/SKILL.md b/.claude/skills/audio-nodes/SKILL.md index cb36f26d2..4ff915ece 100644 --- a/.claude/skills/audio-nodes/SKILL.md +++ b/.claude/skills/audio-nodes/SKILL.md @@ -401,6 +401,7 @@ Tail-bearing nodes (Delay/Convolver/Biquad) need no special handling: while conn - `AudioNode` — standard effect or analysis node - `AudioScheduledSourceNode` — source with start/stop scheduling - `AudioBufferBaseSourceNode` — source that plays back an AudioBuffer with pitch control + With `pitchCorrection` the WSOLA stretcher holds ~30 ms of audio, so the node must outlive its last sample. It does that itself (`stretchTailFramesLeft_`): once the processor reports the source ran dry it keeps the node playing and feeds the stretcher the zero-filled input until `WsolaTimeStretcher::latencyTailFrames` are consumed. Never pad the buffer with silence to get the same effect — that forces a private copy per node and breaks buffer sharing with the JS-facing `AudioBuffer`. 2. **Header file** (`core//MyNode.h`) - Annotate every method with `/// @note JS Thread only` or `/// @note Audio Thread only` diff --git a/.claude/skills/host-objects/SKILL.md b/.claude/skills/host-objects/SKILL.md index 515af7ee9..29c9755b8 100644 --- a/.claude/skills/host-objects/SKILL.md +++ b/.claude/skills/host-objects/SKILL.md @@ -357,6 +357,8 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { The view aliases native memory for as long as JS keeps it, and the `shared_ptr` inside the `jsi::ArrayBuffer` keeps that memory alive on its own. When the native side later needs the view to stop aliasing (Web Audio's "acquire the content" on `AudioBufferSourceNode.start()`), it must retain the returned object and neutralise it afterwards — see `detachReturnedChannelData` in the real `AudioBufferHostObject`. +`AudioBufferSourceNodeHostObject` hands samples to the audio thread in exactly one place, `acquireBufferContent`, which runs on `start()` and on a `setBuffer()` made after start. A `setBuffer()` before start only records the buffer and publishes its channel count; the node reads nothing until acquire, so every JS write made in between is played and nothing has to track content versions. Acquire detaches the live `getChannelData` views first and then calls `shareForPlayback`, so the per-channel bookkeeping vectors in `AudioBufferHostObject` must be sized in its constructor — sizing them lazily in `shareForPlayback` crashed `detachReturnedChannelData` the moment the order flipped. + ### External memory pressure Call `setExternalMemoryPressure` whenever returning a HostObject or typed array that wraps a large native buffer. This lets the JS GC schedule collection correctly: diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp index 674ae4ee1..bf6b75980 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp @@ -11,7 +11,9 @@ namespace audioapi { AudioBufferHostObject::AudioBufferHostObject(const std::shared_ptr &audioBuffer) - : audioBuffer_(audioBuffer), channelViewHandedOut_(audioBuffer->getNumberOfChannels(), false) { + : audioBuffer_(audioBuffer), + channelSharedWithNode_(audioBuffer->getNumberOfChannels(), false), + channelViewHandedOut_(audioBuffer->getNumberOfChannels(), false) { addGetters( JSI_EXPORT_PROPERTY_GETTER(AudioBufferHostObject, sampleRate), JSI_EXPORT_PROPERTY_GETTER(AudioBufferHostObject, length), @@ -28,7 +30,6 @@ AudioBufferHostObject::AudioBufferHostObject(AudioBufferHostObject &&other) noex : HostObject(std::move(other)), audioBuffer_(std::move(other.audioBuffer_)), channelSharedWithNode_(std::move(other.channelSharedWithNode_)), - contentVersion_(other.contentVersion_), channelViewHandedOut_(std::move(other.channelViewHandedOut_)) {} std::shared_ptr AudioBufferHostObject::shareForPlayback() { @@ -37,7 +38,7 @@ std::shared_ptr AudioBufferHostObject::shareForPlayback() { } void AudioBufferHostObject::makeChannelWritable(size_t channel) { - if (channel < channelSharedWithNode_.size() && channelSharedWithNode_[channel]) { + if (channelSharedWithNode_[channel]) { replaceChannelStorage(channel); } } @@ -46,7 +47,6 @@ void AudioBufferHostObject::replaceChannelStorage(size_t channel) { // mark the channel as no longer shared with a node, so that future writes to it don't trigger another copy-on-write audioBuffer_->detachSharedChannel(channel); channelSharedWithNode_[channel] = false; - ++contentVersion_; } void AudioBufferHostObject::detachReturnedChannelData() { diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h index 6a7deb44c..0369a2e16 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h @@ -31,7 +31,6 @@ class AudioBufferHostObject : public HostObject { HostObject::operator=(std::move(other)); audioBuffer_ = std::move(other.audioBuffer_); channelSharedWithNode_ = std::move(other.channelSharedWithNode_); - contentVersion_ = other.contentVersion_; channelViewHandedOut_ = std::move(other.channelViewHandedOut_); } return *this; @@ -47,12 +46,6 @@ class AudioBufferHostObject : public HostObject { /// The JS side is copy-on-write, so it will get a fresh copy if it writes into it while a node is reading it. [[nodiscard]] std::shared_ptr shareForPlayback(); - /// @brief Bumped every time a channel's storage is replaced. A source node compares it - /// with the version it shared at to decide whether "acquire the content" must re-share. - [[nodiscard]] uint64_t getContentVersion() const { - return contentVersion_; - } - /// @brief Web Audio's "acquire the content" step for the views handed out by /// `getChannelData`. Call once playback of this buffer has been scheduled. Every /// channel with a previously returned Float32Array gets fresh storage, so those views @@ -85,7 +78,6 @@ class AudioBufferHostObject : public HostObject { /// One flag per channel: true while a node handed out by `shareForPlayback` may still /// be reading that channel's current storage. std::vector channelSharedWithNode_; - uint64_t contentVersion_ = 0; /// true once `getChannelData` handed out a view of that channel's /// current storage, cleared by `detachReturnedChannelData`. std::vector channelViewHandedOut_; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp index f83d5a238..e40e68017 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp @@ -24,10 +24,6 @@ AudioBufferSourceNodeHostObject::AudioBufferSourceNodeHostObject( loopSkip_(options.loopSkip), loopStart_(options.loopStart), loopEnd_(options.loopEnd) { - if (options.buffer != nullptr) { - setBuffer(options.buffer); - } - addGetters( JSI_EXPORT_PROPERTY_GETTER(AudioBufferSourceNodeHostObject, loop), JSI_EXPORT_PROPERTY_GETTER(AudioBufferSourceNodeHostObject, loopSkip), @@ -143,21 +139,11 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, start) { } void AudioBufferSourceNodeHostObject::acquireBufferContent() { - if (bufferHostObject_ == nullptr) { - return; - } - - bufferHostObject_->detachReturnedChannelData(); - - if (bufferHostObject_->getContentVersion() == sharedContentVersion_) { - // Nothing replaced the storage the node already reads, so it holds the content - // exactly as acquired. - return; + if (assignedBufferHostObject_ != nullptr) { + assignedBufferHostObject_->detachReturnedChannelData(); } - // Some channel storage could be swapped since the node received its samples, either by - // the detach above or by an earlier copy-on-write. Re-hand it the current content. - auto buffers = prepareNodeBuffers(bufferHostObject_->audioBuffer_, bufferHostObject_); + auto buffers = prepareNodeBuffers(assignedBufferHostObject_); auto event = [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { node->setBuffer(buffers.nodeBuffer, buffers.audioBuffer); @@ -173,24 +159,17 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, setBuffer) { thisValue.asObject(runtime).setExternalMemoryPressure( runtime, getMemoryPressure() + bufferHostObject->getSizeInBytes()); - setBuffer(bufferHostObject->audioBuffer_, bufferHostObject); - } - - // Per Web Audio, assigning a buffer to an already-started source acquires its - // content right away, because start() had nothing to acquire back then. - if (hasBeenStarted_) { - acquireBufferContent(); + setBuffer(bufferHostObject); } return jsi::Value::undefined(); } AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::prepareNodeBuffers( - const std::shared_ptr &buffer, const std::shared_ptr &bufferHostObject) { NodeBuffers buffers; - if (buffer == nullptr) { + if (bufferHostObject == nullptr) { buffers.nodeBuffer = nullptr; buffers.audioBuffer = std::make_shared( RENDER_QUANTUM_SIZE, @@ -199,28 +178,14 @@ AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::pr return buffers; } + const auto &buffer = bufferHostObject->audioBuffer_; if (pitchCorrection_) { - // The stretcher needs tail padding past the end of the samples, so this path keeps - // its own padded copy. initStretch(static_cast(buffer->getNumberOfChannels()), buffer->getSampleRate()); - auto extraTailFrames = - static_cast((inputLatency_ + outputLatency_) * buffer->getSampleRate()); - size_t totalSize = buffer->getSize() + extraTailFrames; - buffers.nodeBuffer = std::make_shared( - totalSize, buffer->getNumberOfChannels(), buffer->getSampleRate()); - buffers.nodeBuffer->copy(*buffer, 0, 0, buffer->getSize()); - buffers.nodeBuffer->zero(buffer->getSize(), extraTailFrames); - } else if (bufferHostObject != nullptr) { - // stamp the buffer so from now on each call to getChannelData() returns a fresh view - // and the node can share the storage without risk of JS writing into it. - buffers.nodeBuffer = bufferHostObject->shareForPlayback(); - } else { - buffers.nodeBuffer = std::make_shared(*buffer); } - if (bufferHostObject != nullptr) { - sharedContentVersion_ = bufferHostObject->getContentVersion(); - } + // The node reads the JS-facing storage directly; from now on JS writes to it go + // copy-on-write, so nothing can change the samples under the node. + buffers.nodeBuffer = bufferHostObject->shareForPlayback(); buffers.audioBuffer = std::make_shared( RENDER_QUANTUM_SIZE, @@ -230,26 +195,24 @@ AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::pr } void AudioBufferSourceNodeHostObject::setBuffer( - const std::shared_ptr &buffer, const std::shared_ptr &bufferHostObject) { - bufferHostObject_ = bufferHostObject; - auto buffers = prepareNodeBuffers(buffer, bufferHostObject); + assignedBufferHostObject_ = bufferHostObject; // Publish the new output width on the host thread before renegotiation so // MAX / CLAMPED_MAX downstream nodes see it immediately - const size_t newOutputChannelNumber = buffer == nullptr + const size_t newOutputChannelNumber = bufferHostObject == nullptr ? AudioBufferSourceOptions::kDefaultOutputChannelNumber - : buffer->getNumberOfChannels(); + : bufferHostObject->audioBuffer_->getNumberOfChannels(); if (newOutputChannelNumber != audioBufferSourceNode_->getOutputChannelNumber()) { audioBufferSourceNode_->setOutputChannelNumber(newOutputChannelNumber); renegotiate(); } - auto event = - [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { - node->setBuffer(buffers.nodeBuffer, buffers.audioBuffer); - }; - audioBufferSourceNode_->scheduleAudioEvent(std::move(event)); + // Per Web Audio, assigning a buffer to an already-started source acquires its + // content right away, because start() had nothing to acquire back then. + if (hasBeenStarted_) { + acquireBufferContent(); + } } } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h index bed2ca136..d02f6f341 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h @@ -4,7 +4,6 @@ #include #include -#include #include namespace audioapi { @@ -52,34 +51,30 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje double loopStart_; double loopEnd_; - /// The JS-visible buffer behind the last `setBuffer`, kept so the "acquire the - /// content" step can run on it. Null when the buffer came from options or was cleared. - std::shared_ptr bufferHostObject_; - /// `bufferHostObject_->getContentVersion()` at the moment the node last received its - /// samples. A newer version means JS replaced some channel storage since, so the node - /// is reading stale content and must be re-handed the buffer when it acquires it. - uint64_t sharedContentVersion_ = 0; + /// The JS-facing buffer assigned last, held here until "acquire the content" hands its + /// samples to the audio thread. Until then the node has not seen it, so JS can keep + /// writing into it and every write is honoured. Null when cleared. + std::shared_ptr assignedBufferHostObject_; bool hasBeenStarted_ = false; - /// The samples the node will read (shared with the JS-facing buffer when possible, a - /// padded private copy for pitch correction) plus its render-quantum scratch buffer. + /// The samples the node will read (storage shared with the JS-facing buffer) plus its + /// render-quantum scratch buffer. struct NodeBuffers { std::shared_ptr nodeBuffer; std::shared_ptr audioBuffer; }; - NodeBuffers prepareNodeBuffers( - const std::shared_ptr &buffer, - const std::shared_ptr &bufferHostObject); + NodeBuffers prepareNodeBuffers(const std::shared_ptr &bufferHostObject); - void setBuffer( - const std::shared_ptr &buffer, - const std::shared_ptr &bufferHostObject = nullptr); + /// Records the assignment and publishes the buffer's channel count to the graph. The + /// samples themselves reach the node only through `acquireBufferContent`, which runs + /// right away when the node has already started. + void setBuffer(const std::shared_ptr &bufferHostObject); - /// Web Audio's "acquire the content" step: runs on start() when a buffer is set, and on - /// setBuffer() once already started. Cuts off every live getChannelData() view and - /// if the JS-facing buffer's storage moved on since the node last received it, - /// re-hands the node the current content. + /// Web Audio's "acquire the content" step, the single point where the node receives + /// samples: runs on start(), and on setBuffer() once already started. Cuts off every + /// live getChannelData() view, then hands the node the buffer's content as it is now, + /// so JS writes made before this moment are played and later ones are not. /// https://webaudio.github.io/web-audio-api/#acquire-the-content void acquireBufferContent(); }; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h index c921ff095..2f580b58b 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h @@ -284,15 +284,6 @@ inline AudioBufferSourceOptions parseAudioBufferSourceOptions( const jsi::Object &optionsObject) { AudioBufferSourceOptions options(parseBaseAudioBufferSourceOptions(runtime, optionsObject)); - if (optionsObject.hasProperty(runtime, "buffer")) { - auto bufferValue = optionsObject.getProperty(runtime, "buffer"); - if (bufferValue.isObject() && - bufferValue.getObject(runtime).isHostObject(runtime)) { - options.buffer = - bufferValue.getObject(runtime).asHostObject(runtime)->audioBuffer_; - } - } - auto loopValue = optionsObject.getProperty(runtime, "loop"); if (loopValue.isBool()) { options.loop = loopValue.getBool(); diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.cpp b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.cpp index d2724b7ff..e92110289 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.cpp @@ -49,6 +49,7 @@ void AudioBufferBaseSourceNode::initStretch( const std::shared_ptr &playbackRateBuffer) { wsolaStretcher_.configure(channelCount, sampleRate); playbackRateBuffer_ = playbackRateBuffer; + stretchTailFramesLeft_ = WsolaTimeStretcher::latencyTailFrames(sampleRate); } std::shared_ptr AudioBufferBaseSourceNode::getDetuneParam() const { @@ -141,8 +142,19 @@ void AudioBufferBaseSourceNode::processWithPitchCorrection( detune / static_cast(SEMITONES_PER_OCTAVE)); const float bufferPlaybackRate = rate >= 0.0f ? rate : -rate; + const bool stopTimeReached = isStopScheduled(); runBufferProcessor(playbackRateBuffer_, startOffset, offsetLength, bufferPlaybackRate, false); + // The source ran dry (as opposed to stop() time arriving), but the stretcher still + // holds audio. The processor has already zero-filled the rest of the input, so keep + // feeding that silence until the tail has drained, then let the node finish. + const bool sourceRanDry = !stopTimeReached && isStopScheduled(); + if (sourceRanDry && stretchTailFramesLeft_ > 0) { + playbackState_ = PlaybackState::PLAYING; + stretchTailFramesLeft_ -= + std::min(stretchTailFramesLeft_, static_cast(framesNeededToStretch)); + } + wsolaStretcher_.process( *playbackRateBuffer_, static_cast(framesNeededToStretch), diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h index d4f4d6bf5..f9f69e063 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h @@ -70,6 +70,10 @@ class AudioBufferBaseSourceNode : public AudioScheduledSourceNode { const bool pitchCorrection_; WsolaTimeStretcher wsolaStretcher_; std::shared_ptr playbackRateBuffer_; + /// Silence still owed to the stretcher after the source runs dry, so the audio it + /// buffers is played out instead of being cut off with the last real sample. This + /// replaces padding the buffer itself, which would force a private copy per node. + size_t stretchTailFramesLeft_ = 0; const std::shared_ptr detuneParam_; const std::shared_ptr playbackRateParam_; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/dsp/WsolaTimeStretcher.h b/packages/react-native-audio-api/common/cpp/audioapi/dsp/WsolaTimeStretcher.h index 9af54135b..8272f9d0b 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/dsp/WsolaTimeStretcher.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/dsp/WsolaTimeStretcher.h @@ -41,10 +41,18 @@ class WsolaTimeStretcher { // some arbitrary value has to be set here to limit the size of the buffer for wsola algorithm static constexpr float MAX_PLAYBACK_RATE = 4; - /// Rough latency estimates for buffer tail padding (seconds). + /// Rough latency estimates (milliseconds) reported to JS and used to size the + /// silent tail that drains the stretcher once its source runs out. static constexpr float INPUT_LATENCY_MS = 20.0f; static constexpr float OUTPUT_LATENCY_MS = 10.0f; + /// @brief How many frames of silence a source has to keep feeding after its last real + /// sample so the audio still buffered in the stretcher reaches the output. + [[nodiscard]] static size_t latencyTailFrames(float sampleRate) { + constexpr float kMsPerSecond = 1000.0f; + return static_cast((INPUT_LATENCY_MS + OUTPUT_LATENCY_MS) / kMsPerSecond * sampleRate); + } + private: static constexpr float OLA_WINDOW_MS = 20.0f; static constexpr float SEARCH_INTERVAL_MS = 30.0f; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h b/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h index 290c1e12e..050949a0d 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h @@ -191,7 +191,6 @@ struct AudioBufferSourceOptions : BaseAudioBufferSourceOptions { /// Spec: with no buffer assigned the node emits one channel of silence. static constexpr size_t kDefaultOutputChannelNumber = 1; - std::shared_ptr buffer = nullptr; float loopStart = 0.0f; float loopEnd = 0.0f; bool loop = false; diff --git a/packages/react-native-audio-api/common/cpp/test/src/core/sources/AudioBufferSourceNodeTest.cpp b/packages/react-native-audio-api/common/cpp/test/src/core/sources/AudioBufferSourceNodeTest.cpp index 67f854006..62c094ca8 100644 --- a/packages/react-native-audio-api/common/cpp/test/src/core/sources/AudioBufferSourceNodeTest.cpp +++ b/packages/react-native-audio-api/common/cpp/test/src/core/sources/AudioBufferSourceNodeTest.cpp @@ -95,23 +95,19 @@ class AudioBufferSourceNodeTest : public ::testing::Test { } std::unique_ptr makeNode(bool pitchCorrection) { - BaseAudioBufferSourceOptions baseOptions; - baseOptions.pitchCorrection = pitchCorrection; - baseOptions.playbackRate = 1.0f; - baseOptions.detune = 0.0f; - AudioBufferSourceOptions options(baseOptions); - - auto node = std::make_unique(context, options); + auto node = makeNodeWithOptions(pitchCorrection); - // Enough source for NUM_QUANTA at 2x plus WSOLA tail padding (mirrors HostObject setBuffer). - const size_t extraTailFrames = static_cast( - (WsolaTimeStretcher::INPUT_LATENCY_MS + WsolaTimeStretcher::OUTPUT_LATENCY_MS) / 1000.0f * - SAMPLE_RATE); - const size_t bufferFrames = static_cast(NUM_QUANTA * QUANTUM * 4) + extraTailFrames; - auto buffer = makeSilentBuffer(bufferFrames); - auto outputBuffer = - std::make_shared(QUANTUM, 1, static_cast(SAMPLE_RATE)); + // Enough source for NUM_QUANTA at 2x. + return startWithBuffer( + std::move(node), makeSilentBuffer(NUM_QUANTA * QUANTUM * 4), pitchCorrection); + } + /// Mirrors the HostObject: a pitch-corrected node gets its stretcher initialised + /// before the buffer lands, and the buffer is handed over as-is (never padded). + std::unique_ptr startWithBuffer( + std::unique_ptr node, + const std::shared_ptr &buffer, + bool pitchCorrection) { if (pitchCorrection) { auto playbackRateBuffer = std::make_shared( static_cast(WsolaTimeStretcher::MAX_PLAYBACK_RATE * QUANTUM), @@ -120,11 +116,20 @@ class AudioBufferSourceNodeTest : public ::testing::Test { node->initStretch(1, static_cast(SAMPLE_RATE), playbackRateBuffer); } - node->setBuffer(buffer, outputBuffer); + node->setBuffer(buffer, makeOutputBuffer()); node->start(0.0); return node; } + std::unique_ptr makeNodeWithOptions(bool pitchCorrection) { + BaseAudioBufferSourceOptions baseOptions; + baseOptions.pitchCorrection = pitchCorrection; + baseOptions.playbackRate = 1.0f; + baseOptions.detune = 0.0f; + return std::make_unique( + context, AudioBufferSourceOptions(baseOptions)); + } + std::unique_ptr makeNodeWithoutBuffer() { return std::make_unique( context, AudioBufferSourceOptions(BaseAudioBufferSourceOptions())); @@ -269,4 +274,71 @@ TEST_F(AudioBufferSourceNodeTest, BufferSwapKeepsHostPublishedOutputChannelNumbe EXPECT_EQ(node->getOutputChannelNumber(), 1u); } +/// The stretcher holds ~INPUT_LATENCY + OUTPUT_LATENCY of audio. The buffer used to be +/// copied with that much silence appended so the node outlived its last sample; now +/// the node plays the buffer as handed over and feeds the stretcher silence itself +/// once the source runs dry, ending only after the tail has drained. +TEST_F(AudioBufferSourceNodeTest, PitchCorrectedSourceDrainsStretcherTailAfterBufferEnds) { + static constexpr uint64_t ENDED_CALLBACK_ID = 54; + // At the modulated 2x rate the whole buffer is consumed in two quanta. + static constexpr size_t BUFFER_FRAMES = 2 * QUANTUM * 2; + static constexpr size_t QUANTA_TO_CONSUME_BUFFER = 2; + const size_t tailFrames = WsolaTimeStretcher::latencyTailFrames(SAMPLE_RATE); + const size_t quantaToDrainTail = tailFrames / (2 * QUANTUM) + 1; + + auto *node = addNode(startWithBuffer( + makeNodeWithOptions(/*pitchCorrection=*/true), + makeSilentBuffer(BUFFER_FRAMES), + /*pitchCorrection=*/true)); + node->assignOnEndedCallbackId(ENDED_CALLBACK_ID); + EXPECT_CALL(*eventRegistry, unregisterHandler(AudioEvent::ENDED, ENDED_CALLBACK_ID)) + .Times(testing::AnyNumber()); + EXPECT_CALL( + *eventRegistry, + dispatchEventFromAudioThread(testing::_, AudioEvent::ENDED, ENDED_CALLBACK_ID, testing::_)) + .WillOnce(testing::Return(true)); + + for (size_t q = 0; q < QUANTA_TO_CONSUME_BUFFER; ++q) { + renderQuantumWithModulation(*node, MODULATION); + } + EXPECT_TRUE(node->isPlaying()) << "the source ran dry but the stretcher tail is not drained"; + + const double bufferDuration = static_cast(BUFFER_FRAMES) / SAMPLE_RATE; + size_t quantaUntilEnded = 0; + while (!node->isFinished() && quantaUntilEnded <= quantaToDrainTail) { + renderQuantumWithModulation(*node, MODULATION); + ++quantaUntilEnded; + if (node->isPlaying()) { + EXPECT_LE(node->getCurrentPosition(), bufferDuration) + << "the reported position must not run past the real buffer into the tail"; + } + } + + EXPECT_TRUE(node->isFinished()) << "the node never ended after the tail drained"; + EXPECT_GE(quantaUntilEnded, quantaToDrainTail - 1); +} + +/// Without the stretcher in the loop (rate 1 skips it) there is nothing to drain, so a +/// pitch-corrected node ends as soon as its buffer does. +TEST_F(AudioBufferSourceNodeTest, PitchCorrectedSourceAtUnityRateEndsWithItsBuffer) { + static constexpr uint64_t ENDED_CALLBACK_ID = 55; + static constexpr size_t BUFFER_FRAMES = 2 * QUANTUM; + + auto *node = addNode(startWithBuffer( + makeNodeWithOptions(/*pitchCorrection=*/true), + makeSilentBuffer(BUFFER_FRAMES), + /*pitchCorrection=*/true)); + node->assignOnEndedCallbackId(ENDED_CALLBACK_ID); + EXPECT_CALL(*eventRegistry, unregisterHandler(AudioEvent::ENDED, ENDED_CALLBACK_ID)) + .Times(testing::AnyNumber()); + EXPECT_CALL( + *eventRegistry, + dispatchEventFromAudioThread(testing::_, AudioEvent::ENDED, ENDED_CALLBACK_ID, testing::_)) + .WillOnce(testing::Return(true)); + + processGraph(); + processGraph(); + EXPECT_TRUE(node->isFinished()); +} + // NOLINTEND diff --git a/packages/react-native-audio-api/wpt_tests/wpt/wpt-results.mjs b/packages/react-native-audio-api/wpt_tests/wpt/wpt-results.mjs index babd011c7..0061f1050 100644 --- a/packages/react-native-audio-api/wpt_tests/wpt/wpt-results.mjs +++ b/packages/react-native-audio-api/wpt_tests/wpt/wpt-results.mjs @@ -53,9 +53,6 @@ const COVERAGE_NOTES = { 'the-analysernode-interface': 'One failure: minDecibels/maxDecibels are stored as float, so reading back a ' + 'high-precision double value loses a few digits (cosmetic round-trip mismatch).', - 'the-audiobuffer-interface': - 'Remaining failures: acquire-the-content expects AudioBufferSourceNode.start() to ' + - 'snapshot the buffer contents (we still share the underlying memory).', 'the-biquadfilternode-interface': 'The filter runs in double precision, so the static frequency response matches the ' + 'reference. The remaining failures are the parameter-automation tests, which expect ' + From 762fa9e193e1802d31a2048b358aac6d8f503b73 Mon Sep 17 00:00:00 2001 From: michal Date: Mon, 12 Oct 2026 15:53:32 +0200 Subject: [PATCH 7/9] refactor: apply changes also to the audiobufferqueuesourcenode --- .claude/skills/host-objects/SKILL.md | 2 + .../audio-buffer-queue-source-node.mdx | 4 ++ .../AudioBufferBaseSourceNodeHostObject.cpp | 13 ++--- .../AudioBufferBaseSourceNodeHostObject.h | 2 + .../AudioBufferQueueSourceNodeHostObject.cpp | 54 +++++++++---------- .../AudioBufferQueueSourceNodeHostObject.h | 1 - .../core/sources/AudioBufferBaseSourceNode.h | 2 +- .../src/core/AudioBufferQueueSourceNode.ts | 6 ++- 8 files changed, 48 insertions(+), 36 deletions(-) diff --git a/.claude/skills/host-objects/SKILL.md b/.claude/skills/host-objects/SKILL.md index 29c9755b8..da12da7ab 100644 --- a/.claude/skills/host-objects/SKILL.md +++ b/.claude/skills/host-objects/SKILL.md @@ -359,6 +359,8 @@ The view aliases native memory for as long as JS keeps it, and the `shared_ptr` `AudioBufferSourceNodeHostObject` hands samples to the audio thread in exactly one place, `acquireBufferContent`, which runs on `start()` and on a `setBuffer()` made after start. A `setBuffer()` before start only records the buffer and publishes its channel count; the node reads nothing until acquire, so every JS write made in between is played and nothing has to track content versions. Acquire detaches the live `getChannelData` views first and then calls `shareForPlayback`, so the per-channel bookkeeping vectors in `AudioBufferHostObject` must be sized in its constructor — sizing them lazily in `shareForPlayback` crashed `detachReturnedChannelData` the moment the order flipped. +`AudioBufferQueueSourceNodeHostObject::enqueueBuffer` reuses the same mechanism: a buffer of the queue's width is shared (`detachReturnedChannelData` then `shareForPlayback`) instead of copied, only a buffer of another width is mixed into a private copy, and the TS wrapper calls `invalidateChannelDataCache` after every enqueue. The stretcher is configured inline on the JS thread by the host base `initStretch`, once per node and with no audio event: the audio thread touches it only once the node has content, and the first content is always scheduled behind that call. Audio events are `FatFunction<72, …>`; a handle, a node pointer, two `shared_ptr`s and one scalar already fill the slot, so derive what you can on the audio thread (the queue event reads the width off its content) instead of capturing it. + ### External memory pressure Call `setExternalMemoryPressure` whenever returning a HostObject or typed array that wraps a large native buffer. This lets the JS GC schedule collection correctly: diff --git a/packages/audiodocs/docs/sources/audio-buffer-queue-source-node.mdx b/packages/audiodocs/docs/sources/audio-buffer-queue-source-node.mdx index e6049f2aa..c849ff64e 100644 --- a/packages/audiodocs/docs/sources/audio-buffer-queue-source-node.mdx +++ b/packages/audiodocs/docs/sources/audio-buffer-queue-source-node.mdx @@ -111,6 +111,10 @@ Adds another buffer to queue. Returns `bufferId` that can be used to identify th #### Returns `string`. +:::tip +The first enqueued buffer sets the number of channels the queue plays at. A buffer with that same number of channels is played directly from its own memory, so enqueuing it costs no copy. A buffer with a different number of channels is first mixed into a copy of the queue's width, which is slower and allocates memory for every such buffer. For best performance, enqueue buffers that all have the same number of channels. +::: + ### `dequeueBuffer` Removes a buffer from the queue. Note that [`onbufferended`](audio-buffer-queue-source-node#onbufferended) event will not be fired for the removed buffers. diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.cpp index 590a858ac..e8b9214fb 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.cpp @@ -81,18 +81,19 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferBaseSourceNodeHostObject, getOutputLatency) { } void AudioBufferBaseSourceNodeHostObject::initStretch(int channelCount, float sampleRate) { - auto handle = node_->handle; + if (stretchHasBeenInit_) { + return; + } + stretchHasBeenInit_ = true; + inputLatency_ = WsolaTimeStretcher::INPUT_LATENCY_MS / 1000.0; outputLatency_ = WsolaTimeStretcher::OUTPUT_LATENCY_MS / 1000.0; auto playbackRateBuffer = std::make_shared( WsolaTimeStretcher::MAX_PLAYBACK_RATE * RENDER_QUANTUM_SIZE, channelCount, sampleRate); - auto event = [handle, node = bufferBaseSourceNode_, playbackRateBuffer, channelCount, sampleRate]( - BaseAudioContext &) { - node->initStretch(static_cast(channelCount), sampleRate, playbackRateBuffer); - }; - bufferBaseSourceNode_->scheduleAudioEvent(std::move(event)); + bufferBaseSourceNode_->initStretch( + static_cast(channelCount), sampleRate, playbackRateBuffer); } } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.h index 886d5c7ca..78c653ce9 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferBaseSourceNodeHostObject.h @@ -40,7 +40,9 @@ class AudioBufferBaseSourceNodeHostObject : public AudioScheduledSourceNodeHostO double inputLatency_ = 0; double outputLatency_ = 0; bool pitchCorrection_; + bool stretchHasBeenInit_ = false; + /// Configures the node's stretcher for content of this shape, inline and only once void initStretch(int channelCount, float sampleRate); }; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp index e4ffc912f..b7983f7a9 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp @@ -77,47 +77,47 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferQueueSourceNodeHostObject, resume) { JSI_HOST_FUNCTION_IMPL(AudioBufferQueueSourceNodeHostObject, enqueueBuffer) { auto audioBufferHostObject = args[0].getObject(runtime).asHostObject(runtime); - // TODO: add optimized memory management for buffer changes, e.g. - // when the same buffer is reused across threads and - // buffer modification is not allowed on JS thread + const auto &samples = audioBufferHostObject->audioBuffer_; + const auto bufferChannelNumber = samples->getNumberOfChannels(); - auto swapBuffer = false; // whether to swap internal node buffer with the new buffer + // The first buffer fixes the width the queue plays at. if (outputChannelNumber_ == 0) { - outputChannelNumber_ = audioBufferHostObject->audioBuffer_->getNumberOfChannels(); - swapBuffer = true; + outputChannelNumber_ = bufferChannelNumber; } - // first buffer defines the output channel number, the rest are mixed to it - auto copiedBuffer = std::make_shared( - audioBufferHostObject->audioBuffer_->getSize(), - outputChannelNumber_, - audioBufferHostObject->audioBuffer_->getSampleRate()); - - copiedBuffer->sum(*audioBufferHostObject->audioBuffer_); + std::shared_ptr content; + + if (bufferChannelNumber == outputChannelNumber_) { + // Played from the JS buffer's own storage: its live getChannelData() views are cut off + // and JS writes go copy-on-write from now on, so nothing can change the samples under + // the node. + audioBufferHostObject->detachReturnedChannelData(); + content = audioBufferHostObject->shareForPlayback(); + } else { + // A buffer of another width is mixed into a private copy of the queue's width. + content = std::make_shared( + samples->getSize(), static_cast(outputChannelNumber_), samples->getSampleRate()); + content->sum(*samples); + } std::shared_ptr tailBuffer = nullptr; if (pitchCorrection_ && !stretchHasBeenInit_) { - initStretch( - static_cast(copiedBuffer->getNumberOfChannels()), copiedBuffer->getSampleRate()); + initStretch(static_cast(outputChannelNumber_), samples->getSampleRate()); auto extraTailFrames = - static_cast((inputLatency_ + outputLatency_) * copiedBuffer->getSampleRate()); + static_cast((inputLatency_ + outputLatency_) * samples->getSampleRate()); tailBuffer = std::make_shared( - extraTailFrames, copiedBuffer->getNumberOfChannels(), copiedBuffer->getSampleRate()); + extraTailFrames, static_cast(outputChannelNumber_), samples->getSampleRate()); tailBuffer->zero(); - stretchHasBeenInit_ = true; } - auto event = [node = bufferQueueSourceNode_, - copiedBuffer, + auto event = [handle = node_->handle, + node = bufferQueueSourceNode_, + content, bufferId = bufferId_, - tailBuffer, - swapBuffer, - outputChannelNumber = outputChannelNumber_](BaseAudioContext &) { - if (swapBuffer) { - node->resizeOutputBuffer(static_cast(outputChannelNumber)); - } - node->enqueueBuffer(copiedBuffer, bufferId, tailBuffer); + tailBuffer](BaseAudioContext &) { + node->resizeOutputBuffer(content->getNumberOfChannels()); + node->enqueueBuffer(content, bufferId, tailBuffer); }; bufferQueueSourceNode_->scheduleAudioEvent(std::move(event)); diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.h index f13e7c966..b1afbd969 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.h @@ -42,7 +42,6 @@ class AudioBufferQueueSourceNodeHostObject : public AudioBufferBaseSourceNodeHos AudioBufferQueueSourceNode *const bufferQueueSourceNode_; size_t bufferId_ = 0; - bool stretchHasBeenInit_ = false; /// Width of the first enqueued buffer; later buffers are mixed to it. 0 until then. size_t outputChannelNumber_ = 0; }; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h index f9f69e063..d3d3e7157 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h @@ -27,7 +27,7 @@ class AudioBufferBaseSourceNode : public AudioScheduledSourceNode { const std::shared_ptr &context, const BaseAudioBufferSourceOptions &options); - /// @note Audio Thread only + /// @note JS Thread only, and only while the node has no content yet void initStretch( size_t channelCount, float sampleRate, diff --git a/packages/react-native-audio-api/src/core/AudioBufferQueueSourceNode.ts b/packages/react-native-audio-api/src/core/AudioBufferQueueSourceNode.ts index c42bdf433..280dce06c 100644 --- a/packages/react-native-audio-api/src/core/AudioBufferQueueSourceNode.ts +++ b/packages/react-native-audio-api/src/core/AudioBufferQueueSourceNode.ts @@ -24,9 +24,13 @@ export default class AudioBufferQueueSourceNode extends AudioBufferBaseSourceNod } public enqueueBuffer(buffer: AudioBuffer): string { - return (this.node as IAudioBufferQueueSourceNode).enqueueBuffer( + const bufferId = (this.node as IAudioBufferQueueSourceNode).enqueueBuffer( buffer.buffer ); + // The node plays the buffer's own storage from now on, so native has cut off the + // views this buffer handed out; the wrapper must stop returning those dead views. + buffer.invalidateChannelDataCache(); + return bufferId; } public dequeueBuffer(bufferId: string): void { From 6b9e6bf835d6d9b7a15fe69a0d08a7a608527a21 Mon Sep 17 00:00:00 2001 From: michal Date: Fri, 16 Oct 2026 16:38:54 +0200 Subject: [PATCH 8/9] refactor: put the logic code deeper in the layers --- .../build-compilation-dependencies/SKILL.md | 2 +- .claude/skills/host-objects/SKILL.md | 11 +- .../effects/ConvolverNodeHostObject.cpp | 2 +- .../sources/AudioBufferHostObject.cpp | 93 +++---------- .../sources/AudioBufferHostObject.h | 56 +------- .../AudioBufferQueueSourceNodeHostObject.cpp | 9 +- .../AudioBufferSourceNodeHostObject.cpp | 31 ++--- .../sources/AudioBufferSourceNodeHostObject.h | 21 ++- .../HostObjects/utils/NodeOptionsParser.h | 5 +- .../core/sources/SharedAudioBuffer.cpp | 75 ++++++++++ .../audioapi/core/sources/SharedAudioBuffer.h | 73 ++++++++++ .../cpp/audioapi/events/AudioEventPayload.h | 2 +- .../cpp/audioapi/utils/AudioArrayBuffer.hpp | 12 +- .../common/cpp/clangd/CMakeLists.txt | 2 - .../common/cpp/test/CMakeLists.txt | 1 + .../test/src/AudioBufferHostObjectStub.cpp | 22 --- .../core/sources/SharedAudioBufferTest.cpp | 129 ++++++++++++++++++ 17 files changed, 338 insertions(+), 208 deletions(-) create mode 100644 packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.cpp create mode 100644 packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h delete mode 100644 packages/react-native-audio-api/common/cpp/test/src/AudioBufferHostObjectStub.cpp create mode 100644 packages/react-native-audio-api/common/cpp/test/src/core/sources/SharedAudioBufferTest.cpp diff --git a/.claude/skills/build-compilation-dependencies/SKILL.md b/.claude/skills/build-compilation-dependencies/SKILL.md index 490fbe3a4..ff16f6800 100644 --- a/.claude/skills/build-compilation-dependencies/SKILL.md +++ b/.claude/skills/build-compilation-dependencies/SKILL.md @@ -321,7 +321,7 @@ CI runs a parallel `cpp-coverage` job via `.github/workflows/cpp-coverage-job.ym - Completely standalone — no Gradle, no Xcode, no prebuilt Android libraries needed - Sources resolved from `node_modules` (symlinked to `packages/` in yarn workspaces) - HostObjects, worklets nodes, AudioContext, and FfmpegDecoder are excluded from the test build -- HostObject **headers** still compile under `RN_AUDIO_API_TEST`; only their `.cpp` bodies are missing. When a test first links core code that constructs one (e.g. `AudioRecorderCallback` → `AudioBufferHostObject`), the linker fails on the constructor. Fix it with a stub translation unit under `test/src/` that defines just the constructors (see `test/src/AudioBufferHostObjectStub.cpp`), never with an `#if RN_AUDIO_API_TEST` test double inside the production header. The static lib also compiles `jsi/jsi.cpp` so `jsi::HostObject`'s virtuals resolve +- The static lib compiles `jsi/jsi.cpp`, so real JSI types (`jsi::HostObject`, `jsi::MutableBuffer`) work in tests. When core code a test links constructs a HostObject (e.g. `AudioRecorderCallback` → `AudioBufferHostObject`), add that HostObject's `.cpp` to the `list(APPEND RNAUDIOAPI_SRC …)` exceptions in `test/CMakeLists.txt` (as for `JsEnumParser.cpp` and `AudioBufferHostObject.cpp`) instead of writing a stub translation unit or an `#if RN_AUDIO_API_TEST` test double - Compile definitions: `RN_AUDIO_API_ENABLE_WORKLETS=0`, `RN_AUDIO_API_TEST=1` (FFmpeg is off because the define is simply absent) - Google Test auto-fetched via `FetchContent` if not installed locally - New test files in `test/src/**/*.cpp` are picked up automatically by glob — no CMakeLists edit needed diff --git a/.claude/skills/host-objects/SKILL.md b/.claude/skills/host-objects/SKILL.md index da12da7ab..ef48dc534 100644 --- a/.claude/skills/host-objects/SKILL.md +++ b/.claude/skills/host-objects/SKILL.md @@ -340,8 +340,7 @@ return result; ```cpp JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { - auto channel = static_cast(args[0].getNumber()); - auto audioArrayBuffer = audioBuffer_->getSharedChannel(channel); + auto audioArrayBuffer = sharedBuffer_->exposeChannel(static_cast(args[0].getNumber())); auto arrayBuffer = jsi::ArrayBuffer(runtime, audioArrayBuffer); auto float32ArrayCtor = runtime.global() @@ -355,11 +354,13 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { } ``` -The view aliases native memory for as long as JS keeps it, and the `shared_ptr` inside the `jsi::ArrayBuffer` keeps that memory alive on its own. When the native side later needs the view to stop aliasing (Web Audio's "acquire the content" on `AudioBufferSourceNode.start()`), it must retain the returned object and neutralise it afterwards — see `detachReturnedChannelData` in the real `AudioBufferHostObject`. +The view aliases native memory for as long as JS keeps it, and the `shared_ptr` inside the `jsi::ArrayBuffer` keeps that memory alive on its own. When the native side later needs the view to stop aliasing (Web Audio's "acquire the content" on `AudioBufferSourceNode.start()`), it must retain the returned storage and replace it afterwards, so the old view keeps the old storage and stops reaching the buffer. -`AudioBufferSourceNodeHostObject` hands samples to the audio thread in exactly one place, `acquireBufferContent`, which runs on `start()` and on a `setBuffer()` made after start. A `setBuffer()` before start only records the buffer and publishes its channel count; the node reads nothing until acquire, so every JS write made in between is played and nothing has to track content versions. Acquire detaches the live `getChannelData` views first and then calls `shareForPlayback`, so the per-channel bookkeeping vectors in `AudioBufferHostObject` must be sized in its constructor — sizing them lazily in `shareForPlayback` crashed `detachReturnedChannelData` the moment the order flipped. +That bookkeeping — copy-on-write against playing nodes, tracking exposed views, the spec's `copyFromChannel` / `copyToChannel` range rules — lives in the engine class `core/sources/SharedAudioBuffer`, not in `AudioBufferHostObject`. An AudioBuffer involves no inter-thread events (every member runs on the JS thread), so nothing forces its logic into the host layer; the host object only unpacks JSI arguments and wraps the returned storage. `SharedAudioBuffer` is unit-tested in `test/src/core/sources/SharedAudioBufferTest.cpp`, which the HostObject layer cannot be (see `build-compilation-dependencies`). Its per-channel flag vectors are sized in the constructor: sizing them lazily on first share crashed the view cut-off the moment the order of the two steps flipped. -`AudioBufferQueueSourceNodeHostObject::enqueueBuffer` reuses the same mechanism: a buffer of the queue's width is shared (`detachReturnedChannelData` then `shareForPlayback`) instead of copied, only a buffer of another width is mixed into a private copy, and the TS wrapper calls `invalidateChannelDataCache` after every enqueue. The stretcher is configured inline on the JS thread by the host base `initStretch`, once per node and with no audio event: the audio thread touches it only once the node has content, and the first content is always scheduled behind that call. Audio events are `FatFunction<72, …>`; a handle, a node pointer, two `shared_ptr`s and one scalar already fill the slot, so derive what you can on the audio thread (the queue event reads the width off its content) instead of capturing it. +`AudioBufferSourceNodeHostObject` holds the assigned `SharedAudioBuffer` and hands samples to the audio thread in exactly one place, `acquireBufferContent`, which runs on `start()` and on a `setBuffer()` made after start. A `setBuffer()` before start only records the buffer and publishes its channel count; the node reads nothing until acquire, so every JS write made in between is played and nothing has to track content versions. `SharedAudioBuffer::acquireContent` both cuts off the live views and returns the storage-sharing buffer the node plays. + +`AudioBufferQueueSourceNodeHostObject::enqueueBuffer` reuses the same mechanism: a buffer of the queue's width is shared through `SharedAudioBuffer::acquireContent` instead of copied, only a buffer of another width is mixed into a private copy, and the TS wrapper calls `invalidateChannelDataCache` after every enqueue. The stretcher is configured inline on the JS thread by the host base `initStretch`, once per node and with no audio event: the audio thread touches it only once the node has content, and the first content is always scheduled behind that call. Audio events are `FatFunction<72, …>`; a handle, a node pointer, two `shared_ptr`s and one scalar already fill the slot, so derive what you can on the audio thread (the queue event reads the width off its content) instead of capturing it. ### External memory pressure diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp index 4f72bd149..fb315daee 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp @@ -53,7 +53,7 @@ JSI_HOST_FUNCTION_IMPL(ConvolverNodeHostObject, setBuffer) { irBytes_ = bufferHostObject->getSizeInBytes(); thisValue.asObject(runtime).setExternalMemoryPressure(runtime, getMemoryPressure()); - setBuffer(bufferHostObject->audioBuffer_); + setBuffer(bufferHostObject->sharedBuffer_->getSamples()); return jsi::Value::undefined(); } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp index bf6b75980..dc8f10a5b 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp @@ -2,18 +2,13 @@ #include -#include #include #include -#include -#include namespace audioapi { AudioBufferHostObject::AudioBufferHostObject(const std::shared_ptr &audioBuffer) - : audioBuffer_(audioBuffer), - channelSharedWithNode_(audioBuffer->getNumberOfChannels(), false), - channelViewHandedOut_(audioBuffer->getNumberOfChannels(), false) { + : sharedBuffer_(std::make_shared(audioBuffer)) { addGetters( JSI_EXPORT_PROPERTY_GETTER(AudioBufferHostObject, sampleRate), JSI_EXPORT_PROPERTY_GETTER(AudioBufferHostObject, length), @@ -26,68 +21,30 @@ AudioBufferHostObject::AudioBufferHostObject(const std::shared_ptr JSI_EXPORT_FUNCTION(AudioBufferHostObject, copyToChannel)); } -AudioBufferHostObject::AudioBufferHostObject(AudioBufferHostObject &&other) noexcept - : HostObject(std::move(other)), - audioBuffer_(std::move(other.audioBuffer_)), - channelSharedWithNode_(std::move(other.channelSharedWithNode_)), - channelViewHandedOut_(std::move(other.channelViewHandedOut_)) {} - -std::shared_ptr AudioBufferHostObject::shareForPlayback() { - channelSharedWithNode_.assign(audioBuffer_->getNumberOfChannels(), true); - return audioBuffer_->shareChannels(); -} - -void AudioBufferHostObject::makeChannelWritable(size_t channel) { - if (channelSharedWithNode_[channel]) { - replaceChannelStorage(channel); - } -} - -void AudioBufferHostObject::replaceChannelStorage(size_t channel) { - // mark the channel as no longer shared with a node, so that future writes to it don't trigger another copy-on-write - audioBuffer_->detachSharedChannel(channel); - channelSharedWithNode_[channel] = false; -} - -void AudioBufferHostObject::detachReturnedChannelData() { - for (size_t channel = 0; channel < channelViewHandedOut_.size(); ++channel) { - if (channelViewHandedOut_[channel]) { - replaceChannelStorage(channel); - channelViewHandedOut_[channel] = false; - } - } -} - JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, sampleRate) { - return {audioBuffer_->getSampleRate()}; + return {sharedBuffer_->getSamples()->getSampleRate()}; } JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, length) { - return {static_cast(audioBuffer_->getSize())}; + return {static_cast(sharedBuffer_->getSamples()->getSize())}; } JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, duration) { - return {audioBuffer_->getDuration()}; + return {sharedBuffer_->getSamples()->getDuration()}; } JSI_PROPERTY_GETTER_IMPL(AudioBufferHostObject, numberOfChannels) { - return {static_cast(audioBuffer_->getNumberOfChannels())}; + return {static_cast(sharedBuffer_->getSamples()->getNumberOfChannels())}; } JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { - auto channel = static_cast(args[0].getNumber()); - // The returned Float32Array is a live, JS-writable view straight into audioBuffer_'s - // storage, so that storage must not be one a node is playing. - makeChannelWritable(channel); - - auto audioArrayBuffer = audioBuffer_->getSharedChannel(channel); + auto audioArrayBuffer = sharedBuffer_->exposeChannel(static_cast(args[0].getNumber())); auto arrayBuffer = jsi::ArrayBuffer(runtime, audioArrayBuffer); auto float32ArrayCtor = runtime.global().getPropertyAsFunction(runtime, "Float32Array"); auto float32Array = float32ArrayCtor.callAsConstructor(runtime, arrayBuffer).getObject(runtime); float32Array.setExternalMemoryPressure(runtime, audioArrayBuffer->size()); - channelViewHandedOut_[channel] = true; return float32Array; } @@ -95,19 +52,11 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, getChannelData) { JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, copyFromChannel) { auto arrayBuffer = args[0].getObject(runtime).getPropertyAsObject(runtime, "buffer").getArrayBuffer(runtime); - auto *destination = reinterpret_cast(arrayBuffer.data(runtime)); - auto destinationLength = arrayBuffer.size(runtime) / sizeof(float); - auto channelNumber = static_cast(args[1].getNumber()); - auto rawStart = args[2].getNumber(); - auto channelSize = audioBuffer_->getSize(); - - // Per spec, an out-of-range (or negative) startInChannel copies nothing, and a - // copy that would run past the end of the channel is truncated rather than throwing. - if (rawStart >= 0 && static_cast(rawStart) < channelSize) { - auto startInChannel = static_cast(rawStart); - auto framesToCopy = std::min(destinationLength, channelSize - startInChannel); - audioBuffer_->getChannel(channelNumber)->copyTo(destination, startInChannel, 0, framesToCopy); - } + sharedBuffer_->copyFromChannel( + reinterpret_cast(arrayBuffer.data(runtime)), + arrayBuffer.size(runtime) / sizeof(float), + static_cast(args[1].getNumber()), + args[2].getNumber()); return jsi::Value::undefined(); } @@ -115,21 +64,11 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, copyFromChannel) { JSI_HOST_FUNCTION_IMPL(AudioBufferHostObject, copyToChannel) { auto arrayBuffer = args[0].getObject(runtime).getPropertyAsObject(runtime, "buffer").getArrayBuffer(runtime); - auto *source = reinterpret_cast(arrayBuffer.data(runtime)); - auto sourceLength = arrayBuffer.size(runtime) / sizeof(float); - auto channelNumber = static_cast(args[1].getNumber()); - // Mutates audioBuffer_ in place, so that channel must not be one a node is playing. - makeChannelWritable(static_cast(channelNumber)); - auto rawStart = args[2].getNumber(); - auto channelSize = audioBuffer_->getSize(); - - // Per spec, an out-of-range (or negative) startInChannel copies nothing, and a - // copy that would run past the end of the channel is truncated rather than throwing. - if (rawStart >= 0 && static_cast(rawStart) < channelSize) { - auto startInChannel = static_cast(rawStart); - auto framesToCopy = std::min(sourceLength, channelSize - startInChannel); - audioBuffer_->getChannel(channelNumber)->copy(source, 0, startInChannel, framesToCopy); - } + sharedBuffer_->copyToChannel( + reinterpret_cast(arrayBuffer.data(runtime)), + arrayBuffer.size(runtime) / sizeof(float), + static_cast(args[1].getNumber()), + args[2].getNumber()); return jsi::Value::undefined(); } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h index 0369a2e16..bcb231fd8 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.h @@ -1,61 +1,30 @@ #pragma once +#include #include #include #include -#include #include -#include #include -#include -#include namespace audioapi { using namespace facebook; -/// @brief JS-facing AudioBuffer. Source nodes play its channel storage directly instead of -/// deep-copying it (see `shareForPlayback`), so the one rule this class enforces is that the -/// JS thread never writes into storage a node may be reading: every write path first gives -/// the touched channel fresh storage (copy-on-write) and leaves the old one to the nodes. class AudioBufferHostObject : public HostObject { public: - std::shared_ptr audioBuffer_; + std::shared_ptr sharedBuffer_; explicit AudioBufferHostObject(const std::shared_ptr &audioBuffer); AudioBufferHostObject(const AudioBufferHostObject &) = delete; AudioBufferHostObject &operator=(const AudioBufferHostObject &) = delete; - AudioBufferHostObject(AudioBufferHostObject &&other) noexcept; - AudioBufferHostObject &operator=(AudioBufferHostObject &&other) noexcept { - if (this != &other) { - HostObject::operator=(std::move(other)); - audioBuffer_ = std::move(other.audioBuffer_); - channelSharedWithNode_ = std::move(other.channelSharedWithNode_); - channelViewHandedOut_ = std::move(other.channelViewHandedOut_); - } - return *this; - } + AudioBufferHostObject(AudioBufferHostObject &&other) noexcept = default; + AudioBufferHostObject &operator=(AudioBufferHostObject &&other) noexcept = default; ~AudioBufferHostObject() override = default; [[nodiscard]] size_t getSizeInBytes() const { - return audioBuffer_->getSize() * audioBuffer_->getNumberOfChannels() * sizeof(float); - } - - /// @brief from this call both the js and native side can read the same channel storage. - /// The JS side is copy-on-write, so it will get a fresh copy if it writes into it while a node is reading it. - [[nodiscard]] std::shared_ptr shareForPlayback(); - - /// @brief Web Audio's "acquire the content" step for the views handed out by - /// `getChannelData`. Call once playback of this buffer has been scheduled. Every - /// channel with a previously returned Float32Array gets fresh storage, so those views - /// stop pointing to `audioBuffer_` and the next `getChannelData` call hands out a fresh view. - void detachReturnedChannelData(); - - /// @brief Whether any `getChannelData` view is live, i.e. handed out since the last - /// `detachReturnedChannelData`. - [[nodiscard]] bool hasReturnedChannelData() const { - return std::ranges::any_of(channelViewHandedOut_, [](bool handedOut) { return handedOut; }); + return sharedBuffer_->getSizeInBytes(); } JSI_PROPERTY_GETTER_DECL(sampleRate); @@ -66,20 +35,5 @@ class AudioBufferHostObject : public HostObject { JSI_HOST_FUNCTION_DECL(getChannelData); JSI_HOST_FUNCTION_DECL(copyFromChannel); JSI_HOST_FUNCTION_DECL(copyToChannel); - - private: - /// Copy-on-write: call before exposing or mutating a channel from JS. If a node may be - /// reading that channel's storage, the buffer gets a private copy of it first. - void makeChannelWritable(size_t channel); - - /// Replaces the channel's storage with a copy and records that nothing shares it yet. - void replaceChannelStorage(size_t channel); - - /// One flag per channel: true while a node handed out by `shareForPlayback` may still - /// be reading that channel's current storage. - std::vector channelSharedWithNode_; - /// true once `getChannelData` handed out a view of that channel's - /// current storage, cleared by `detachReturnedChannelData`. - std::vector channelViewHandedOut_; }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp index b7983f7a9..e3d31a96b 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferQueueSourceNodeHostObject.cpp @@ -77,7 +77,8 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferQueueSourceNodeHostObject, resume) { JSI_HOST_FUNCTION_IMPL(AudioBufferQueueSourceNodeHostObject, enqueueBuffer) { auto audioBufferHostObject = args[0].getObject(runtime).asHostObject(runtime); - const auto &samples = audioBufferHostObject->audioBuffer_; + const auto &sharedBuffer = audioBufferHostObject->sharedBuffer_; + const auto &samples = sharedBuffer->getSamples(); const auto bufferChannelNumber = samples->getNumberOfChannels(); // The first buffer fixes the width the queue plays at. @@ -88,11 +89,7 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferQueueSourceNodeHostObject, enqueueBuffer) { std::shared_ptr content; if (bufferChannelNumber == outputChannelNumber_) { - // Played from the JS buffer's own storage: its live getChannelData() views are cut off - // and JS writes go copy-on-write from now on, so nothing can change the samples under - // the node. - audioBufferHostObject->detachReturnedChannelData(); - content = audioBufferHostObject->shareForPlayback(); + content = sharedBuffer->acquireContent(); } else { // A buffer of another width is mixed into a private copy of the queue's width. content = std::make_shared( diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp index e40e68017..6e4e75cd7 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -139,11 +140,8 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, start) { } void AudioBufferSourceNodeHostObject::acquireBufferContent() { - if (assignedBufferHostObject_ != nullptr) { - assignedBufferHostObject_->detachReturnedChannelData(); - } - - auto buffers = prepareNodeBuffers(assignedBufferHostObject_); + auto buffers = + prepareNodeBuffers(assignedBuffer_ == nullptr ? nullptr : assignedBuffer_->acquireContent()); auto event = [handle = node_->handle, node = audioBufferSourceNode_, buffers](BaseAudioContext &) { node->setBuffer(buffers.nodeBuffer, buffers.audioBuffer); @@ -159,17 +157,17 @@ JSI_HOST_FUNCTION_IMPL(AudioBufferSourceNodeHostObject, setBuffer) { thisValue.asObject(runtime).setExternalMemoryPressure( runtime, getMemoryPressure() + bufferHostObject->getSizeInBytes()); - setBuffer(bufferHostObject); + setBuffer(bufferHostObject->sharedBuffer_); } return jsi::Value::undefined(); } AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::prepareNodeBuffers( - const std::shared_ptr &bufferHostObject) { + const std::shared_ptr &acquiredContent) { NodeBuffers buffers; - if (bufferHostObject == nullptr) { + if (acquiredContent == nullptr) { buffers.nodeBuffer = nullptr; buffers.audioBuffer = std::make_shared( RENDER_QUANTUM_SIZE, @@ -178,14 +176,12 @@ AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::pr return buffers; } - const auto &buffer = bufferHostObject->audioBuffer_; if (pitchCorrection_) { - initStretch(static_cast(buffer->getNumberOfChannels()), buffer->getSampleRate()); + initStretch( + static_cast(acquiredContent->getNumberOfChannels()), acquiredContent->getSampleRate()); } - // The node reads the JS-facing storage directly; from now on JS writes to it go - // copy-on-write, so nothing can change the samples under the node. - buffers.nodeBuffer = bufferHostObject->shareForPlayback(); + buffers.nodeBuffer = acquiredContent; buffers.audioBuffer = std::make_shared( RENDER_QUANTUM_SIZE, @@ -194,15 +190,14 @@ AudioBufferSourceNodeHostObject::NodeBuffers AudioBufferSourceNodeHostObject::pr return buffers; } -void AudioBufferSourceNodeHostObject::setBuffer( - const std::shared_ptr &bufferHostObject) { - assignedBufferHostObject_ = bufferHostObject; +void AudioBufferSourceNodeHostObject::setBuffer(const std::shared_ptr &buffer) { + assignedBuffer_ = buffer; // Publish the new output width on the host thread before renegotiation so // MAX / CLAMPED_MAX downstream nodes see it immediately - const size_t newOutputChannelNumber = bufferHostObject == nullptr + const size_t newOutputChannelNumber = buffer == nullptr ? AudioBufferSourceOptions::kDefaultOutputChannelNumber - : bufferHostObject->audioBuffer_->getNumberOfChannels(); + : buffer->getSamples()->getNumberOfChannels(); if (newOutputChannelNumber != audioBufferSourceNode_->getOutputChannelNumber()) { audioBufferSourceNode_->setOutputChannelNumber(newOutputChannelNumber); renegotiate(); diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h index d02f6f341..1f986ff4a 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/sources/AudioBufferSourceNodeHostObject.h @@ -11,7 +11,7 @@ using namespace facebook; struct AudioBufferSourceOptions; class BaseAudioContext; -class AudioBufferHostObject; +class SharedAudioBuffer; class AudioBufferSourceNode; class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObject { @@ -51,31 +51,28 @@ class AudioBufferSourceNodeHostObject : public AudioBufferBaseSourceNodeHostObje double loopStart_; double loopEnd_; - /// The JS-facing buffer assigned last, held here until "acquire the content" hands its - /// samples to the audio thread. Until then the node has not seen it, so JS can keep - /// writing into it and every write is honoured. Null when cleared. - std::shared_ptr assignedBufferHostObject_; + /// The buffer assigned last, held here until "acquire the content" hands its samples to + /// the audio thread. Until then the node has not seen it, so JS can keep writing into it + /// and every write is honoured. Null when cleared. + std::shared_ptr assignedBuffer_; bool hasBeenStarted_ = false; - /// The samples the node will read (storage shared with the JS-facing buffer) plus its + /// The samples the node will read (storage shared with the assigned buffer) plus its /// render-quantum scratch buffer. struct NodeBuffers { std::shared_ptr nodeBuffer; std::shared_ptr audioBuffer; }; - NodeBuffers prepareNodeBuffers(const std::shared_ptr &bufferHostObject); + NodeBuffers prepareNodeBuffers(const std::shared_ptr &acquiredContent); /// Records the assignment and publishes the buffer's channel count to the graph. The /// samples themselves reach the node only through `acquireBufferContent`, which runs /// right away when the node has already started. - void setBuffer(const std::shared_ptr &bufferHostObject); + void setBuffer(const std::shared_ptr &buffer); /// Web Audio's "acquire the content" step, the single point where the node receives - /// samples: runs on start(), and on setBuffer() once already started. Cuts off every - /// live getChannelData() view, then hands the node the buffer's content as it is now, - /// so JS writes made before this moment are played and later ones are not. - /// https://webaudio.github.io/web-audio-api/#acquire-the-content + /// samples: runs on start(), and on setBuffer() once already started. void acquireBufferContent(); }; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h index 8e6514468..75985cd21 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/utils/NodeOptionsParser.h @@ -164,8 +164,9 @@ inline ConvolverOptions parseConvolverOptions( auto bufferValue = optionsObject.getProperty(runtime, "buffer"); if (bufferValue.isObject() && bufferValue.getObject(runtime).isHostObject(runtime)) { - options.buffer = - bufferValue.getObject(runtime).asHostObject(runtime)->audioBuffer_; + options.buffer = bufferValue.getObject(runtime) + .asHostObject(runtime) + ->sharedBuffer_->getSamples(); } } return options; diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.cpp b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.cpp new file mode 100644 index 000000000..b8665db0f --- /dev/null +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.cpp @@ -0,0 +1,75 @@ +#include + +#include +#include +#include +#include + +namespace audioapi { + +SharedAudioBuffer::SharedAudioBuffer(std::shared_ptr samples) + : samples_(std::move(samples)), + channelSharedWithNode_(samples_->getNumberOfChannels(), false), + channelExposed_(samples_->getNumberOfChannels(), false) {} + +std::shared_ptr SharedAudioBuffer::exposeChannel(size_t channel) { + makeChannelWritable(channel); + channelExposed_[channel] = true; + return samples_->getSharedChannel(channel); +} + +void SharedAudioBuffer::copyFromChannel( + float *destination, + size_t destinationLength, + size_t channel, + double startInChannel) const { + auto channelSize = samples_->getSize(); + if (startInChannel < 0 || startInChannel >= static_cast(channelSize)) { + return; + } + + auto start = static_cast(startInChannel); + auto framesToCopy = std::min(destinationLength, channelSize - start); + samples_->getChannel(channel)->copyTo(destination, start, 0, framesToCopy); +} + +void SharedAudioBuffer::copyToChannel( + const float *source, + size_t sourceLength, + size_t channel, + double startInChannel) { + auto channelSize = samples_->getSize(); + if (startInChannel < 0 || startInChannel >= static_cast(channelSize)) { + return; + } + + makeChannelWritable(channel); + auto start = static_cast(startInChannel); + auto framesToCopy = std::min(sourceLength, channelSize - start); + samples_->getChannel(channel)->copy(source, 0, start, framesToCopy); +} + +std::shared_ptr SharedAudioBuffer::acquireContent() { + for (size_t channel = 0; channel < channelExposed_.size(); ++channel) { + if (channelExposed_[channel]) { + replaceChannelStorage(channel); + channelExposed_[channel] = false; + } + } + + channelSharedWithNode_.assign(samples_->getNumberOfChannels(), true); + return samples_->shareChannels(); +} + +void SharedAudioBuffer::makeChannelWritable(size_t channel) { + if (channelSharedWithNode_[channel]) { + replaceChannelStorage(channel); + } +} + +void SharedAudioBuffer::replaceChannelStorage(size_t channel) { + samples_->detachSharedChannel(channel); + channelSharedWithNode_[channel] = false; +} + +} // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h new file mode 100644 index 000000000..79be35ea3 --- /dev/null +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h @@ -0,0 +1,73 @@ +#pragma once + +#include +#include + +#include +#include +#include + +namespace audioapi { + +/// @brief Samples of an AudioBuffer, shared between the control thread and the source +/// nodes playing them. Nodes read the channel storage directly instead of deep-copying it, +/// so a js trying to write invokes copy-on-write and leaves the old one to the nodes. +/// All members must be called on the JS thread. +class SharedAudioBuffer { + public: + explicit SharedAudioBuffer(std::shared_ptr samples); + + /// @brief For reading only: writing through it bypasses copy-on-write and can change + /// samples a node is playing. Write with `copyToChannel` or through `exposeChannel`. + [[nodiscard]] const std::shared_ptr &getSamples() const { + return samples_; + } + + [[nodiscard]] size_t getSizeInBytes() const { + return samples_->getSize() * samples_->getNumberOfChannels() * sizeof(float); + } + + /// @brief Storage of @p channel for a JS-writable view (`getChannelData`). Writes through + /// it reach this buffer until the next `acquireContent` cuts the view off. + [[nodiscard]] std::shared_ptr exposeChannel(size_t channel); + + /// @brief An out-of-range (or negative) @p startInChannel copies nothing, and a copy that + /// would run past the end of the channel is truncated rather than failing. @p startInChannel + /// is the unconverted JS number, so a negative value is rejected instead of wrapping around. + /// https://webaudio.github.io/web-audio-api/#dom-audiobuffer-copyfromchannel + void copyFromChannel( + float *destination, + size_t destinationLength, + size_t channel, + double startInChannel) const; + + /// @brief Same range rules as `copyFromChannel`. + void + copyToChannel(const float *source, size_t sourceLength, size_t channel, double startInChannel); + + /// @brief Web Audio's "acquire the content" step, run when a source node schedules + /// playback of this buffer. Cuts off every view handed out by `exposeChannel`, then + /// returns a buffer sharing the current storage for the node to play, so JS writes made + /// before this call are played and later ones are not. + /// https://webaudio.github.io/web-audio-api/#acquire-the-content + [[nodiscard]] std::shared_ptr acquireContent(); + + private: + std::shared_ptr samples_; + + /// One flag per channel: true while a buffer returned by `acquireContent` may still be + /// reading that channel's current storage. + std::vector channelSharedWithNode_; + /// One flag per channel: true once `exposeChannel` handed out that channel's current + /// storage, cleared when `acquireContent` cuts the view off. + std::vector channelExposed_; + + /// Moves @p channel to fresh storage if a node may be reading its current one; afterwards + /// the channel is safe to write or expose. + void makeChannelWritable(size_t channel); + + /// Gives the channel fresh storage holding a copy of its samples, shared with nothing. + void replaceChannelStorage(size_t channel); +}; + +} // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/events/AudioEventPayload.h b/packages/react-native-audio-api/common/cpp/audioapi/events/AudioEventPayload.h index f2d9a4059..27778b8f0 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/events/AudioEventPayload.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/events/AudioEventPayload.h @@ -80,7 +80,7 @@ struct AudioReadyPayload { facebook::jsi::Object toJsiObject(facebook::jsi::Runtime &rt) const { facebook::jsi::Object obj(rt); - const bool hasBuffer = buffer != nullptr && buffer->audioBuffer_ != nullptr; + const bool hasBuffer = buffer != nullptr && buffer->sharedBuffer_ != nullptr; if (hasBuffer) { obj.setProperty(rt, "buffer", facebook::jsi::Object::createFromHostObject(rt, buffer)); diff --git a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioArrayBuffer.hpp b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioArrayBuffer.hpp index 4094d7ae1..535c19cb8 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioArrayBuffer.hpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/utils/AudioArrayBuffer.hpp @@ -2,21 +2,13 @@ #include -#if !RN_AUDIO_API_TEST #include -using JsiBuffer = facebook::jsi::MutableBuffer; -#else -// Dummy class to inherit from nothing if testing -struct JsiBuffer { - virtual size_t size() const = 0; - virtual uint8_t *data() = 0; -}; -#endif namespace audioapi { template -class AlignedAudioArrayBuffer : public JsiBuffer, public AlignedAudioArray { +class AlignedAudioArrayBuffer : public facebook::jsi::MutableBuffer, + public AlignedAudioArray { public: explicit AlignedAudioArrayBuffer(size_t size) : AlignedAudioArray(size) {}; AlignedAudioArrayBuffer(const float *data, size_t size) diff --git a/packages/react-native-audio-api/common/cpp/clangd/CMakeLists.txt b/packages/react-native-audio-api/common/cpp/clangd/CMakeLists.txt index 89c173456..6b9131291 100644 --- a/packages/react-native-audio-api/common/cpp/clangd/CMakeLists.txt +++ b/packages/react-native-audio-api/common/cpp/clangd/CMakeLists.txt @@ -5,8 +5,6 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS ON) set(CMAKE_CXX_STANDARD 20) set(COMMON_CPP_DIR ${CMAKE_SOURCE_DIR}/..) -# =1 swaps real types for dummy stand-ins too (e.g. JsiBuffer), so production -# code needs =0; only test/*.cpp needs =1 — split via separate targets below. set(RN_AUDIO_API_TEST 0) set(REACT_NATIVE_DIR ../../../../../node_modules/react-native) diff --git a/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt b/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt index f9a76bd33..2ae3081ee 100644 --- a/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt +++ b/packages/react-native-audio-api/common/cpp/test/CMakeLists.txt @@ -45,6 +45,7 @@ list(FILTER RNAUDIOAPI_SRC EXCLUDE REGEX ".*/audioapi/HostObjects/.*\\.cpp$") list(APPEND RNAUDIOAPI_SRC "${REACT_NATIVE_AUDIO_API_DIR}/common/cpp/audioapi/HostObjects/utils/JsEnumParser.cpp" + "${REACT_NATIVE_AUDIO_API_DIR}/common/cpp/audioapi/HostObjects/sources/AudioBufferHostObject.cpp" ) # AudioThreadGuard provides global operator new/delete overrides. Compile it into diff --git a/packages/react-native-audio-api/common/cpp/test/src/AudioBufferHostObjectStub.cpp b/packages/react-native-audio-api/common/cpp/test/src/AudioBufferHostObjectStub.cpp deleted file mode 100644 index 619b6db9b..000000000 --- a/packages/react-native-audio-api/common/cpp/test/src/AudioBufferHostObjectStub.cpp +++ /dev/null @@ -1,22 +0,0 @@ -#include - -#include -#include - -/// The test build compiles no HostObject sources, yet core code it does compile -/// (AudioRecorderCallback, ContextPromiseResolver) constructs an AudioBufferHostObject -/// when a recording or decode finishes. These definitions satisfy the linker in place of -/// AudioBufferHostObject.cpp, which cannot be compiled here: under RN_AUDIO_API_TEST an -/// AudioArrayBuffer is not a jsi::MutableBuffer (see AudioArrayBuffer.hpp), so its JSI -/// accessor bodies do not build. The stub registers no accessors, and no test ever -/// reaches the object through a JS runtime. - -namespace audioapi { - -AudioBufferHostObject::AudioBufferHostObject(const std::shared_ptr &audioBuffer) - : audioBuffer_(audioBuffer) {} - -AudioBufferHostObject::AudioBufferHostObject(AudioBufferHostObject &&other) noexcept - : HostObject(std::move(other)), audioBuffer_(std::move(other.audioBuffer_)) {} - -} // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/test/src/core/sources/SharedAudioBufferTest.cpp b/packages/react-native-audio-api/common/cpp/test/src/core/sources/SharedAudioBufferTest.cpp new file mode 100644 index 000000000..05bc4fbbb --- /dev/null +++ b/packages/react-native-audio-api/common/cpp/test/src/core/sources/SharedAudioBufferTest.cpp @@ -0,0 +1,129 @@ +#include +#include +#include + +#include +#include + +using namespace audioapi; + +// NOLINTBEGIN +static constexpr size_t BUF_SIZE = 8; +static constexpr float SR = 44100.0f; + +class SharedAudioBufferTest : public ::testing::Test { + protected: + std::shared_ptr samples = std::make_shared(BUF_SIZE, 2, SR); + SharedAudioBuffer buffer{samples}; + + void SetUp() override { + fill(*samples, 0, 0.5f); + fill(*samples, 1, 0.25f); + } + + static void fill(AudioBuffer &target, size_t channel, float value) { + for (size_t i = 0; i < target.getSize(); ++i) { + (*target.getChannel(channel))[i] = value; + } + } + + void writeToChannel(size_t channel, float value, double startInChannel = 0) { + std::vector source(BUF_SIZE, value); + buffer.copyToChannel(source.data(), source.size(), channel, startInChannel); + } + + static void expectChannel(const AudioBuffer &target, size_t channel, float value) { + for (size_t i = 0; i < target.getSize(); ++i) { + EXPECT_FLOAT_EQ((*target.getChannel(channel))[i], value) + << "channel " << channel << " frame " << i; + } + } +}; + +TEST_F(SharedAudioBufferTest, AcquireContentSharesStorageInsteadOfCopying) { + auto content = buffer.acquireContent(); + + EXPECT_EQ(content->getSharedChannel(0), samples->getSharedChannel(0)); + EXPECT_EQ(content->getSharedChannel(1), samples->getSharedChannel(1)); +} + +TEST_F(SharedAudioBufferTest, CopyToChannelAfterAcquireDoesNotReachThePlayedContent) { + auto content = buffer.acquireContent(); + + writeToChannel(0, 1.0f); + + expectChannel(*content, 0, 0.5f); + expectChannel(*buffer.getSamples(), 0, 1.0f); + EXPECT_EQ(content->getSharedChannel(1), buffer.getSamples()->getSharedChannel(1)) + << "Only the written channel is copied."; +} + +TEST_F(SharedAudioBufferTest, CopyToChannelWithoutAcquireWritesInPlace) { + auto storageBefore = samples->getSharedChannel(0); + + writeToChannel(0, 1.0f); + + EXPECT_EQ(samples->getSharedChannel(0), storageBefore); + expectChannel(*samples, 0, 1.0f); +} + +TEST_F(SharedAudioBufferTest, WritesThroughAnExposedViewBeforeAcquireArePlayed) { + auto view = buffer.exposeChannel(0); + (*view)[2] = 1.0f; + + auto content = buffer.acquireContent(); + + EXPECT_FLOAT_EQ((*content->getChannel(0))[2], 1.0f); +} + +TEST_F(SharedAudioBufferTest, AcquireContentCutsOffExposedViews) { + auto view = buffer.exposeChannel(0); + auto content = buffer.acquireContent(); + + (*view)[2] = 1.0f; + + expectChannel(*content, 0, 0.5f); + expectChannel(*buffer.getSamples(), 0, 0.5f); + EXPECT_NE(buffer.exposeChannel(0), view) << "The next view must point to fresh storage."; +} + +TEST_F(SharedAudioBufferTest, ExposingAChannelAfterAcquireDoesNotHandOutPlayedStorage) { + auto content = buffer.acquireContent(); + + auto view = buffer.exposeChannel(1); + (*view)[0] = 1.0f; + + expectChannel(*content, 1, 0.25f); +} + +TEST_F(SharedAudioBufferTest, ExposingTheSameChannelTwiceReusesItsStorage) { + auto first = buffer.exposeChannel(0); + + EXPECT_EQ(buffer.exposeChannel(0), first); +} + +TEST_F(SharedAudioBufferTest, OutOfRangeStartCopiesNothing) { + writeToChannel(0, 1.0f, -1); + writeToChannel(0, 1.0f, BUF_SIZE); + + expectChannel(*samples, 0, 0.5f); + + std::vector destination(BUF_SIZE, -1.0f); + buffer.copyFromChannel(destination.data(), destination.size(), 0, -1); + buffer.copyFromChannel(destination.data(), destination.size(), 0, BUF_SIZE); + + EXPECT_EQ(destination, std::vector(BUF_SIZE, -1.0f)); +} + +TEST_F(SharedAudioBufferTest, CopiesPastTheEndOfTheChannelAreTruncated) { + writeToChannel(0, 1.0f, BUF_SIZE - 2); + + std::vector destination(BUF_SIZE, -1.0f); + buffer.copyFromChannel(destination.data(), destination.size(), 0, BUF_SIZE - 3); + + EXPECT_FLOAT_EQ(destination[0], 0.5f); + EXPECT_FLOAT_EQ(destination[1], 1.0f); + EXPECT_FLOAT_EQ(destination[2], 1.0f); + EXPECT_FLOAT_EQ(destination[3], -1.0f) << "Frames past the channel end stay untouched."; +} +// NOLINTEND From f653784aa5faa11fe09f12d9989e0d99a54bb2ef Mon Sep 17 00:00:00 2001 From: michal Date: Thu, 22 Oct 2026 11:20:37 +0200 Subject: [PATCH 9/9] refactor: comments --- .../audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp | 2 +- .../audioapi/HostObjects/effects/ConvolverNodeHostObject.h | 2 +- .../common/cpp/audioapi/core/sources/AudioBufferSourceNode.h | 4 ---- .../common/cpp/audioapi/core/sources/SharedAudioBuffer.h | 4 +--- .../common/cpp/audioapi/types/NodeOptions.h | 2 +- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp index fb315daee..b0f55e1e7 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.cpp @@ -58,7 +58,7 @@ JSI_HOST_FUNCTION_IMPL(ConvolverNodeHostObject, setBuffer) { return jsi::Value::undefined(); } -void ConvolverNodeHostObject::setBuffer(const std::shared_ptr &buffer) { +void ConvolverNodeHostObject::setBuffer(const std::shared_ptr &buffer) { if (buffer == nullptr) { return; } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.h b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.h index e363f85ec..7301b64bc 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/HostObjects/effects/ConvolverNodeHostObject.h @@ -33,6 +33,6 @@ class ConvolverNodeHostObject : public AudioNodeHostObject { bool normalize_; size_t irBytes_ = 0; - void setBuffer(const std::shared_ptr &buffer); + void setBuffer(const std::shared_ptr &buffer); }; } // namespace audioapi diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h index 64fd9ab00..4541bd9cc 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.h @@ -33,10 +33,6 @@ class AudioBufferSourceNode : public AudioBufferBaseSourceNode { return loopEnd_; } - /// @brief Hands the current buffers to the disposer and installs the new ones. Serves - /// both a JS `buffer` assignment and the "acquire the content" refresh on start(). - /// Loop bounds are left untouched: the effective loop end is resolved against the - /// buffer at render time, so a stale `loopEnd_` cannot outrun a shorter buffer. /// @note Audio Thread only void setBuffer( const std::shared_ptr &buffer, diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h index 79be35ea3..d2622f6fb 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/sources/SharedAudioBuffer.h @@ -17,9 +17,7 @@ class SharedAudioBuffer { public: explicit SharedAudioBuffer(std::shared_ptr samples); - /// @brief For reading only: writing through it bypasses copy-on-write and can change - /// samples a node is playing. Write with `copyToChannel` or through `exposeChannel`. - [[nodiscard]] const std::shared_ptr &getSamples() const { + [[nodiscard]] std::shared_ptr getSamples() const { return samples_; } diff --git a/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h b/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h index 050949a0d..d8cc8500e 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h +++ b/packages/react-native-audio-api/common/cpp/audioapi/types/NodeOptions.h @@ -121,7 +121,7 @@ struct PannerOptions : public AudioNodeOptions { struct ConvolverOptions : AudioNodeOptions { bool disableNormalization = false; - std::shared_ptr buffer = nullptr; + std::shared_ptr buffer = nullptr; ConvolverOptions() { requiresTailProcessing = true;