[Bug 46450] New: Volume Control doesn't work in Firefox videos ( and browsers based on it) with PulseAudio driver
https://bugs.winehq.org/show_bug.cgi?id=46450
Bug ID: 46450 Summary: Volume Control doesn't work in Firefox videos (and browsers based on it) with PulseAudio driver Product: Wine Version: 4.0-rc5 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: winepulse.drv Assignee: wine-bugs@winehq.org Reporter: gabrielopcode@gmail.com CC: aeikum@codeweavers.com Distribution: ---
The volume control on videos does not work and has no effect. Tested on Firefox 52 ESR, Waterfox and latest Pale Moon (you have to enable media.wmf.vp9.enabled to true for the latter so you can play videos on Youtube without native mfplat / vdec DLLs).
I skimmed through Firefox's source code, it appears to be using IAudioStreamVolume::SetAllVolumes or similar calls. When I poked through Wine's code, it seems only coreaudio driver actually does anything here.
The pulseaudio driver's SetAllVolumes is basically a no-op because it doesn't do anything with the volume it sets (except retrieve it with GetAllVolumes), and it's never sent to libpulse so of course it doesn't have any effect. The coreaudio driver does call ca_setvol or ca_session_setvol where the pulse driver is missing such a thing completely.
With a quick search I found that pa_context_set_sink_input_volume might be useful for this purpose, but I don't really know much about libpulse, perhaps there's a better way. It definitely can't stay like this though, since right now it's basically a stub without even a FIXME for it...
https://bugs.winehq.org/show_bug.cgi?id=46450
--- Comment #1 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Created attachment 64145 --> https://bugs.winehq.org/attachment.cgi?id=64145 Set the PulseAudio stream volume when it changes
Here's what I had in mind, seems to work fine, but I don't know if it's the way we want to do this. In particular this will actually change the stream volume in PulseAudio (so it shows up in the Volume Control and so on).
https://bugs.winehq.org/show_bug.cgi?id=46450
--- Comment #2 from Andrew Eikum aeikum@codeweavers.com --- Yeah, I don't want to do it that way. If the user sets a volume in their mixer, I don't want the application to be able to override it.
https://bugs.winehq.org/show_bug.cgi?id=46450
--- Comment #3 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Created attachment 64361 --> https://bugs.winehq.org/attachment.cgi?id=64361 Adjust the buffer volume before sending it to PulseAudio.
How about something like this? In this case, we process the buffer and supported sample formats ourselves and adjust the volume.
Tested it on 3 different apps that relied on it and it seems to work fine, even the 24-bit encoding which is more complicated.
If this is fine, I will add ALAW/ULAW handling as well (from PA sources so it's the same encoding) which should cover all the supported formats.
https://bugs.winehq.org/show_bug.cgi?id=46450
--- Comment #4 from Andrew Eikum aeikum@codeweavers.com --- Yeah that looks really good. I'd change the enum members to be ALL CAPS to distinguish them from local variables, and maybe change them to something like WINEPULSE_WRITE_NOFREE to make it clear they're specific to winepulse.
I understand the 24-bit stuff is awkward on big-endian, but shouldn't the PROCESS_BUFFERS macro work fine on big-endian?
Can you also mention here what apps you tested it with?
https://bugs.winehq.org/show_bug.cgi?id=46450
--- Comment #5 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Sure I'll do the renames. The applications I tested with were Firefox & Palemoon, Winamp with WaveOut (which had the same bug), and I also tested REAPER but it appears it handles volume changes on its own, so instead I tried all formats from REAPER but changing to an artificial 0.5 mastervol in the source code to halve the volume, and it worked fine.
PROCESS_BUFFERS is endian-specific since it uses the target's endianess to read one sample at a time for efficiency, which is important for low latency buffers (compiler will also auto-vectorize it this way, so it's much more efficient). Since the Windows API uses little-endian, it needs to match.
I can add a big-endian case, but I didn't find it worth it, since as far as I know, Windows doesn't run on Big Endian and I doubt this will run properly when compiled like that. But just in case I'll be asking: is there some macro/function I can use to bswap or do I have to read 1 byte at a time manually in such case? (the formats will still be little-endian since that's what the Windows API uses)
https://bugs.winehq.org/show_bug.cgi?id=46450
Gabriel Ivăncescu gabrielopcode@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |cad249b6881240e287b8dddda54 | |24b7e1036197f Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED
--- Comment #6 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Fixed by cad249b6881240e287b8dddda5424b7e1036197f.
https://bugs.winehq.org/show_bug.cgi?id=46450
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #7 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 4.10.
https://bugs.winehq.org/show_bug.cgi?id=46450
Michael Stefaniuc mstefani@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |4.0.x
https://bugs.winehq.org/show_bug.cgi?id=46450
Michael Stefaniuc mstefani@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|4.0.x |---
--- Comment #8 from Michael Stefaniuc mstefani@winehq.org --- Removing the 4.0.x milestone from bug fixes included in 4.0.3.
-
wine-bugs＠winehq.org -
WineHQ Bugzilla