-
Notifications
You must be signed in to change notification settings - Fork 3.2k
ao_pipewire: avoid setting any media.role by default #17035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
63802ac to
81cdc20
Compare
sfan5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please link https://gitlab.freedesktop.org/pipewire/wireplumber/-/issues/489 in the description too
|
I'm not sure if it's this exact issue, but I found the volume preserving behavior of pipewire to be obstructive/wrong when I switched to it years ago. stream.rules = [
{
matches = [
{ application.id = "~mpv" }
]
actions = {
update-props = {
state.restore-props = false
}
}
}
] |
|
That should avoid the issue but it would require every user to independently set that config. |
81cdc20 to
0a358b4
Compare
|
Why not simply set "Movie" as the media role unconditionally? Also, this is a config issue with wireplumber. It should be possible to tell it to not restore based on media.role. I, for one, like to listen to music at low volume as kind of background noise. But when I watch a video I want considerably more volume. So these roles fit the bill. Could this not be solved by an option to set the media role? That combined with conditional autoprofiles could make both camps happy, I think. |
Only the ones who don't like this. |
Because this doesn't avoid the issue that mpv's volume will be tied to any other app that uses the "Movie" media role. This is an anti-feature and there's a reason why mpv stopped setting the media role for PA backend in 2015. |
Fair enough.
It is for you and some others. How many issues are there complaining about this? That should be the watermark, I think. |
Across the linked issues, there's 12 unique users who've complained about this, and I didn't look to see if this issue was reported in some other ticket as well. There's also usually one person asking about this in the IRC channel on a monthly basis. It is much more likely that many users don't even report this and just go along their day using ao_pulse because it doesn't set the media.role and thus doesn't exhibit this obstructive behavior. |
|
Doesn't mean it has to go entirely. Also, there are other reasons whey pipewire ao is not popular with mpv, the sucky volume control, remember? You seem hellbent on crippling it even further. |
|
All I'm trying to say, if you find pulseaudio so annoying why do you make the transition to pipewire so unattractive? |
|
Please stop responding with off topic comments, they have nothing to do with this PR but I'll respond anyway.
I summarized my thoughts concisely in this comment here #15835 (comment)
I'm fixing bugs that at least a dozen people complained about if not more, and even upstream isn't sure if said feature should exist or not. |
The hint about the ao-volume control was meant to tell you that there may be other reasons why people don't come complaining about ao-pipewire, like almost nobody using it for reasons like e.g. sucky ao-volume handling. So it was topic adjacent.
And you only demonstrated that you missed the real issue. My mistake was to mention other semi-related issues which I came by when researching pre-PR... But, I'll say no more on that particular matter.
No, if anything, you are trying to fix pipewire's bugs, or work around them, rather. And a bug this ain't; as stated, not only by me, this is intended behavior and there is no error anywhere in the chain. It boils down to preference. And how to change that in a totally non-destructive way has been shown as well. I implore you to reconsider. Babies, bathwater and all that. Sometimes it is just a PEBCAK situation and users just need to RTFM. Counter offer: How about I write something up in the documentation section for ao-pw and wherever else you please? First, explain the media.role thing and how to change pw prefs. Basically, the man page asks users all the time to change some file; it just happens to be its own config. So why not just tell people "not our bug" and redirect them to the correct source? |
|
Maybe users can do for pipewire like I do for pulseaudio. |
|
@DanOscarsson thanks for chiming in. So, you're basically working around mpv dropping that functionality. Case in point. ;) I think one just needs to compare the amount of work involved in either changing wireplumber stream restoration precedence now, which boils down to (re)moving one single line, or in the future creating an autoprofile to hack mpv's advertised client name based on presence of video track (how do I iterate over the tracks table in one Lua expression?) and hack some wireplumber script to then assign the correct media role. |
|
I'd like to chime in as well at this point. I personally use exactly the media.role property to link applications to specific outputs without having to specify each application explicitly. This way, I can change the respective sink's name easily, without having to change several configs. But it relies on applications to set meaningful media.role properties. Hence, I find a command line option to set the media role explicitly to be the most flexible and least intrusive change. I have been using it this way for a while now: franzitrone@776966e Although the commit above is probably rather hacky and only served as a personal patch/hack. |
0a358b4 to
e13ade3
Compare
|
I've made it an option that's disabled by default, and also made it set it for pulse depending on the option |
e13ade3 to
6e93a4a
Compare
Isn't this not our problem? And should be directed as a bugs to upstream projects? I feel like we are trying to workaround issue that we shouldn't have. |
it's far too late for that, because "we" (more specifically wm4 decided to) already worked around this in 2015: b7325b2 also here's the upstream issue for pipewire: https://gitlab.freedesktop.org/pipewire/wireplumber/-/issues/489 |
Ok. In this case "we" can have this change. |
sfan5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherweise
This adds option `--audio-set-media-role` to configure whether mpv should set media roles to supported audio APIs or not. Disabled by default. By default, WirePlumber restores the volume of each application to whatever the last application with the same media role used. This is completely insane behavior, and I couldn’t find any way for a user to opt out of it on my end. The `state-stream.lua` script responsible for this mess only uses media.role if the property is present, so the only reliable workaround is simply not to set one at all. This WirePlumber behavior is inherited from PulseAudio, which also exhibits the same behavior to this day. This commit also mirrors the same change made to ao_pulse in 2015: b7325b2 See also: https://gitlab.freedesktop.org/pipewire/wireplumber/-/issues/489
6e93a4a to
affcef3
Compare
By default, WirePlumber restores the volume of each application to
whatever the last application with the same media role used. This is
completely insane behavior, and I couldn’t find any way for a user to
opt out of it on my end. The
state-stream.luascript responsible forthis mess only uses media.role if the property is present, so the only
reliable workaround is simply not to set one at all.
This WirePlumber behavior is inherited from PulseAudio, which also
exhibits the same behavior to this day.
This commit also mirrors the same change made to ao_pulse in 2015:
b7325b2
Fixes #16459 #16370 and similar
Fixes #16978