Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPU2: Replace SndOut with AudioStream from DuckStation #11135

Merged
merged 6 commits into from
May 4, 2024
Merged

SPU2: Replace SndOut with AudioStream from DuckStation #11135

merged 6 commits into from
May 4, 2024

Conversation

stenzek
Copy link
Contributor

@stenzek stenzek commented Apr 23, 2024

Description of Changes

Guts the cruddy SndOut code that I've patched up so many times over the last couple of years, and replaces it with my.. erm.. nicer AudioStream class from DuckStation.

This has the benefit of:

  • Adds an SDL backend for those folks who are having issues with cubeb+pulseaudio on Linux.
  • Adds support for decoding surround formats through FreeSurround, including DPL/DPLII.
  • Improved time stretcher behaviour when running at faster speeds, and responds quicker to speed changes.
  • Resampling instead of outputting silence, which considerably reduces pops/crackles when changing speed.
  • Separate fast forward and normal speed volumes.
  • Much less gross code.
  • An actual working no-stretch mode.

Currently, Async Mix has been removed.

If you are depending on this feature, please let us know in this thread, and we'll consider re-introducing it. But personally I think it should stay gone, it breaks every game it touches in some way.

Rationale behind Changes

See above.

Suggested Testing Steps

Test audio-related things, including:

  • Standard audio output with all 3 backends.
  • DPL/DPLII decoding with quadaphonic or 5.1/7.1 setups, if you have one.
  • Hotkeys for volume control.
  • Fast forward volume/normal volume.
  • Sync to host refresh rate.
  • Make sure video dumping with audio still has audio.
  • Test performance at high frame rates (e.g. in menus), make sure there's no performance regression.

@Blackbird88
Copy link
Contributor

Adds support for decoding surround formats through FreeSurround, including DPL/DPLII.

Resolves #432?

@stenzek
Copy link
Contributor Author

stenzek commented Apr 23, 2024

No, this is not DTS decoding/passthrough.

@AmyRoxwell
Copy link

AmyRoxwell commented Apr 24, 2024

Pretty good so far, but it has an issue, it doesn't let you set per-game volume sliders for normal audio and fast-forward, this is very bad considering some games audio balance can be too quiet or too loud
Master:
Screenshot_04_23_2024-1

Will always be stuck to 100%
Pr:
Screenshot_04_23_2024

It also doesn't seem to keep the already establish volume levels from master, so you may cause someone to blow up their eardrums

@TheLastRar
Copy link
Contributor

TheLastRar commented Apr 24, 2024

Currently, Async Mix has been removed.

If you are depending on this feature, please let us know in this thread, and we'll consider re-introducing it. But personally I think it should stay gone, it breaks every game it touches in some way.

This setting is useful on underpowered systems
I've seen people use this to allow music to play at full speed when the game itself isn't.

Also I'm surprised to learn that is breaks every game. From what I'd been told it only really adversely affected a single rhythm game.

@bigol83
Copy link

bigol83 commented Apr 24, 2024

I have not an underpowered system and yet Async mix is useful for a game when the audio stutters no matter which syncronization mode i use. Keeping it, even if advising it can break games is the best solution in my opinion.

@stenzek
Copy link
Contributor Author

stenzek commented Apr 24, 2024

Also I'm surprised to learn that is breaks every game. From what I'd been told it only really adversely affected a single rhythm game.

It breaks a game that synchronizes anything to audio. FMVs, speech, sound effects, etc.

I've decided I'm keeping it scrapped, because it relied on a bunch of expressions that make absolutely no sense. If someone wants it that desperately, feel free to re-add it in a follow-up PR.

@TheLastRar
Copy link
Contributor

Also, should FreeSurround not be in 3rdparty?

@stenzek
Copy link
Contributor Author

stenzek commented Apr 24, 2024

It's basically a single file (the channel maps could go in the main file, even), and can benefit from AVX2. 3rdparty doesn't have AVX2 configs in MSBuild.

Edit: actually, why don't we have AVX2 for 3rdparty in MSBuild... that doesn't make any sense...

