Change FFDShow DPL2 decoder to FreeSurround #5235

LAGonauta · 2017-04-09T19:53:33Z

Added the FreeSurround decoder to the mixer, and removed the legacy FFDShow decoder.
The decoding is done through the new SurroundDecoder class using a FixedSizeQueue as a ring buffer as the decoder needs a fixed amount of frames to be able to decode the surround information.

Tested on XAudio2 (with PR #6171), OpenAL, and Cubeb on Windows.
Also tested on OpenAL, Cubeb, and PulseAudio on Ubuntu, but Cubeb seems to be downmixing the surround information to stereo. Works on Windows, so maybe there is a bug in Cubeb library associated with Linux.

The upmix works really well, but I am not sure if we should set SURROUND_BLOCK_SIZE to 1024, 512, or even 256. All work, but the algorithm has more quality with 1024, less steering glitches and less crosstalk. For now I set it to 512 for an OK mix of quality and latency. The best quality is with SURROUND_FRAMES_PER_CALL set to 4096, but the latency seems to be equivalent to time-stretching with a buffer size of 80 ms.

Fixes bug #9047.

~~The new decoder seems to have some kind of incompatibility with some distros (Ubuntu 16.04, for example) as it causes glitches on decode. I will investigate the glitches later.~~
It seems that a fully updated Ubuntu 16.04 distro does not show this behaviour anymore.

Any suggestion is greatly appreciated :)

lioncash · 2017-04-09T20:09:20Z

All of this external library code should likely be placed into Externals, rather than AudioCommon, especially the FreeSurround stuff.

LAGonauta · 2017-04-09T22:24:21Z

Just moved everything to Externals.

Orphis · 2017-04-09T23:03:45Z

Where is the CMakeLists.txt for non-Visual Studio platforms?

CMakeLists.txt

  endif()
+  message(STATUS "Using static FreeSurround from Externals")
+  add_subdirectory(Externals/FreeSurround)
+  include_directories(Externals)


Externals/FreeSurround/CMakeLists.txt

+	FreeSurroundDecoder.cpp
+	)
+
+add_library(FreeSurround STATIC ${SRCS})


LAGonauta · 2017-04-11T19:11:40Z

I noticed that I had forgotten to update the References on the Debug build for Visual Studio. Now everything should compile fine there.

I am doing experiments on padding the values to not only be able to use SoundTouch but also to allow its use on other backends. I noticed that PulseAudio, for example, rarely returns the desired 512 samples, it oscillates over it (505-520).
Sadly still no go, you can see my "progress" here: https://github.com/LAGonauta/dolphin/tree/fs-buffer-tests

Even though some days ago SoundTouch had horrible latency when forcing the output of 512 samples (for me), I found out the latency is bearable when changing the default periods and period size on OpenAL Soft's config file to small numbers like 3 and 128, respectively. Maybe on the OpenAL backend we can just do that? Personally I prefer to use OpenAL's frequency-shifter.

LAGonauta · 2017-04-13T00:45:21Z

Just rebased this PR with the changes from merry's audio-stretching PR.
Now just need to find a way to make the buildbot not get stuck while trying to compile...

BhaaLseN · 2017-04-14T21:55:34Z

Nothing you can do on your end, buildbot has a size limit on patches it can receive, and it seems our monkey-patch for that got lost in the upgrade. delroth knows, but he's on vacation and probably won't work on it until he's back.

CMakeLists.txt

@@ -650,6 +650,10 @@ else()
 endif()

 find_package(OpenAL)
+if(OPENAL_FOUND)


Externals/FreeSurround/CMakeLists.txt

+
+add_library(FreeSurround STATIC ${SRCS})
+target_include_directories(FreeSurround PUBLIC include)
+add_definitions(-w)


Externals/FreeSurround/CMakeLists.txt

+	source/KissFFT.cpp
+	source/KissFFTR.cpp
+	source/FreeSurroundDecoder.cpp
+	)


