DTK rewrite #259
DTK rewrite #259
Conversation
|
skid_au expressed some concern on IRC that some games (maybe the ones mentioned in magumagu@f0a5214 ?) might be affected specifically by the changes involving g_bStream. Our current behavior is clearly wrong... but it's possible I broke something anyway. I don't want to make further changes based on guesswork, so any testing on games using DTK audio, especially Pac-man Fever or Eternal Darkness, would be welcome. (If anyone wants a Windows build, contact me on IRC.) |
|
So I don't forget, skid_au also mentioned Zoids: Battle Legends. |
|
Updated with a few more tweaks. Testing shows this version seems to be working well (but the games skid_au mentioned still haven't been tested). I think this is ready to be reviewed. |
| @@ -74,11 +75,15 @@ unsigned int CMixer::Mix(short* samples, unsigned int numSamples, bool consider_ | |||
| s16 l1 = Common::swap16(m_buffer[indexR & INDEX_MASK]); //current | |||
| s16 l2 = Common::swap16(m_buffer[indexR2 & INDEX_MASK]); //next | |||
| int sampleL = ((l1 << 16) + (l2 - l1) * (u16)frac) >> 16; | |||
| sampleL += samples[currentSample + 1]; | |||
| MathUtil::Clamp(&sampleL, -32767, 32767); | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Is this still a work in progress PR? |
|
We should probably post a test build to the forums to get more testing coverage... but other than that I think it's basically finished. I might try to see if I can write an hwtest to figure out some edge cases at some point, but that doesn't need to block this patch. |
|
So, what are the results of the forum testing grounds? |
|
poke @magumagu |
The primary motivation here is to make sure we submit samples from the CPU thread. This makes sure the timing of related interrupts accurate, and generally keeps the different kinds of audio synchronized. This will also allow improvements to audio dumping functionality. The new code is also more concise because it gets rid of some duplicated audio mixing code.
The code actually handles this case correctly; the algorithm is linear interpolation between the two closest samples, and the way it is written should work correctly with any ratio.
|
I got some testing... apparently this breaks Crazy Taxi, and I'm not sure why. |
|
I didn't have any such problems with Dave Mirra; I'll try Tony Hawk some more in a bit. |
|
Crazy Taxi has a known issue where it repeats the first song only (All I Want). Whether or not that's related, I'm unsure |
|
Can you describe the conditions or give more information on the issue? @magumagu It appears this bug isn't because of your branch, a forum post describes it as happening in 4.0-738 or so; well before your rewrite. You could probably handle it separately or leave it for someone else. The Tony Hawk Games work fine for me in switching songs. |
|
Retested on the latest build. All is good to me except the volume. Pacman Fever had music switching problems before, and so did Crazy Taxi. Tony Hawk Pro Skater 3/4 now work fine. All my other DTK games run well enough. |
|
LGTM now. Does as advertised, makes DTK synchronous without introducing new regressions. |
| m_streaming_mixer.SetVolume(lvolume, rvolume); | ||
| } | ||
|
|
||
| void CMixer::MixerFifo::SetVolume(unsigned lvolume, unsigned rvolume) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
There's a few minor regressions I noticed now playing games a lot more. 1: Audio in Tony Hawk Pro Skater 4 is different. I'm not sure it's inaccurate, but it SFX is louder. Adjusting the in-game audio is fine for it, but, I don't know if Dolphin as wrong before or not. I will test on console. |
|
@JMC47 Please don't use closed PRs to report issues. Use the issue tracker for that instead, and CC the PR authors in them. If anything, you can then link to those issue reports from the PR. Reason is that otherwise there's a chance the regressions just get forgotten about. |
Still a work in progress; submitting now for comments to see if I'm going in the right direction.
Rewrite DTK processing so we submit samples from the CPU thread.
Haven't done much testing yet, and there's probably some cleanup left, but
everything appears to be essentially working.