@TheLastRar
Copy link
Contributor

TheLastRar commented Apr 24, 2024

It's basically a single file (the channel maps could go in the main file, even), and can benefit from AVX2. 3rdparty doesn't have AVX2 configs in MSBuild.

AVX2 is a pretty good reason.

On that note, what about cmake builds using DISABLE_ADVANCE_SIMD and its MULTI_ISA_SHARED_COMPILATION?, (which I think is the default for nightly builds)
FreeSurround doesn't seem to be included in any of SourcesUnshared lists, was it not easily possible or worthwhile to do so?

@refractionpcsx2
Copy link
Member

I have not an underpowered system and yet Async mix is useful for a game when the audio stutters no matter which syncronization mode i use. Keeping it, even if advising it can break games is the best solution in my opinion.

Is that the case even with this PR? responsiveness has changed.

@stenzek
Copy link
Contributor Author

stenzek commented Apr 24, 2024

I've removed the discrepancy between core and 3rdparty, so I moved it out.

It doesn't make use of the wider vector width, so it's not going to make a non-trivial difference. Main benefit would just be more registers. But I doubt it's worthwhile complicating it with Multi-ISA.

@github-actions github-actions bot added the Dependencies Pull requests that update a dependency file label Apr 24, 2024
@Hirato
Copy link

Hirato commented Apr 24, 2024

Perhaps as a compromise, "Async Mix" can be restricted to only activate while turbo and slow modes are active?

I generally quite like the feature, and I usually don't see it cause any problems provided the game runs at full speed and isn't dropping frames - only exception in my own library is Xenosaga 2 where the cutscenes play at half speed with it.

I like it because I don't really like the pitch shift that comes from accelerating the audio playback, nor the effects of tiemstretching (nor its pitch correction) on the results.
The primary case I enjoy it is with games that use PCM for music and voice lines. Provided neither are tied to strict timers or animation cues, the audio plays at a completely natural and pleasant pace, whether the game speed is 20% (though it tends to underrun heavily around here) or 4x - Dungeon crawlers like Persona 4 would be a really good one for this case, as would Ratchet And Clank, which often struggles to run at full speed at the best of times.
It's even fairly pleasant for midi, as this just adjusts the tempo without making the audio sound particularly off.

And this one's more personal, as I am a developer myself.
But I appreciate that it also gives an inside view of how the individual games were made.
For example in Jak 2/3, if the game isn't running at full speed, the cutscenes still play at normal speed (I believe the inverse is true too), because this game is using audio cues for timekeeping purposes.

Another is Final Fantasy X, which relies extremely heavily on both audio cues and animation cues to progress its cutscenes, and I found it fascinating how the characters would return to T poses at times.
I was actually amused enough by it to stream nearly an entire playthrough from start to finish just to see all the ways turbo w/ async would wreak havoc with the cutscenes, and I do find it sad that it won't be possible going forward.

While I can understand it's a support burden, I hope you'll at least reconsider.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Apr 24, 2024

Perhaps as a compromise, "Async Mix" can be restricted to only activate while turbo and slow modes are active?

I generally quite like the feature, and I usually don't see it cause any problems provided the game runs at full speed and isn't dropping frames

but if you're turboing or slow mode, you aren't at full speed. you're either side of it.

Async mix is problematic as it messes with the emulation, since it's requesting audio samples at a different frequency to what is expected, and there's a whole bunch of games which don't like this behaviour. I can understand Stenzeks unwillingness to re-add it, it's a very hacky solution to a problem.

In regards to fast forwarding, in this PR it should sound a lot better now than it did. It will still be sped up (i mean the game is running fast, what do you expect?) but it'll be much smoother than it was.

For example in Jak 2/3, if the game isn't running at full speed, the cutscenes still play at normal speed

I'm not sure what this has to do with async? if the cutscenes are running slow but you want to async the sound, it just means all the sound fx and speech are going way out of time with what's happening, can you explain how this is a good thing?

@stenzek
Copy link
Contributor Author

stenzek commented Apr 24, 2024