LAGonauta · 2017-05-28T23:34:37Z

Just rebased because of Cubeb. I will also try to add support for FreeSurround on Cubeb, but right now it is not respecting how many frames I ask for.

ligfx · 2017-06-05T02:48:08Z

@LAGonauta I said a few things on IRC before I realized you were gone :)

ligfx 7:11pm: LAGonauta does it alway ask for exactly 480 frames? is there maybe one callback at the beginning that asks for the full amount you requested (512)?
ligfx 7:13pm: I'm thinking it might be correctly honoring the request for a larger buffer, but then the callbacks get scheduled at the same time as the audio engine (every 480 frames), which leaves 32 frames unread / only 480 writable frames

Externals/FreeSurround/FreeSurround.vcxproj

+  <PropertyGroup Label="UserMacros" />
+  <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
+    <Lib>
+      <AdditionalLibraryDirectories>%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories>


Source/Core/AudioCommon/AudioCommon.vcxproj

@@ -78,6 +88,9 @@
    <ProjectReference Include="$(CoreDir)Common\Common.vcxproj">
      <Project>{2e6c348c-c75c-4d94-8d1e-9c1fcbf3efe4}</Project>
    </ProjectReference>
+    <ProjectReference Include="..\..\..\Externals\FreeSurround\FreeSurround.vcxproj">


Source/Core/AudioCommon/AudioCommon.vcxproj

@@ -35,6 +35,16 @@
    <Import Project="..\..\VSProps\PCHUse.props" />
  </ImportGroup>
  <PropertyGroup Label="UserMacros" />
+  <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
+    <ClCompile>
+      <AdditionalIncludeDirectories>$(ExternalsDir)FreeSurround\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>


LAGonauta · 2017-06-05T10:27:35Z

@ligfx No, the first callback asks for 960 frames while all other callback asks for 480 (may depend on the CPU usage, I guess). No callback ever asks for 512 frames.

LAGonauta · 2018-03-14T22:19:56Z

Anything I can do to make this PR move forward for merge?

LAGonauta · 2018-04-14T13:51:42Z

After extensive testing I found that it is still needed to clamp some rare samples depending on the game, re-added that function for better user experience.

Also rebased the branch to the current master.

LAGonauta · 2018-05-29T10:37:07Z

Recently had the time try to find out what caused the glitches on Ubuntu 16.04 and they seem to have been fixed when using a fully updated distro. Yay! I guess?

LAGonauta · 2018-09-22T13:24:13Z

@lioncash @JMC47 @ligfx @shuffle2
Anything I can do to make this PR move forward?

shuffle2 · 2018-09-22T21:05:00Z

This PR seems fine to me, tbh my guess would be that people aren't merging it just because not many people have a way to test it...

weihuoya · 2018-10-18T15:29:23Z

it's work fine on android, thx.

DanLaRoche · 2019-01-30T00:40:26Z

It's a shame that the reviewers are ignoring this PR, some of us would love to get surround sound working correctly without having to rely on inconsistent pro logic support on modern HTIB systems and sound bars. In the meantime, LAGonauta, could you do a pull from master to keep your fork up to date? I'll be testing this soon on my HTPC that is connected to a surround sound system.

LAGonauta · 2019-01-30T09:56:43Z

@DanLaRoche Just rebased and pushed.
Remember to use Cubeb or OpenAL on Windows and PulseAudio on Linux. Other backends do not support surround sound yet.

stenzek · 2019-01-30T10:08:10Z

The code seems fine to me, aside from a few small nitpicks (e.g. replace the clamping by if/else with MathUtil::Clamp). But I'm not very familiar with the audio side of Dolphin, so I'm probably not the best person to review it.

But it's a shame for this to be sitting around, so unless anyone has any better ideas, I could merge it as-is and deal with any regressions later.

ghost · 2019-02-08T23:58:35Z

@LAGonauta Needs another rebase. I have a 5.1 setup my self and I'm sad at the gaming/PC HW audio industry, audio is extremely underappreciated in general, so my hat off for this PR.

