Skip to content

Conversation

@sbooth
Copy link
Owner

@sbooth sbooth commented Nov 13, 2025

No documentation for AVAudioEngine that I can find states that the object may only be used from a single thread/queue. On the other hand, no documentation that I can find states that it is safe to use the object from multiple threads/queues either.

AVAudioEngine appears to use mutexen internally to protect graph modifications, so this PR removes the extra logic and overhead associated with the audio engine isolation dispatch queue.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the audio engine isolation dispatch queue (mEngineQueue) that was previously used to serialize access to the AVAudioEngine instance. The change is based on the assumption that AVAudioEngine uses internal mutexes for thread safety.

Key Changes:

  • Replaced the withEngine: callback-based API with a direct audioEngine readonly property for accessing the underlying AVAudioEngine
  • Removed all dispatch_async_and_wait(mEngineQueue, ...) calls throughout the codebase, simplifying the code and reducing overhead
  • Removed the SFBAudioPlayerAVAudioEngineBlock typedef that was used for the callback pattern

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
Sources/CSFBAudioEngine/include/SFBAudioEngine/SFBAudioPlayer.h Replaced withEngine: method with audioEngine property; removed block typedef; updated documentation
Sources/CSFBAudioEngine/Player/SFBAudioPlayer.mm Updated implementation to return audioEngine property instead of executing callback
Sources/CSFBAudioEngine/Player/AudioPlayer.h Removed mEngineQueue member variable; replaced WithEngine method with simple GetAudioEngine getter
Sources/CSFBAudioEngine/Player/AudioPlayer.mm Removed queue creation in constructor; eliminated all dispatch queue synchronization in engine operations, volume control, device management, and logging

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sbooth and others added 2 commits November 13, 2025 15:59
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sbooth sbooth marked this pull request as ready for review November 13, 2025 22:04
@sbooth sbooth merged commit 045b549 into main Nov 13, 2025
1 check passed
@sbooth sbooth deleted the remove-engine-queue branch November 13, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants