Skip to content

Ensure stale audio is discarded after stopping - #976

Draft
sbooth wants to merge 5 commits into
mainfrom
stale
Draft

Ensure stale audio is discarded after stopping#976
sbooth wants to merge 5 commits into
mainfrom
stale

Conversation

@sbooth

@sbooth sbooth commented Aug 6, 2026

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI lite review requested due to automatic review settings August 6, 2026 13:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 aims to prevent previously-decoded (“stale”) audio remaining in the ring buffer from being rendered after the player is stopped/reset, by marking audio as stale and letting the render path discard it.

Changes:

  • Set Flags::audioStale during AudioPlayer::stop() so the next render cycle flushes the ring buffer.
  • Set Flags::audioStale during AudioPlayer::reset() for the same render-time flush behavior.
Suppressed comments (1)

Sources/CSFBAudioEngine/Player/AudioPlayer.mm:842

  • Same concern as stop(): reset() sets Flags::audioStale, but the flag is only cleared during render(). If the engine remains stopped after reset, this can block decoding (and potentially cause the decoding thread to poll) until playback is restarted and a render callback runs. Consider ensuring the decoding side doesn’t spin while waiting for audioStale to clear when the engine isn’t running.
    // Discard any leftover audio during the next render cycle
    setFlags(Flags::audioStale);

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

Comment thread Sources/CSFBAudioEngine/Player/AudioPlayer.mm Outdated
@sbooth
sbooth requested a lite review from Copilot August 6, 2026 15:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Sources/CSFBAudioEngine/Player/AudioPlayer.mm:1638

  • duration is computed in seconds (frames / sampleRate), but the timeout multiplies by nanosecondsPerMillisecond, producing a value 1000× too small (e.g., 0.25s becomes 0.25ms). This can cause the decoding thread to wake far more frequently than intended and waste CPU. Multiply by nanosecondsPerSecond (or convert duration to milliseconds before using nanosecondsPerMillisecond).
    const auto duration = static_cast<double>(targetMaxFreeSpace - freeSpace) / audioBuffer_.format().mSampleRate;
    return static_cast<int64_t>(duration * static_cast<double>(nanosecondsPerMillisecond));

@sbooth
sbooth marked this pull request as draft August 6, 2026 18:17
@sbooth
sbooth requested a balanced review from Copilot August 8, 2026 13:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

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