Also cleaned up its source code to support only 5.1 and 7.1 setups.

Added class in AudioCommon for the surround decoder

Also removed minimum number of frames needed when decoding DPL2, and use std::numeric_limits to clamp the samples when needed. Clamping is still needed, but those samples are much rarer now and depend on the game.

LAGonauta · 2019-02-14T00:59:59Z

@ghost
Just rebased.

stenzek

I'm assuming this has been tested, and so far there's no objections to it being merged as-is. Had a quick look through the code again and can't see any major issues.

retrue · 2019-02-14T10:24:36Z

Hi
I have noticed problems with this change.
I am playing Resident Evil 4, with Surround activated. Before this pull request, sound was perfect. Now, it is choppy. If I disable Dolby Pro Logic II Decoder in Sound Settings, the sound is clean again.
My computer; i7 4770, 16 GB RAM, Nvidia 1060 6GB, SSD. Integrated sound chip Realtek High Definition Audio. OS, Windows 10 Pro 64bit.
My sound settings, in the image.

retrue · 2019-02-14T11:16:30Z

Hi, I have made more tests. With the above settings, using HLE and LLE Interpreter I get bad sound too.
If I disable Audio Stretching with Dolby Pro Logic enabled, sound is much better, but not so good as before this PR.

LAGonauta · 2019-02-16T09:48:05Z

Investigated a little bit and it seems this is happening and it seems that the non-uniform number of samples output from SoundTouch might be doing that... I need to investigate more.
However, what do you mean with not as good as before the PR without audio stretching? Is it stuttering for you? Or just different?
The older decoder did not really do any decoding, so with FreeSurround the sound placement will be really different.

retrue · 2019-02-16T16:06:55Z

Hi
What I meant is that it seems FreeSurround and audio stretching are not working well together and produces bad sound.
If I disable audio stretching the audio improves a lot but there is a little bit of stuttering. My opinion (I can be wrong) is that you are simply losing the fluidity in the sound you would get with audio stretching enabled.

LAGonauta changed the title ~~Changed FFDShow DPL2 decoder to FreeSurround.~~ Change FFDShow DPL2 decoder to FreeSurround Apr 9, 2017

LAGonauta force-pushed the fs-dplii-decoder branch from df1627d to b491309 Compare April 9, 2017 22:21

LAGonauta force-pushed the fs-dplii-decoder branch from b491309 to b45f57e Compare April 10, 2017 00:29

Orphis suggested changes Apr 10, 2017

View reviewed changes

LAGonauta force-pushed the fs-dplii-decoder branch 2 times, most recently from c834012 to 02d066e Compare April 11, 2017 19:03

LAGonauta force-pushed the fs-dplii-decoder branch 2 times, most recently from f37f0e1 to 1f0aaa4 Compare April 13, 2017 00:41

BhaaLseN reviewed Apr 17, 2017

View reviewed changes

CMakeLists.txt Outdated

@@ -650,6 +650,10 @@ else()

endif()

find_package(OpenAL)

if(OPENAL_FOUND)

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

LAGonauta force-pushed the fs-dplii-decoder branch 3 times, most recently from 418cf32 to e6b3785 Compare May 9, 2017 23:40

Orphis reviewed May 10, 2017

View reviewed changes

Externals/FreeSurround/CMakeLists.txt Outdated

add_library(FreeSurround STATIC ${SRCS})

target_include_directories(FreeSurround PUBLIC include)

add_definitions(-w)

This comment was marked as off-topic.

Sign in to view

Orphis reviewed May 10, 2017

View reviewed changes

Externals/FreeSurround/CMakeLists.txt Outdated

source/KissFFT.cpp

source/KissFFTR.cpp

source/FreeSurroundDecoder.cpp

)

This comment was marked as off-topic.

Sign in to view

LAGonauta force-pushed the fs-dplii-decoder branch from e6b3785 to c067237 Compare May 10, 2017 12:34

