-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Audio SPU emulation (duckstation) #1601
base: master
Are you sure you want to change the base?
Conversation
Source/AliveLibAE/Sound/Midi.cpp
Outdated
{ | ||
std::vector<psx::Sample*> samples; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double endline
Source/AliveLibAE/Sound/Midi.cpp
Outdated
|
||
if (soundDatFile.read(soundData, fileSize)) | ||
{ | ||
// Tada! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
I don't see anything happening here, what's the magic trick?
Source/AliveLibAE/Sound/Midi.cpp
Outdated
|
||
EXPORT void CC SND_Stop_Channels_Mask_4CA810(u32 bitMask) | ||
{ | ||
bitMask; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line, and the ones in SND_Load_VABS_4CA350 don't seem to do anything, is this needed?
Source/AliveLibAE/Sound/Midi.cpp
Outdated
return player->SFX_SfxDefinition_Play(reinterpret_cast<psx::SfxDefinition*>(const_cast<SfxDefinition*>(sfxDef)), volume, pitch_min, pitch_max); | ||
} | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good practice to end the source code files with an endline at the end. Also, not having one in an .hpp file can cause problems with including multiple ones one after the other.
Seems multiple files miss it here.
@@ -228,8 +228,39 @@ EXPORT s32 CC SND_PlayEx_4EF740(const SoundEntry* pSnd, s32 panLeft, s32 panRigh | |||
pDSoundBuffer->SetFrequency(freqHz); | |||
pDSoundBuffer->SetVolume(sVolumeTable_BBBD38[panRightConverted]); | |||
|
|||
// OLD PAN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the new panning is superior, no need to keep the original bad one in, should be removed instead, with all vars that it exclusively uses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds panning to the existing SDL audio rendering and is unrelated to the SPU emulation path. Since the existing SDL implementation is mono it's superior in the sense that it adds panning. I'm mentioning in case you don't want to modify the existing audio renderer otherwise I can remove the old panning.
Source/AliveLibCommon/CMakeLists.txt
Outdated
audio/Stream.cpp | ||
audio/MidiPlayer.hpp | ||
audio/MidiPlayer.cpp | ||
audio/oddio.h) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep ) on the new line like it was originally :)
/* AO reads sequence inline with data. AE reads from a sounds.dat file */ | ||
virtual ResourceData* readSeq(const char_type* fileName, const char_type* sequenceName) | ||
{ | ||
fileName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably safe to remove, like the other lines like this in this file (and others)
void SPU::DeInit() | ||
{ | ||
mutex.lock(); | ||
// TODO - does it matter? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, you tell us 😛
{ | ||
SPUSeqStop(seqId); | ||
} | ||
// SPUSeqSetVolume(seqId, 100, 100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some commented out code here. If it's not needed, please remove :)
This is an attempt at replacing sound with Duckstation's SPU. It is a non-ABI breaking change and can be turned off and on using
options.cmake
valueAUDIO_SPU_EMULATION
.I have not hooked up FMV audio to the SPU emulator, but it should be trivial. I'll wait to see if this PR is even something that would be wanted.
MidiPlayer
defines the SPU/SND functions.Sequencer
is the modified duckstation SPUoddio
defines fixes for vag attributes (which I'll mention more below)The SPU runs at 44100hz which is executing in the SDL callback. The emulator should probably run in its own thread as running calculations in the SDL callback is probably bad practice. Unsure if this will cause clipping on slower CPU's. CPU usage seems comparable to master branch, however if many sounds are playing (slig constantly shooting), my CPU usage is slightly higher than master.
Interestingly the PC vag attributes differ from PSX. This can be seen by disabling the vag attribute fixes by commenting out
mSoundSampleParser.applyFix(...)
inMidiPlayer
. With it commented it will sound like pc, with it uncommented it will sound like psx. One obvious example is Abe's "Hello". He has a deeper voice in pc, whereas psx is slightly higher pitch.R6 is even more messed up and fixes can not be applied without more inspection, right now it uses all pc sounds, so Abe will have pc's lower pitch voice.
Further, I believe a lot of the volumes are incorrect in the sound definitions within things like
slig.cpp
.eHitBottomOfDDeathPit
at full volume. This should almost certainly be played at a lower volume.SND_SEQ_SetVol()
SND_SEQ_SetVol()
I have not applied fixes for AE, so that could probably be improved. However, I know greeters wheeling around make no noise, but it's because their volume is too low, increasing it the sound will play.
In short; I'm pretty sure the pc port introduced more sound issues than just the player, most likely to cover for the incorrect player.