With all the optimization work we've done throughout 1.7, you'd have to either have a very slow PC (as in, dual-core, probably >10 years old), and be trying to run a heavy game, to not be able to maintain full speed. So that side of async is kinda redundant these days.

But it's mainly because I can't be arsed to reverse-engineer the formula it used to screw with the tick interval. Which I'm pretty sure would break even more fantastically if the speed varied, and wasn't constant.

"haha it breaks the game in funny ways" isn't a reason to have a feature... Users see that, and think PCSX2 is rubbish as a result, worsening the unjustified negative reputation we have amongst some.

@Hirato
Copy link

Hirato commented Apr 24, 2024

but if you're turboing or slow mode, you aren't at full speed. you're either side of it.
I know.

I'm more just stating that where the target isn't "100% speed", Async Mixing is at its most desirable.

It's more that I personally don't want the audio sped up in turbo mode.
I had mentioned Persona 4 in this category; I personally find the dungeon crawling sections extremely dull and boring, and so I just turbo through them.
It's for this reason that I genuinely find one of PCSX2's greatest features is that I can both do this, but still enjoy the BGM at its normal speed, and even have all my companions yelling "Watch out senpai!" and other such lines at their normal cadence.

I'm not sure what this has to do with async? if the cutscenes are running slow but you want to async the sound, it just means all the sound fx and speech are going way out of time with what's happening, can you explain how this is a good thing?

As I understand it, Jak 2/3 uses silent audio cues for time keeping.
Which is to say a second's worth of "silence" makes a second's worth of time pass in the game, even if the framerate isn't keeping up.
The cutscenes are animated based on measuring the elapsing of realtime in this manner, so the audio and the visuals actually stay completely in sync.
The game even features slow/fast movie "cheats", which mess with this inside the game itself.

I mentioned it more as a interesting curiosity that the current Async Mix implementation reveals.
I'm personally not aware of any other game that does timekeeping in a similar manner.

@stenzek
Copy link
Contributor Author

stenzek commented Apr 24, 2024

It's for this reason that I genuinely find one of PCSX2's greatest features is that I can both do this, but still enjoy the BGM at its normal speed, and even have all my companions yelling "Watch out senpai!" and other such lines at their normal cadence.

But it's not normal. Because you could easily run out of voices, and lines could get missed entirely, which will just result in them being missed at best, or softlock/crash the game at worst. I can see the advantage here, but it's really not worth the risk of breaking games for users.

@bigol83
Copy link

bigol83 commented Apr 24, 2024

I have not an underpowered system and yet Async mix is useful for a game when the audio stutters no matter which syncronization mode i use. Keeping it, even if advising it can break games is the best solution in my opinion.

Is that the case even with this PR? responsiveness has changed.

Yeah, unfortunately it still happens. The drops are too steep to be smoothed out. Audio stutter is pretty noticeable. The game I am talking about is PES 5, I already mentioned it on Discord, those drops from 60 to 50 fps last less than half second but they are enough to make the sound stutter.

Maybe there is a way in this PR to make audio synchronization less sensible to half second fps drops, I didn't test every setting.

@stenzek
Copy link
Contributor Author

stenzek commented Apr 24, 2024

The higher the buffer size (=> target latency), the less impact frame time spikes will have, and/or rate drops, since the stretcher will have more of a buffer to catch up.

@AmyRoxwell
Copy link

AmyRoxwell commented Apr 24, 2024

Okay, the update fixes the issue of not being able to adjust volume per-game, good job...

Now there is a new bug where changing the volume at all will crash the emulator on linux, like just moving the sliders for both the fast-forward and normal volume in both per-game and global audio settings will just froze and crash the appimage.

The appimage spits this on the terminal:
1st.txt
woag.txt

Video of it happening:
https://youtu.be/utrcA4Cn2zo?si=cBK-NEKEPSZjLSmF

@seta-san
Copy link
Contributor

Sometimes I wonder how Avx2 isn’t just the minimum standard. It’s been around since 2013.

@Ziemas
Copy link
Contributor

Ziemas commented Apr 24, 2024

