-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[RFC] Add a PipeWire audio backend #7902
Conversation
ao->samplerate = 48000; | ||
params[0] = spa_format_audio_raw_build(&b, SPA_PARAM_EnumFormat, | ||
&SPA_AUDIO_INFO_RAW_INIT( | ||
.format = SPA_AUDIO_FORMAT_F32P, |
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.
Isn’t that format a bit wasteful, when some very common format would be signed 16-bit integer per sample?
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.
This, along with the sample rate should probably be part of the format negotiation with PipeWire. Unfortunately, the PipeWire documentation is not as complete as I would like it to be and I shied away from taking a stab at it. I might still try to figure it out.
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.
This hardcoding shouldn't be necessary. pw_stream
internally loads the audioconvert spa plugin that can convert, resample, mix, etc to bring the audio to the format that the rest of the graph expects.
|
||
ao->format = AF_FORMAT_FLOATP; | ||
ao->sstride = ao->channels.num * sizeof(float); | ||
ao->samplerate = 48000; |
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.
Is it possible to change that on the fly depending on the sample rate of the audio file being played?
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.
See above.
Possibly because it's stereo? I think the number of samples logic in mpv assumes a per-channel number, though I might be misremembering. |
In practice, these arguments are not used. You can safely call The argument parsing in |
Please justify pipewire. |
If we could actually replace the pulse backend with pipewire, and do it fairly quickly, I think that would be a fair justification. The problem is that history tells us this doesn't happen. pulse replaced alsa replaced oss and see where we are... |
From what I can see, pipewire is an even more awful thing than pulseaudio. Maybe we should drop pulseaudio support, and rely on the ALSA wrapper? |
@wm4 PW should give audio pipeline node-routing/graph-building handling of JACK with fancy-shmancy DAC/ADC management of PA and some gstreamer processing (which also integrates with ffmpeg via gstreamer-plugins-libav) that is better done with routing and hardware info that audio management system would have (like knowing total system latency, real volume and layout). Distributions will always use something over naked ALSA, generally PA, JACK - for something that requires a modicum of quality, because PA sucks at setting correct formats, at latency handling and is almost unable to do node routing while JACK sucks at autoconfiguration and is unable to deal with multiple hardware interfaces. PW should replace both PA and JACK permanently if it doesn't pull PA/NM thing first. And naked ALSA will never be default for any serious distro ever again. So, you could just not bother and rely on ALSA emulation or whatever. But the technically correct thing would be to embrace technically superior solution. Personally, I'm going to use mpv with PW either way unless my post-processing pipeline breaks and not going to be fixed for some unfathomable reason, then it's JACK with PA-as-JACK-shim and PA backend of mpv (because it's less prone to xruns than native JACK backend now for some reason) sandwich time. |
Still default here and it just works 99% of the time. |
This is pretty much the reception I expected, so I will just close this pull request. I will probably just toy around with the code in my own repository. Or not, whatever. |
I just want to make it clear that I was not necessarily against pipewire, just that I think supporting plain alsa should remain an option. |
For what it's worth, I welcome any replacement for 'pulse' that introduces the concept of filter graphs. Because JACK is kinda shitty for normal 'desktop' usage, due to its design being tied to 'pro audio'. A best-of-all-worlds audio server would be warmly welcomed by me. (But I'm skeptical as to whether or not I trust the pipewire developers to deliver on this promise. I'll let time tell) That said, criticism about pipewire itself is irrelevant in a PR about |
based on this original pull request mpv-player#7902 by andreaskem
@andreaskem I'm interested in getting this back on track again. I rebased it onto current master and it seems to work. Are you still interested in feedback or do you want me to take over the commit? Edit: I'm personally just using |
I also see that @Oschowa is working on their own fork of this commit as well. |
My fork of this adds format negotiation and fixes channel layouts, and it seems to be fully functional and preferable to this original version here. However, as i've stated in the feature request before, it doesn't really provide any tangible benefit over mpv' current ALSA backend on pipewire, so I haven't considered to PR it. You could in theory add device enumeration and volume control to the pipewire backend, but that would need at least ~3x the lines of code and complexity as it currently has and I don't really see a benefit - you can just use pavucontrol/pactl/pw-cli/whatever to switch devices and change volume. |
Having the |
Sorry, I did not really have time to work on this much beyond a rudimentary implementation. It is good to see a much more advanced version. |
Bump it would be really nice to see a pipewire native audio backend merged. I've been hitting some difficult to reproduce desynchronisation issues which I suspect are due to using the jack ao with pipewire, and am hoping a native backend would get rid of those. |
this issue has been re-started as issue #8569 |
@Oschowa Do you still have an interest in submitting a PR, including @t-8ch's changes? I really don't think there is any opposition to new AO backends, and allegedly there are even users of your PR in the wild. See #9462 and even https://aur.archlinux.org/packages/mpv-pipewire/ |
Without the pipewire patch, MPV is kind of unusable. |
It's nice to have native backend for modern desktop but, come on, the whole point of PW is in transparent support for everything: So, it's plenty usable, just not optimal. |
based on this original pull request mpv-player#7902 by andreaskem
Sometimes the most obvious things can be missed. Reflects authorship described in the original commit. * #7902 * Oschowa@fddb143 * #9587
I realize that yet another audio backend might be poorly received but I implemented this for fun and decided to make it public in case somebody might find it useful. I marked it
[RFC]
accordingly.This is an extremely basic implementation that hardcodes some stuff but seems to work in basic tests (mono, stereo, audio+video) I performed on my setup (i.e., with simple stereo headphones). I am not all that familiar with either PipeWire or mpv internals so this might crash and burn with slightly more elaborate setups or requirements. It is only 165 lines, after all, so don't expect any miracles.
A few things I am not sure about:
argc/argv
arguments and can pull some configuration from the application command line. I basically stubbed this. No idea how that could/should be wired up with mpv's command line handling.nframes = maxbuf / ao->sstride / 2;
I have no idea why the/ 2
is required here butmpv
keeps throwing buffer underrun errors without it. PipeWire seems to provide 16 kiB buffers but mpv only seems to be able to fill 8 kiB buffers.end_time
calculation but it does not seem to be all that broken.