Improve accuracy of frames rendered bookkeeping#960
Open
sbooth wants to merge 13 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the accuracy of “frames rendered” bookkeeping by pairing decoded audio chunks with explicit per-chunk metadata (sequence number, frame position/length, playback generation) and using that metadata to emit more precise render events.
Changes:
- Add a dedicated SPSC metadata ring buffer (
audioMetadata_) and new chunk descriptor structs to track which decoder produced which rendered frames. - Update seek handling to return an optional frame position and incorporate a playback generation counter to discard stale render events after seeks/cancellations.
- Rename/clarify buffer constants and add the
CXXRingBufferdependency to support metadata buffering.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Sources/CSFBAudioEngine/Player/AudioPlayer.mm | Implements dual ring-buffer (audio + metadata) flow, generation-based stale-event rejection, and updated seek/render bookkeeping. |
| Sources/CSFBAudioEngine/Player/AudioPlayer.h | Adds decoded/rendering chunk descriptor types and new members (audioMetadata_, playbackGeneration_, renderingChunk_). |
| Package.swift | Adds CXXRingBuffer as a SwiftPM dependency for the metadata ring buffer. |
| Package.resolved | Pins CXXRingBuffer to v0.6.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1531
to
+1536
| // Write the decoded chunk descriptor to the metadata buffer | ||
| if (!audioMetadata_.write(descriptor)) { | ||
| os_log_fault( | ||
| log_, | ||
| "Error writing audio chunk descriptor to ring buffer: spsc::RingBuffer::write failed"); | ||
| } |
sbooth
marked this pull request as ready for review
July 19, 2026 19:51
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
Sources/CSFBAudioEngine/Player/AudioPlayer.mm:1535
- If
audioMetadata_.push(descriptor)fails, the code currently logs but continues writing audio. That can permanently desynchronize audio frames from their metadata (sequence number/generation), breaking frames-rendered bookkeeping. If the metadata push fails, skip writing audio for that chunk and force a drain/new generation so render state stays consistent.
// Write the decoded chunk descriptor to the metadata buffer
if (!audioMetadata_.push(descriptor)) {
os_log_fault(log_, "Error writing chunk descriptor: spsc::Queue::push failed");
}
Comment on lines
+1536
to
1541
| // Write the decoded audio to the audio buffer for rendering | ||
| const auto framesWritten = audioBuffer_.write(buffer.audioBufferList, buffer.frameLength); | ||
| if (framesWritten != buffer.frameLength) { | ||
| os_log_fault( | ||
| log_, | ||
| "Error writing audio to ring buffer: spsc::AudioRingBuffer::write failed for %u frames", | ||
| buffer.frameLength); | ||
| os_log_fault(log_, "Error writing audio: spsc::AudioRingBuffer::write failed for %u frames", | ||
| buffer.frameLength); | ||
| } |
Comment on lines
27
to
32
| .package(url: "https://github.com/sbooth/AVFAudioExtensions", .upToNextMinor(from: "0.5.1")), | ||
| .package(url: "https://github.com/sbooth/CXXAudioRingBuffer", .upToNextMinor(from: "0.1.1")), | ||
| .package(url: "https://github.com/sbooth/CXXDispatchSemaphore", .upToNextMinor(from: "0.4.1")), | ||
| .package(url: "https://github.com/sbooth/CXXMessageQueue", .upToNextMinor(from: "0.2.0")), | ||
| .package(url: "https://github.com/sbooth/CXXQueue", branch: "main"), | ||
| .package(url: "https://github.com/sbooth/CXXUnfairLock", .upToNextMinor(from: "0.3.1")), |
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
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.
No description provided.