[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: [e-mail]
          Reporter: [e-mail]
      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_confi...

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

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Ken Thomases  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

--- Comment #1 from Andrew Eikum  ---
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.

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Alistair Leslie-Hughes  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

--- Comment #2 from Nikola Pavlica  ---
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.

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

--- Comment #3 from Andrew Eikum  ---
Yes, that analysis looks good to me. Thanks for doing that.

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Nikola Pavlica  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64159|0                           |1
        is obsolete|                            |

--- Comment #4 from Nikola Pavlica  ---
Created attachment 64227
  --> https://bugs.winehq.org/attachment.cgi?id=64227
the two new patch files

here are the new patches

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

--- Comment #5 from Andrew Eikum  ---
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!

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

--- Comment #6 from Andrew Eikum  ---
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 
Date:   Tue Apr 23 20:04:23 2019 +0200

dsound: Added 7.1 to stereo downmix.

Signed-off-by: Nikola Pavlica 
    Signed-off-by: Andrew Eikum 
    Signed-off-by: Alexandre Julliard

commit 87eaa2f593d3a6b2e1440de3bf4c6b37ef089fcf
Author: Nikola Pavlica 
Date:   Tue Apr 23 20:04:22 2019 +0200

dsound: Revised 5.1 to stereo downmix.

Signed-off-by: Nikola Pavlica 
    Signed-off-by: Andrew Eikum 
    Signed-off-by: Alexandre Julliard

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Nikola Pavlica  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Nikola Pavlica  ---
Nothing left to do really, I mean we did fix the bug.

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Andrew Eikum  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |fc84b8675a848683988d1793ea4
                   |                            |bc9f95b7ea395

--- Comment #8 from Andrew Eikum  ---
Great, thanks for your contribution!

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #9 from Alexandre Julliard  ---
Closing bugs fixed in 4.7.

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Michael Stefaniuc  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.0.x

[Bug 46996] Improvements to the 7.1 and 5.1 to stereo conversion

https://bugs.winehq.org/show_bug.cgi?id=46996

Michael Stefaniuc  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.x                       |---

--- Comment #10 from Michael Stefaniuc  ---
Removing the 4.0.x milestone from bug fixes included in 4.0.2.