Skip to content

Don't merge: process stream commands also when stream is paused (hotfix stream ejection)#5

Open
strohel wants to merge 2 commits intomainfrom
stopped-callback-ejection-hotfix
Open

Don't merge: process stream commands also when stream is paused (hotfix stream ejection)#5
strohel wants to merge 2 commits intomainfrom
stopped-callback-ejection-hotfix

Conversation

@strohel
Copy link
Copy Markdown
Member

@strohel strohel commented Oct 22, 2025

Not intended for merging, but should be good for our use in tonari (at least short-term until a upstreamable solution is devised).

Copy link
Copy Markdown
Collaborator

@mbernat mbernat left a comment

Choose a reason for hiding this comment

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

LGTM

@strohel strohel changed the title Hack: process stream commands also when stream is paused (hotfix stream ejection) Don't merge: process stream commands also when stream is paused (hotfix stream ejection) Oct 22, 2025
@strohel
Copy link
Copy Markdown
Member Author

strohel commented Oct 28, 2025

Submitted a bug description upstream in SolarLiner#105

@strohel
Copy link
Copy Markdown
Member Author

strohel commented Oct 28, 2025

FWIW the pipewire-rs API unsoundness was already reported, I replied in https://gitlab.freedesktop.org/pipewire/pipewire-rs/-/issues/103#note_3167277

SolarLiner added a commit to SolarLiner/interflow that referenced this pull request Feb 9, 2026
## Description

Fixes #105, at least for me.

To be reviewed by commits:
- first commit adds a new example `eject_stream_pipewire.rs`, which
hangs at this point
- this would be even better as an integration test, but it requires
running pipewire, output device, the `pw-link` command
- the second commit is a small cleanup/refactor to make the following
change easier
- the third commit is the actual bugfix
- instead of handling `StreamCommand`s in the `process()` callback
(which runs in the RT thread), we now handle them in the PipeWire main
loop. This also prompted a transition from `rtrb` to
`pipewire::channel`; this makes their handling independent
- a `Mutex` is introduced in the audio callback **but we never wait on
it** in the audio process() callback. I tried hard to avoid it, but I
believe it is necessary (short of unsafe code), as we need `&mut` access
to the `Option<Callback>` in two separate threads.

## Type of Change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] Code cleanup or refactor

## How Has This Been Tested?

- [x] Tested by introducing the `eject_stream_pipewire.rs` example.
- [ ] Test in more realistic setting that is works at least as good as
tonarino#5 @mbernat maybe? 🙏

## Checklist:

- [x] My code follows the style guidelines of this project
- ~I have made corresponding changes to the documentation~
- [x] My changes generate no new warnings
- [x] Wherever possible, I have added tests that prove my fix is
effective or that my feature works. For changes that need to be
validated manually (i.e. a new audio driver), use examples that can be
run to easily validate them.
- [x] New and existing unit tests pass locally with my changes
- [x] I have checked my code and corrected any misspellings

CC @mbernat. Supersedes tonarino#5.
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