Skip to content

Leave the graph in a functional state if ring buffer allocation fails #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sbooth merged 1 commit into from
Jul 14, 2026
Merged

Leave the graph in a functional state if ring buffer allocation fails #958

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions Sources/CSFBAudioEngine/Player/AudioPlayer.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2461,6 +2461,21 @@ Flags clearFlags(Flags flags, std::memory_order order = std::memory_order_acq_re

os_log_debug(log_, "Reconfiguring audio processing graph for %{public}@", stringDescribingAVAudioFormat(format));

// Allocate a temporary ring buffer for the new format before touching the engine or graph
spsc::AudioRingBuffer ringBuffer;
if (!ringBuffer.allocate(*(format.streamDescription), ringBufferCapacity)) {
os_log_error(log_,
"Unable to create audio ring buffer: spsc::AudioRingBuffer::allocate failed with format "
"%{public}@ and capacity %zu",
SFBASBDFormatDescription(format.streamDescription), ringBufferCapacity);
if (error != nullptr) {
*error = [NSError errorWithDomain:SFBAudioPlayerErrorDomain
code:SFBAudioPlayerErrorCodeInternalError
userInfo:nil];
}
return false;
}

std::lock_guard lock{engineMutex_};

// Even if the engine isn't running, call -stop to force release of any render resources
Expand All @@ -2476,19 +2491,9 @@ Flags clearFlags(Flags flags, std::memory_order order = std::memory_order_acq_re
outputBus:0] firstObject];
[engine_ disconnectNodeOutput:sourceNode_];

// Allocate the ring buffer for the new format
if (!audioRingBuffer_.allocate(*(format.streamDescription), ringBufferCapacity)) {
os_log_error(log_,
"Unable to create audio ring buffer: spsc::AudioRingBuffer::allocate failed with format "
"%{public}@ and capacity %zu",
SFBASBDFormatDescription(format.streamDescription), ringBufferCapacity);
if (error != nullptr) {
*error = [NSError errorWithDomain:SFBAudioPlayerErrorDomain
code:SFBAudioPlayerErrorCodeInternalError
userInfo:nil];
}
return false;
}
// Adopt the new ring buffer
// The move is not thread-safe but the engine is stopped
audioRingBuffer_ = std::move(ringBuffer);

// Reconnect the source node to the next node in the processing chain
// This is the mixer node in the default configuration, but additional nodes may
Expand Down Expand Up @@ -2522,12 +2527,10 @@ Flags clearFlags(Flags flags, std::memory_order order = std::memory_order_acq_re
if (bits::is_set(prevState, Flags::engineIsRunning)) {
if (NSError *startError = nil; ![engine_ startAndReturnError:&startError]) {
os_log_error(log_, "Error starting AVAudioEngine: %{public}@", startError);
// TODO: Re-evaluate whether failure to start AVAudioEngine during reconfiguration
// should be treated as a fatal error or handled as a recoverable condition,
// and document the chosen and tested behavior.
if (error != nullptr) {
*error = startError;
}
// Engine failed to (re)start
return false;
}

Expand Down
Loading