cai567890 / pcsx2

Automatically exported from code.google.com/p/pcsx2
1 stars 0 forks source link

SDL Audio Support for SPU2X #1413

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
There are two patches here, one for c++03 and one for c++11 support. The latter 
is for whatever pcsx2 moves to c++11. They both include:

Updates to cmake/SelectPcsx2Plugins giving spu2-x an SDL dependency.

Updates to plugins/spu2-x/CMakeLists to compile in SDL support.

Updates to SndOut.h and .cpp to include SDL in the mods[].

Added SDL to the sink picker in Linux/Config, but not Windows/Config, because I 
can't test if the SDL sink works on Windows (no dual boot here).

The c++11 one adds SndOut_SDL in its own subdirectory, and checks if the 
compiler is GCC and a version that supports -std=c++11 before compiling it in, 
and it also has a define check in mods[] to include SDL or not depending on if 
it is compiling or not.

I didn't make it Linux-only, since I imagine Windows SDL support is just adding 
the menu entry on that platforms config, I just can't test it.

There also some notes for whenever the whole project moves to SDL2 - I tried 
experimenting with having both libraries as separate dependencies so I could 
use 32 bit audio and surround sound here, but the symbol tables conflict no 
matter how I tried it, so the notes are there for the transition.

Both patches were working fine yesterday in 5681, but in 5682 some unrelated 
breakages in gsdx tfx.glsl causes continuous errors that slow the emulator and 
audio engine significantly.

If anyone finds bugs on any platform, I'll try to fix them, but I only have my 
local Arch install and Suse, Fedora, and Ubuntu VMs available for testing. Both 
patches compiled and ran fine in 5681, and the audio isn't broken, it just 
shifts frequency and distorts the audio down a few pitches due to the engine 
slowdowns of the gl errors (on my end, at least).

Original issue reported on code.google.com by matt.sch...@gmail.com on 27 Jun 2013 at 9:43

Attachments:

GoogleCodeExporter commented 9 years ago
.. but why? 
We worked on refining the portaudio module for broad plattform support and last 
I tested, it worked just fine.
Everyone is trying to get away from SDL so why this patch?

Original comment by ramapcsx2.code on 27 Jun 2013 at 11:16

GoogleCodeExporter commented 9 years ago
I tried getting ports to play nicely on multiple machines running pulse, but I 
ran into issues on all of them, either prohibitive latency or distortion. 
Rather than try to implement pulse in ports (or spu2x), I just wrote a quick 
sdl plugin (really, its like 100 lines total) that worked for me, since I saw 
pcsx2 already had a dependency on it.

If you are moving away from sdl, no problem, I just wrote this to get working 
sound on my end.

Original comment by matt.sch...@gmail.com on 28 Jun 2013 at 1:54

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I see. 
Well, I don't know the current status of sound from SPU2-X on Linux but i was 
hoping it worked well..
If it doesn't we should try to fix it first. If that proves to be difficult we 
can use your SDL patch :p

Original comment by ramapcsx2.code on 28 Jun 2013 at 2:00

GoogleCodeExporter commented 9 years ago
Well let me know if you guys ever want this mainlined, I'll debug it for all 
the platforms.

Original comment by matt.sch...@gmail.com on 28 Jun 2013 at 9:23

GoogleCodeExporter commented 9 years ago
Would it be possible to merge the 2 patches into a single one with cxx11 
support define at compile time. It would be easier to see the difference.
Can you support both SDL1 and 2 with define like that:
#if SDL_VERSION_ATLEAST(1,3,0) (or maybe 2,0,0 now).

Can you change the license to "LGPL (v3 or later)", it is already complicated 
enough

By the way, cmake is linux only at the moment.

Original comment by gregory....@gmail.com on 30 Jun 2013 at 9:08

