Skip to content

feat(realtime): rename feature, make default, and report RT denials#1187

Open
roderickvd wants to merge 19 commits intomasterfrom
feat/update-thread-priority
Open

feat(realtime): rename feature, make default, and report RT denials#1187
roderickvd wants to merge 19 commits intomasterfrom
feat/update-thread-priority

Conversation

@roderickvd
Copy link
Copy Markdown
Member

@roderickvd roderickvd commented May 2, 2026

  • Rename the audio_thread_priority feature to realtime-dbus and make it default.
  • Gate AAudio PERFORMANCE_MODE_LOW_LATENCY and PipeWire RT_PROCESS.
  • Rename ErrorKind::RealtimeUnavailable to RealtimeDenied.
  • Add RealtimeDenied error reporting to AAudio, PipeWire and JACK.
  • Fix AAudio error_callback not being forwarded to the caller.

- Rename the `audio_thread_priority` feature to `realtime` and make it default.
- Gate AAudio PERFORMANCE_MODE_LOW_LATENCY and PipeWire RT_PROCESS.
- Rename ErrorKind::RealtimeUnavailable to RealtimeDenied.
- Add RealtimeDenied error reporting to AAudio, PipeWire and JACK.
- Fix AAudio error_callback not being forwarded to the caller.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Copy Markdown

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

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

Comments suppressed due to low confidence (1)

src/host/wasapi/stream.rs:446

  • push_command unconditionally unwraps SetEvent(self.pending_scheduled_event). Since pending_scheduled_event is now closed on the worker thread in RunContext::drop, there is a race where send() can succeed while the worker is exiting, but SetEvent can fail (invalid handle) and panic the caller. Please handle SetEvent failure without panicking (and/or avoid closing the handle until it can no longer be signaled).
    fn push_command(&self, command: Command) -> Result<(), SendError<Command>> {
        self.commands.send(command)?;
        unsafe {
            Threading::SetEvent(self.pending_scheduled_event).unwrap();
        }
        Ok(())

Comment on lines +402 to +407
emit_error(
&error_callback,
Error::with_message(
ErrorKind::Other,
format!("PipeWire: failed to bind metadata object; device change notifications may be incomplete: {e}"),
),
Comment on lines +779 to +784
emit_error(
&error_callback,
Error::with_message(
ErrorKind::Other,
format!("PipeWire: could not acquire registry; device change notifications will be unavailable: {e}"),
),
Comment on lines +511 to +516
emit_error(
&error_callback,
Error::with_message(
ErrorKind::Other,
format!("PipeWire: could not acquire registry; device change notifications will be unavailable: {e}"),
),
knz added a commit to knz/cpal that referenced this pull request May 6, 2026
The previous fix queued `DeletePlaybackStream` from `Stream::drop` but
returned immediately. Because the delete is processed asynchronously
on the reactor and the play_all driver thread only exits *after* the
reactor wakes it, a process that exits shortly after dropping the
Stream can race the worker threads — the delete may never reach the
server, leaving exactly the leak this PR set out to fix.

Store the spawned threads' JoinHandles on `Stream` and join them in
`Drop` after signalling cancellation. Skip handles whose thread id
matches the current thread, since the user's error_callback runs on
those workers and may itself drop the Stream; joining ourselves would
deadlock. Mirrors the pattern roderickvd is introducing for PipeWire
in RustAudio#1187.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants