[Bug 46996] New: Improvements to the 7.1 and 5.1 to stereo conversion
https://bugs.winehq.org/show_bug.cgi?id=46996 Bug ID: 46996 Summary: Improvements to the 7.1 and 5.1 to stereo conversion Product: Wine Version: unspecified Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: trivial Priority: P2 Component: directx-dsound Assignee: wine-bugs(a)winehq.org Reporter: pavlica.nikola(a)gmail.com Distribution: --- Created attachment 64159 --> https://bugs.winehq.org/attachment.cgi?id=64159 a tar file with all of the patches The existing implementation of put_surround512stereo is a bit misleading because it takes into account LFE (Low-Freq Effects) that most likely can't be reproduced by stereo setups and should be just ignored. Source: https://www.audiokinetic.com/library/edge/?source=Help&id=standard_configura... Also the values for other channels emphasize more on the front L/R channels than on anything else. This is why some surround effects are barely audible. And instead of relying on a algorithm from pulseaudio to determine the values for the downmixer we can use a standard table for downmixing. Source: https://www.audiokinetic.com/library/edge/?source=Help&id=downmix_tables NOTE: Back speakers apparently are subwoofers in the 5.1 and 7.1 spec. So the documentation may be misleading as well. I also would like if support for converting 7.1 to stereo to be added to the dsound component, because games like Need for Speed: Rivals loose audio some capabilities, fxp. in game dialog is non-existant without this and turn it into a worse experience than on windows. Since I already made these changes and tried them out for basic testing, I'll attach my diffs. Also any tips on contributing would be helpful, Cheers -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Ken Thomases <ken(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aeikum(a)codeweavers.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 --- Comment #1 from Andrew Eikum <aeikum(a)codeweavers.com> --- Hi Nikola! In general, the patches look mostly OK. In one, you introduce tab characters where you should be using spaces. You should also split the dsound_convert.c changes into two separate patches. One revising the 5.1 downmix, and a different patch introducing the 7.1 downmix. When you are ready to submit, they should be sent as patch emails to wine-devel, see the information here https://wiki.winehq.org/Submitting_Patches We can continue revising the patches here until we are both happy with them. I'm unsure about the changes to the downmix algorithm. What we should do is whatever Windows's dsound does. If PulseAudio's algorithm doesn't match Windows, we should figure out what Windows does instead of choosing a different algorithm found online. I'm especially concerned about dropping LFE. If you have access to a Windows machine or VM, this should be easy to write a test (create a stereo primary buffer and a 7.1 secondary buffer and observe the output mix). Or, it's possible someone online has already analyzed the Windows dsound downmix algorithms. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 --- Comment #2 from Nikola Pavlica <pavlica.nikola(a)gmail.com> --- Created attachment 64211 --> https://bugs.winehq.org/attachment.cgi?id=64211 contains test files Hi Andrew, as you suggested I fired up a Windows 10 VM and created a 7.1 file that I later recorded it playback using WASAPI and read the screenshots of those values off pixel by pixel (not the most professional, I'd admit) I just want to ask if this seems like a detailed enough study before moving on. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 --- Comment #3 from Andrew Eikum <aeikum(a)codeweavers.com> --- Yes, that analysis looks good to me. Thanks for doing that. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Nikola Pavlica <pavlica.nikola(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64159|0 |1 is obsolete| | --- Comment #4 from Nikola Pavlica <pavlica.nikola(a)gmail.com> --- Created attachment 64227 --> https://bugs.winehq.org/attachment.cgi?id=64227 the two new patch files here are the new patches -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 --- Comment #5 from Andrew Eikum <aeikum(a)codeweavers.com> --- Thanks, those look fine. Just a couple of nit-picks: You have a couple of lines that end with whitespace or are entirely whitespace. Please remove the unnecessary whitespace (see the dots in these examples, there are more in your patches): + break; +···· + case 7: /* back right */ + value *= 1.0f;· You are also adding some tabs into a file that is indented with spaces: + case 3: + /* LFE is totally ignored in dsound when downmixing to 2 channels */ +»······»·······»·······»·······break; + } Once you've fixed those, feel free to send them to wine-devel for inclusion. Again, please review the Submitting Patches page I linked above. Specifically remember to include your sign-off on the patches. Thanks! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 --- Comment #6 from Andrew Eikum <aeikum(a)codeweavers.com> --- These were merged into Wine and will be in the upcoming 4.7 release. Anything else to do here, or shall we close the bug? commit fc84b8675a848683988d1793ea4bc9f95b7ea395 Author: Nikola Pavlica <pavlica.nikola(a)gmail.com> Date: Tue Apr 23 20:04:23 2019 +0200 dsound: Added 7.1 to stereo downmix. Signed-off-by: Nikola Pavlica <pavlica.nikola(a)gmail.com> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> commit 87eaa2f593d3a6b2e1440de3bf4c6b37ef089fcf Author: Nikola Pavlica <pavlica.nikola(a)gmail.com> Date: Tue Apr 23 20:04:22 2019 +0200 dsound: Revised 5.1 to stereo downmix. Signed-off-by: Nikola Pavlica <pavlica.nikola(a)gmail.com> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Nikola Pavlica <pavlica.nikola(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #7 from Nikola Pavlica <pavlica.nikola(a)gmail.com> --- Nothing left to do really, I mean we did fix the bug. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Andrew Eikum <aeikum(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |fc84b8675a848683988d1793ea4 | |bc9f95b7ea395 --- Comment #8 from Andrew Eikum <aeikum(a)codeweavers.com> --- Great, thanks for your contribution! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #9 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 4.7. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |4.0.x -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46996 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|4.0.x |--- --- Comment #10 from Michael Stefaniuc <mstefani(a)winehq.org> --- Removing the 4.0.x milestone from bug fixes included in 4.0.2. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org