GoogleCodeExporter commented 9 years ago
Here is the single-version diff. spu2x compiles with or without sdl due to 
checks in the cmakelists, and doesn't require changes when sdl2 happens as long 
as FindSDL2 defines SDL2_FOUND rather than overriding SDL_FOUND, etc.

It only builds in c++11 on gcc, but according to what you say it doesn't matter.

The only bug is if you select sdl, close, open it again with an spu2x without 
sdl support when mods[sdl#] = NULL the whole thing crashes, but that happens 
any time a SndOut component is selected at the end of the list and then 
removed. The other option is to put it before Ports (and then regardless of the 
conditional inclusion of it worst case ports is used instead of sdl) but that 
would set anyone using ports to sdl if this ever went into a release, so I 
didn't do it.

I had licensed it under the gpl since other files in spu2x are gpl, but this 
one is lgpl (though the gpl is still leaky here due to files like iconvert).

Original comment by matt.sch...@gmail.com on 1 Jul 2013 at 12:41

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks. It is easier to see the difference this way. 

Don't worry for C++11, we are still supporting MSVC2010, and even MSVC2012 is 
far to support C++11. 
http://msdn.microsoft.com/en-us/library/vstudio/hh567368.aspx
On linux, I could request GCC4.7 as a bare minimum that I won't be an issue.

A small question, improvement on latency was tested on both SDL version or only 
SDL1/2?

Original comment by gregory....@gmail.com on 2 Jul 2013 at 11:34

GoogleCodeExporter commented 9 years ago
Only SDL1, it is a PITA to try testing the SDL2 one because you can't link SDL1 
anywhere or the symbol tables conflict against onepad and wxwidgets usage of 
sdl1 under linux, and I didn't try figuring out a way to build a runnable 
version without wx. There is no fundamental change in SDL2 concerning latency, 
and it still uses pulseaudio on the backend (the principle reason I implemented 
it) the only gains are that it can use the native 32 bit signed audio output 
from pcsx2 and could support positional surround sound (but since spu2x samples 
in stereo it is fruitless to upscale it).

It also isn't necessary to require a version bump of gcc, since it compiles 
fine in c++03 mode (and checks the compiler version before using c++11 
features), you just get some optimizations of initializer lists.

Original comment by matt.sch...@gmail.com on 2 Jul 2013 at 5:28

GoogleCodeExporter commented 9 years ago
Yes I know, that the main reason I removed SDL2 from GSdx. FYI, you can build 
wx without sdl support. Anyway did you try to select another device in 
portaudio?
At the start of portaudio, you got a list like that:
 *** Device 0: 'HDA ATI SB: ALC888 Digital (hw:0,1)' (ALSA)
 *** Device 1: 'HD-Audio Generic: HDMI 0 (hw:1,3)' (ALSA)
 *** Device 2: 'sysdefault' (ALSA)
 *** Device 3: 'iec958' (ALSA)
 *** Device 4: 'spdif' (ALSA)
 *** Device 5: 'default' (ALSA) (selected)
 *** Device 6: 'dmix' (ALSA)

You can select the "Device" in inis/spu2-x.ini file. You need to put the device 
name like 'dmix' or 'default'.

I only support compiler that I can install on my system and gcc4.6 is the 
oldest I can install as of today. Besides more and more c++11 improvement will 
come into PCSX2 source code. So it is likely that I will drop gcc4.6 support 
next years.

Original comment by gregory....@gmail.com on 3 Jul 2013 at 7:13

GoogleCodeExporter commented 9 years ago
Yea, I did a lot of research into ports as to why I was having such major audio 
issues, and my belief (it is woefully hard to debug, sadly) is that it is more 
bugs in the ALSA abstraction layer pulseaudio uses than ports fault - but that 
project doesn't intend to directly support pulseaudio servers, only assume that 
pulse is catching alsa sinks and redirecting them appropriately, but for some 
implementation detail somewhere between ports, alsa, pulse, the sound card 
drivers, and the eventual output, it either distorts when other alsa-sunk 
applications are running (ex, flash) or it has tremendous latency measured in 
seconds of delay.

That was the main reason I went for the SDL version, because it uses pulseaudio 
natively when available. It fixed the problem - I imagine the root bug is 
somewhere in pulse.

Original comment by matt.sch...@gmail.com on 3 Jul 2013 at 7:14

GoogleCodeExporter commented 9 years ago
I've been testing this for the last day or so. I'm another pulseaudio user 
that's had problems with the portaudio alsa and jack outputs. Gregory pointed 
me at this and overall, it works great. With the alsa output, I was getting 
constant buffer underflows and terrible sound. With jack, besides being 
annoying to start each time I run pcsx2, I was getting 500ms to 1s delays on 
sound. The one problem I've seen might not even be this patch at all, maybe 
spu2x internals. Sometimes when pausing and resuming, the sound will become 
scratchy. It might be triggered by a configuration change before resume, I 
haven't tinkered with it enough to narrow down the specific cause.

Original comment by webgeek1...@gmail.com on 8 Jul 2013 at 5:14

GoogleCodeExporter commented 9 years ago
Go into spu2-x/SndOut_SDL.cpp and change SPU2X_SDL_COUNT from 256 to 512 or 
1024 and tell me if that noise goes away. I was getting a similar effect on 
some devices trying to reproduce noise because 512 samples requires almost no 
slowdown in playback. (and coming back from pause is a significant source of 
delay).

Original comment by matt.sch...@gmail.com on 9 Jul 2013 at 6:20

GoogleCodeExporter commented 9 years ago
No, that didn't help the noise at all (tried 512 and 1024). And yes, after 
tinkering with it some more, I see that the noise is added every time I pause 
and resume.

Original comment by webgeek1...@gmail.com on 9 Jul 2013 at 4:41

GoogleCodeExporter commented 9 years ago
I just went down a rabbit hole. It turns out that SDL can internally change 
your SDL_AudioSpec samples even if you explicitly make desired null to use 
their internal conversion if your desired parameters don't work. Even if you 
use a desired spec, it will change *both* specs to whatever the internal buffer 
ends up using, and it will use that length in the callback.

It also *always* cuts samples in half on pulseaudio, so every time it would 
restart the sink it would cut the buffer size in half. The noise is from the 
buffer overrunning, and if you kept restarting it it would eventually crash. 
The fix uses runtime allocated arrays depending on the returned spec sample 
count from OpenAudio, and also always resets the sample size to the desired one 
each init, which is really hacked.

I'm going to post on the sdl forums about this if it is a bug or not, because 
it is really unexpected and undesired behavior (currently waiting on post 
rights confirmation).

While reading the sdl source, I found out it will also outright break if the 
underlying architecture doesn't support a used audio format, such as how 
pulseaudio in SDL2 doesn't support 32 bit yet, but I submitted a patch to SDL 
to fix that here: http://bugzilla.libsdl.org/show_bug.cgi?id=1949 so hopefully 
that gets in an early 2.X branch release.

Do test this to see if the bugs gone, and if you compile it in debug mode it 
will assert-error out if the buffers malign.

Original comment by matt.sch...@gmail.com on 10 Jul 2013 at 5:42

Attachments:

GoogleCodeExporter commented 9 years ago
This one works fine. Doesn't seem to affect resume performance much, so from a 
users perspective, the hack is fine. Though, from a developers perspective, I 
can understand how ugly that is.

Original comment by webgeek1...@gmail.com on 10 Jul 2013 at 4:20

GoogleCodeExporter commented 9 years ago
@Matt, I integrate your patch into svn with extra modification. I fix a couple 
of memory bug, replace sdl2 check by a typedef. I also drop most of the nullptr 
to enhance readability, they will mass-replaced when we switch to c+11.

@Rama, let's call it a temporary solution ;)

Original comment by gregory....@gmail.com on 14 Sep 2013 at 3:37