Sometimes I wonder how Avx2 isn’t just the minimum standard. It’s been around since 2013.

Because of intel doing shitty market segmentation by arbitrarily excluding it from low cost models.

@Ziemas
Copy link
Contributor

Ziemas commented Apr 24, 2024

This looks good, and from quick testing it seems to behave better than the old output system. I'm also fully in favor of getting rid of async.

@TransparentBlue
Copy link

I'll second async mix being useful here (5900X / RTX 4080), there's always some very heavy game that dips for a little tiny bit and async mix smoothes that out, then again if it's only async mix and not both async mix and none that's gone that's fine.

@RedDevilus
Copy link
Contributor

RedDevilus commented Apr 24, 2024

Master:

image

PR with old config:
image

PR with defaults:
image

Maybe set it back to the older and lower latencies?

Though some weird behaviours when I press Minimal Checkbox the first time, and unchecking does make it the old master default again:

image

image

Because default is 50 + 50, when I press Minimal it does 50 + 10 and uncheck it, it is 50 + 20 essentially.

@Mrlinkwii
Copy link
Contributor

looks good ( needs a good ammount of testing) one very very minor thing i found on the per game settings
image
I'm also fully in favor of getting rid of async.

@TheLastRar
Copy link
Contributor

TheLastRar commented May 6, 2024

why it wasn't noted in the changelog.

Not every change is indicated. I didn't bother creating separate commits to remove XAudio, because that would mean changing the backend list in several places, and all that code was being tossed out with the AudioStream replacement.

I assume they meant listing it in the PR description (i.e. like you did with Async)

@stenzek
Copy link
Contributor Author

stenzek commented May 6, 2024

We discussed it internally, so it's not like nobody was aware :) Remember, they're nightly builds, so anything is subject to change.

Async was a possible point of contention, hence why I mentioned it.

Edit: I forgot, it was also a late change, the initial version of the PR still had an XAudio backend... I tossed it later on due to latency issues, and my slack ass didn't update the original post.

@michaelx333
Copy link

It wasn't just volume difference, that'd be easy to fix by increasing volume. It was more like parts of the music sounded quieter than they should, while the volume level was the same.

@stenzek
Copy link
Contributor Author

stenzek commented May 6, 2024

It wasn't just volume difference, that'd be easy to fix by increasing volume. It was more like parts of the music sounded quieter than they should, while the volume level was the same.

That's impossible. The frames generated by the emulated SPU2 are the same regardless of the backend.

@TheLastRar
Copy link
Contributor

It wasn't just volume difference, that'd be easy to fix by increasing volume. It was more like parts of the music sounded quieter than they should, while the volume level was the same.

Record both XAudio & Cubeb and compare them, either by listening or comparing the outputted waveform, and tell us, more specificity, where it differed (along with providing the recordings)

@michaelx333
Copy link

It wasn't just volume difference, that'd be easy to fix by increasing volume. It was more like parts of the music sounded quieter than they should, while the volume level was the same.

Record both XAudio & Cubeb and compare them, either by listening or comparing the outputted waveform, and tell us, more specificity, where it differed (along with providing the recordings)

I thought of doing that but I dont even know how to do it and it would be too much work anyway. I only compared them by listening (switching back and forth between them)

@TheLastRar
Copy link
Contributor

TheLastRar commented May 6, 2024

I thought of doing that but I dont even know how to do it and it would be too much work anyway. I only compared them by listening (switching back and forth between them)

Tenacity has the ability to record PC audio using WASAPI
image
You can compare them in Tenacity or export them in WAV (or another uncompressed format) to compare elsewhere.

I did this using part of Jak3's intro, but I didn't see any differences between cubed/XAudio2 on build 1.7.5745

@Ziemas
Copy link
Contributor

Ziemas commented May 6, 2024

It wasn't just volume difference, that'd be easy to fix by increasing volume. It was more like parts of the music sounded quieter than they should, while the volume level was the same.

That would imply that xaudio is doing further audio processing to cause this impression. Since we've not knowingly enabled such a thing I would suggest it is undesirable.

