Repository navigation
Conversation
A2FastModel::process() handled a call with more frames than the maximum buffer size by calling SetMaxBufferSize(). That is memory-safe, but it allocates on the audio thread, and SetMaxBufferSize() reinitialises every history, so the model's state is dropped mid-stream: after such calls, example_models/A2.nam's output differed by up to 0.1 from what the host would get by making calls of the maximum buffer size itself. Process such a call as consecutive calls of at most the maximum buffer size instead. A model that was never Reset() still sizes itself for its first call, as before; it has no state to keep. The new test processes 100-frame calls with a 32-frame maximum buffer size, for A2-Lite and A2-Full, and requires the output to equal that of 32/32/32/4-frame calls, with no allocation. Without the fix it fails on the first call with 58 allocations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related: sdatkinson#303, #NNN (the WaveNet half)
Summary
When a call has more frames than the reservation,
A2FastModel::process()calls
SetMaxBufferSize(num_frames)(a2_fast.cpp:735–736). That ismemory-safe, but it allocates on the audio thread, and
SetMaxBufferSize()re-zeroes every layer and head history (
:302–337), so the model loses itsstate mid-stream. After such calls,
A2.nam's output differed by up to 0.08(A2-Lite) and 0.04 (A2-Full) from the same audio processed in
reservation-sized calls.
This PR splits the call into consecutive calls of at most the reserved size,
which is what #NNN does for the generic WaveNet. A model that was never
Reset()(size 0) still sizes itself on its first call, as before, since ithas no state to lose.
Tests
test_a2_fast.cppgetstest_oversized_call_matches_consecutive_callsforA2-Lite and A2-Full. With a 32-frame reservation and 100-frame calls, the output
must equal that of 32/32/32/4-frame calls exactly, and no call may allocate. On
mainit fails on the first call:A2FastModel<3>::process oversized - Expected 0 allocations … Got 58 allocations.One stale comment in
test_process_realtime_safe, which said the grow path"would legitimately reallocate", now points at the new test.
Verification
NAM_USE_INLINE_GEMM).run_testspasses with-DNAM_ENABLE_A2_FAST=OFF(the test compiles out) and under ASan+UBSan.
libstdc++ and with
-DNAM_USE_LIBCXX_LINUX=ON: CI's steps pass in both Debugbuilds.
run_testsalso passes with-DNAM_ENABLE_A2_FAST=OFF.run_testspasses.#NNN are byte-identical to
main.Compatibility
Hosts that stay within their reservation see no change. Hosts that exceed it
now keep the model's state and no longer allocate on the audio thread.
Not covered
there if Planar NEON kernels, >= 2x faster on Apple Silicon & Raspberry Pi, bit-identical to a2_fast sdatkinson/NeuralAmpModelerCore#313 lands first.