ao_pipewire: avoid setting any media.role by default #17035
Conversation
llyyr
force-pushed
the
pipewire-shitmess
branch
2 times, most recently
from
63802ac to
81cdc20
Compare
sfan5
left a comment
sfan5
left a comment
There was a problem hiding this comment.
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. |
llyyr
force-pushed
the
pipewire-shitmess
branch
from
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. |
llyyr
force-pushed
the
pipewire-shitmess
branch
from
0a358b4 to
e13ade3
Compare
llyyr
changed the title
|
I've made it an option that's disabled by default, and also made it set it for pulse depending on the option |
| char *client_name; | ||
|
|
||
| // Whether mpv should set the media role to the audio server | ||
| bool set_media_role; |
There was a problem hiding this comment.
Do we want to set it for ao_audiotrack? I think it makes sense to set content type on android where it might want to duck music or video if there's an ongoing call for example
There was a problem hiding this comment.
the setting should be consistent, I would simply change the default in mpv-android.
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
llyyr
force-pushed
the
pipewire-shitmess
branch
from
e13ade3 to
6e93a4a
Compare
| {"audio-client-name", OPT_STRING(audio_client_name), .flags = UPDATE_AUDIO}, | ||
| {"audio-buffer", OPT_DOUBLE(audio_buffer), | ||
| .flags = UPDATE_AUDIO, M_RANGE(0, 10)}, | ||
| {"audio-set-media-role", OPT_BOOL(audio_set_media_role), |
There was a problem hiding this comment.
| {"audio-set-media-role", OPT_BOOL(audio_set_media_role), | |
| {"audio-media-role", OPT_CHOICE(audio_media_role), | |
| ... |
I think this is the more common style for an option. To give choice of setting it up.
There was a problem hiding this comment.
I'd rather not do this here because PA, Pipewire and AudioTrack have different keys for video/music and it'd be a pain for mpv to provide an option that unifies all. This just controls whether mpv sets the best matching role possible.
But if you think this is a blocker for this PR, I'll do my best to figure out a way to unify PA/Pipewire/Audiotrack interface here
There was a problem hiding this comment.
PA, Pipewire and AudioTrack have different keys for video/music and it'd be a pain for mpv to provide an option that unifies all.
I don't understand. Each of them is separate AO and those keys are already defined.
But if you think this is a blocker for this PR, I'll do my best to figure out a way to unify PA/Pipewire/Audiotrack interface here
It's fine. Though I don't see how music/video selection is hard to unify.
Anyway, I don't mind, do as you prefer. I still think "we" workaround bugs in upstream in a way that will change the behavior of mpv, which likely will invite another group of users to report bugs who depends on this roles to be set.
There was a problem hiding this comment.
I don't understand. Each of them is separate AO and those keys are already defined.
I don't really see any value in letting the user make mpv lie about the content it's outputting if it's just video/music so I assumed you meant exposing all the possible keys that make sense for mpv to set (Speech, Game, Notification etc.). Do you just want the options to be <auto|none> instead of <yes|no>?
I still think "we" workaround bugs in upstream in a way that will change the behavior of mpv
This is an optional field that isn't required or even encouraged to be set by PA or PW. mpv setting this field is the change in behavior, not the other way around.
Times when mpv set this field: few months in 2015, 2022-now (PW only).
Times when mpv didn't set this field: Every other time not covered by above range, since having a pulse AO.
You tell me which one is the change in behavior.
There was a problem hiding this comment.
mpv setting this field is the change in behavior, not the other way around.
How so? mpv sets those files for years and multiple stable releases. How is changing it now not a change in behaviour?
Also is Android affected by the same bugs as pipewire?
There was a problem hiding this comment.
How so? mpv sets those files for years and multiple stable releases. How is changing it now not a change in behaviour?
It's not a change in behavior, it's a regression fix. The change in behavior was pipewire setting this field in the first place.
This is evidenced by the fact that in the last 10 years, absolutely nobody has ever requested a feature that mpv set the media role in the pulse AO. At least a dozen unique users, and a few others in IRC have reported bugs that were a direct result of mpv setting this field in the pipewire AO. I'm sure you understand that if a dozen users are reporting an issue directly upstream, then the affected population is probably higher than that.
I'd wager the actual number of users who find this behavior to be annoying is much higher, but those bug reports never actually reach mpv. I've seen Arch forum posts that go something like
OP: mpv randomly no audio??
Ans: switch to -ao=pulse
Are you just bored and looking to waste everyone's time including your own?
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. |
6 participants
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