@ThreeDeeJay
Copy link

What is the use case for XAudio2 that Cubeb nor SDL covers? If there's a legitimate use case, I can consider re-adding it, but it has rather bad latency/buffering performance.

This maybe?

I don't think that would do anything for pcsx2 anyway, we don't use those sound positioning API's.

X3DAudio HRTF wouldn't be able to add full 3D audio (which is the main goal) even with XAudio2 alone, perhaps only virtual surround, if XAudio2 was also using X3DAudio.
But since X3DAudio HRTF is loosely based on old OpenAL Soft code, I'd suggest implementing that backend instead, like Dolphin and Ryujinx, which allow virtualizing the decoded surround channel mix using HRTF to hear the surround effects on any device and using headphones), especially if devs eventually manage to use the sounds with 3D coordinates directly for true/object-based 3D audio, which is also supported by OpenAL Soft 👀👌

Anyhow, going back on topic, here's a quick test of the new surround decoder using HeSuVi (system-wide surround virtualization for headphones) https://youtu.be/fWIUJqPfpNA
Dolby Pro-Logic II decoding is pretty good, which is to be expected from FreeSurround, though I agree with #432, using the raw surround mix directly before/without encoding and decoding it using proprietary, lossy formats would provide a much cleaner audio signal.

@ThreeDeeJay ThreeDeeJay mentioned this pull request May 10, 2024
@Ziemas
Copy link
Contributor

Ziemas commented May 10, 2024

using the raw surround mix directly before/without encoding

There is no such thing. The SPU is inherently 2channel, DPLII is "encoded" on the fly using its volume registers (it supports setting negative volumes to invert the phase). The digital formats are usually only used for prerendered FMV's (although some games dynamically encode DTS on the CPU)

@ThreeDeeJay
Copy link

I see. I think I misinterpreted that issue so my apologies 😅
So while I know it'd probably take quite some effort, hypothetically speaking, wouldn't it be possible to restructure the SPU to allow separating the 6 (front and rear + maybe LFE) channels before the game/engine has a chance to apply any DPL encoding? I'm guessing at some point it should know what's front or behind so it knows whether to invert the phase or use any other filters to push audio to the rear channels 🤔
I'm more of a sound guy than a programmer, so I've always wondered why emulators didn't just do that instead of having to bother with decoding matrix upmixed surround formats (other than the general lack of interest in surround sound lol)

@stenzek
Copy link
Contributor Author

stenzek commented May 10, 2024

No, the SPU does not support outputting anything other than stereo via mixing. There's no registers for anything other than left or right volume, for each voice. You'd have to define some other emulated interface for getting the sound out of the game, and that would involve very intensive reversing and patching.

I say this too many times, emulators are not magic.

@Ziemas
Copy link
Contributor

Ziemas commented May 10, 2024

I see. I think I misinterpreted that issue so my apologies 😅 So while I know it'd probably take quite some effort, hypothetically speaking, wouldn't it be possible to restructure the SPU to allow separating the 6 (front and rear + maybe LFE) channels before the game/engine has a chance to apply any DPL encoding? I'm guessing at some point it should know what's front or behind so it knows whether to invert the phase or use any other filters to push audio to the rear channels 🤔 I'm more of a sound guy than a programmer, so I've always wondered why emulators didn't just do that instead of having to bother with decoding matrix upmixed surround formats (other than the general lack of interest in surround sound lol)

There's not really any opportunity to do so. All the SPU gets is a left and right volume per sound channel. All sound positioning is calculated by game code.

@Gaetanol4

This comment was marked as spam.

@zeromon
Copy link

zeromon commented May 25, 2024

Hello
i'm upgrade pcsx2 from version v1.7.5572 to v1.7.5839
and i'm found asnyc mix for audio is gone in new version, and i'm looking in github for this problem and found this thread
i'm know it's get warning when using async mix, but it's optional i can use it or not,
so far my game not have problem with async mix.