LAGonauta force-pushed the fs-dplii-decoder branch 2 times, most recently from 4af908b to 8742ff6 Compare May 28, 2017 23:20

LAGonauta force-pushed the fs-dplii-decoder branch from 8742ff6 to bf092da Compare June 4, 2017 16:25

shuffle2 reviewed Jun 5, 2017

View reviewed changes

LAGonauta force-pushed the fs-dplii-decoder branch from a2bec0e to fe8a560 Compare June 5, 2017 12:11

LAGonauta force-pushed the fs-dplii-decoder branch from 85bb9f4 to f52cece Compare March 27, 2018 12:36

LAGonauta force-pushed the fs-dplii-decoder branch from f52cece to 9db41cc Compare April 14, 2018 13:49

LAGonauta force-pushed the fs-dplii-decoder branch from 9db41cc to 3eceb86 Compare May 28, 2018 10:29

LAGonauta force-pushed the fs-dplii-decoder branch from 3eceb86 to 78dfd70 Compare July 2, 2018 11:36

LAGonauta force-pushed the fs-dplii-decoder branch from 78dfd70 to 24c83c9 Compare January 30, 2019 09:56

LAGonauta added 5 commits February 13, 2019 22:52

Added FreeSurround to Externals

7b93758

Also cleaned up its source code to support only 5.1 and 7.1 setups.

Swapped FFDShow's DPL2 decoder by FreeSurround

df631f0

Added class in AudioCommon for the surround decoder

Cleaned up PulseAudio as we now use FreeSurround

fdfd9d6

Removed zeroing-out the LFE as it is not needed anymore.

f4cf1e4

Also removed minimum number of frames needed when decoding DPL2, and use std::numeric_limits to clamp the samples when needed. Clamping is still needed, but those samples are much rarer now and depend on the game.

Removed FFDShow DPL2 decoder

c209d07

LAGonauta force-pushed the fs-dplii-decoder branch from 24c83c9 to c209d07 Compare February 14, 2019 00:53

stenzek approved these changes Feb 14, 2019

View reviewed changes

stenzek merged commit 326d727 into dolphin-emu:master Feb 14, 2019

LAGonauta deleted the fs-dplii-decoder branch February 16, 2019 09:44

Pokechu22 mentioned this pull request Feb 16, 2023

Externals/FreeSurround: Fix pointer created through new[] being freed via delete #11573

Merged

Change FFDShow DPL2 decoder to FreeSurround #5235

Change FFDShow DPL2 decoder to FreeSurround #5235

Conversation

LAGonauta commented Apr 9, 2017 • edited Loading

lioncash commented Apr 9, 2017

LAGonauta commented Apr 9, 2017

Orphis commented Apr 9, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

LAGonauta commented Apr 11, 2017

LAGonauta commented Apr 13, 2017

BhaaLseN commented Apr 14, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

LAGonauta commented May 28, 2017

ligfx commented Jun 5, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

LAGonauta commented Jun 5, 2017

LAGonauta commented Mar 14, 2018

LAGonauta commented Apr 14, 2018

LAGonauta commented May 29, 2018

LAGonauta commented Sep 22, 2018

shuffle2 commented Sep 22, 2018

weihuoya commented Oct 18, 2018

DanLaRoche commented Jan 30, 2019

LAGonauta commented Jan 30, 2019

stenzek commented Jan 30, 2019

ghost commented Feb 8, 2019 • edited by ghost Loading

LAGonauta commented Feb 14, 2019

stenzek left a comment

Choose a reason for hiding this comment

retrue commented Feb 14, 2019

retrue commented Feb 14, 2019 • edited Loading

LAGonauta commented Feb 16, 2019

retrue commented Feb 16, 2019

LAGonauta commented Apr 9, 2017 •

edited

Loading

ghost commented Feb 8, 2019 •

edited by ghost

Loading

retrue commented Feb 14, 2019 •

edited

Loading