feat(pipewire): set node.rate prop - #1341
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new NODE_RATE reference is unavailable in pipewire 0.10 and prevents compilation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds PipeWire stream rate hints to support configured clock rates.
Changes:
- Sets
node.ratefrom the stream sample rate. - Uses PipeWire’s constant for
node.group. - Updates the changelog.
File summaries
| File | Description |
|---|---|
src/host/pipewire/device.rs |
Configures PipeWire node properties. |
CHANGELOG.md |
Documents the PipeWire change. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
roderickvd
left a comment
roderickvd
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
| // preventing phase drift between simultaneous input/output streams. | ||
| properties.insert("node.group", format!("cpal-{}", std::process::id())); | ||
| properties.insert( | ||
| *pw::keys::NODE_GROUP, |
There was a problem hiding this comment.
Good one! I also prefer to re-use constants. Any chance there's more in src/host/pipewire/utils.rs where we could use some from *pw::keys instead?
There was a problem hiding this comment.
I removed node module from utils and replaced with pw::keys.
- METADATA_NAME is exposed in pipewire-sys but not in pipewire.
- There is pw::keys::DEVICE_ICON but that one exposes
icon-name, noticon_name - I couldn't find anything other than that.
There was a problem hiding this comment.
Perfect, thanks. One nit I added to the review, to use *pw:: instead of &pw::.
287879f to
bc8df40
Compare
| Option<SampleRate>, | ||
| ) = props | ||
| .get(node::LATENCY) | ||
| .get(&pw::keys::NODE_LATENCY) |
There was a problem hiding this comment.
Nitpick: I think *pw::keys::FOO is more idiomatic than &pw::keys::FOO.
40ab8d1 to
f29bc1d
Compare
|
Thanks, merging! |
I was debugging why
default.clock.allowed-ratesisn't working with cpal stream, and came up with this solution.