i'm using laptop and set power plan to high performance, but sometime it's still get frame drop
like game NFS Most wanted from 60 fps to 50 fps (It only happened a few times), when fps drop audio slowdown to match with fps, i know it's normal when using TimeStretch and normal for game when fps drop
soundtrack slowdown but it's drop only 10 fps and soundtrack slowdown like 50% or 80%
it's uncomfortable when driving and suddenly soundtrack is slowdown
and solution for me is using async mix but in new version is gone

there is my laptop specs for running pcsx2:
processor : intel core i7-8750H 2.1Ghz-4.1 Ghz
RAM : 16 GB
storage : SSD nvme
GPU: Intel UHD 630 and Geforce RTX 2060

I hope async mix can back for future update

@refractionpcsx2
Copy link
Member

if your hardware isn't able to maintain full speed, you should use an older version if the sound speed is a problem (and considering the speed drops, it's a least of your problems). I would however look at trying to improve your performance of your laptop, it shouldn't be terrible.

@mirh
Copy link

mirh commented May 25, 2024

Undervolt certainly could help squeezing an extra 10 or even 20 percent improvement (especially on coffee lake).
On the other hand I think the leitmotif is pretty much always the same (that's no potato pc btw)? When you are close-but-not-quite to full speed, desynchronization sounds subjectively better than gaps.
Of course it can break some games, but otherwise I haven't seen a refutation of this ordinal preference being legit.

@Raltyro
Copy link

Raltyro commented Jun 12, 2024

I've been trying to search what commit that removed the Async Mix and it was this
I still needed the async mix for rhythm games because hell my laptop struggles run in 60fps and mostly runs in 54fps
No hurt feelings but yeah it would be great if ASync Mix would come back only for games that only depends on audio

@stenzek
Copy link
Contributor Author

stenzek commented Jun 12, 2024

You definitely didn't want to use async mix for rhythm games, or anything that requires tight synchronization. It's going to be wrong.

@Raltyro
Copy link

Raltyro commented Jun 12, 2024

Without the Async Mix the rhythm would feel wrong

@refractionpcsx2
Copy link
Member

Async mix would be the worst thing as the music will keep going way ahead of the actual game, that's completely the opposite to what you want on a rhythm game.

I would suggest joining the discord and see if anybody can give you suggestions on how to squeeze a bit more performance out of that laptop, if we can

@ghost
Copy link

ghost commented Jun 12, 2024

Losing Async Mix is quite unfortunate for MGS3, which is very resource intensive and otherwise had no bugs with the option enabled.

The game normally drops frames on PS2, but on pcsx2 it experiences slowdowns instead (and seems to target 60fps during many cutscenes instead of 30fps on PS2). Async Mix used to fix this and allow the game to drop frames like PS2.

Also, for Metal Gear Online changing the emulation speed is frowned upon because it messes with the server. Without any alternative, players with weak hardware will unintentionally harm the game for others.

@PCSX2 PCSX2 locked as off-topic and limited conversation to collaborators Jun 12, 2024
@PCSX2 PCSX2 unlocked this conversation Jun 12, 2024
@Moshiro896
Copy link

Hey, stenzek, i understand that you removed this option since it can cause unnecessary trouble to you developers, but i really really miss that option, i only used with midnight club 3 and never had any issue with it, i could speed up the game in some slow parts like the garage or the loadings and still enjoying the game amazing soundtrack basically a QOL thing for me, if it is not too much to ask, can you please consider bringing it back? i do really appreciate your efforts in making this project bigger and better.

@JordanTheToaster
Copy link
Member

Hey, stenzek

No it will not be coming back.

@F0bes
Copy link
Member

F0bes commented Jun 19, 2024

If someone wants to properly implement an async mix option, they can go ahead and pr it. I, and some others after internal discussion, believe that it is a useful option for those games that are just a little too demanding for the host hardware. It's just a matter of who wants to implement it.
As of now, there can't really be any more constructive discussion here, locking.

@PCSX2 PCSX2 locked and limited conversation to collaborators Jun 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Build | Project System Dependencies Pull requests that update a dependency file GS GUI/Qt SPU2
Projects
None yet
Development

Successfully merging this pull request may close these issues.