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

DTK: Adjustments attempting to increase accuracy #664

Merged
merged 1 commit into from Jul 26, 2014

Conversation

booto
Copy link
Contributor

@booto booto commented Jul 23, 2014

Changes how DTK is handled to better reflect real hardware. See below for a more detailed description of changes.

@@ -251,6 +257,7 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base)
mmio->Register(base | AI_INTERRUPT_TIMING,
MMIO::DirectRead<u32>(&m_InterruptTiming),
MMIO::ComplexWrite<u32>([](u32, u32 val) {
DEBUG_LOG(AUDIO_INTERFACE, "AI_INTERRUPT_TIMING=%08x@%08x", val, PowerPC::ppcState.pc);

This comment was marked as off-topic.

@JMC47
Copy link
Contributor

JMC47 commented Jul 23, 2014

After I made some mistakes testing the synchronous DTK audio merge, I made sure to more carefully test the games. I made sure that three things happened.

1: I made sure the DTK audio actually looped.
2: I made sure tracks changed properly (to the best of my knowledge.)
3: I checked for regressions in non-DTK games.

Games Tested

Tony Hawk Pro Skater 3: Regression with Music not changing tracks automatically fixed
Tony Hawk Pro Skater 4: Regression with Music not changing tracks automatically fixed
Crazy Taxi: Regression with Music not changing tracks automatically fixed
Pacman Fever: Issues with music looping fixed.
Starfox Adventures: Audio seems to work fine. For some reason, subtitles haven't shown up in this game in a while. I doubt it's DTK related, but maybe the subtitles are in track information? Or something?
Dave Mirra Pro BMX 2: No regressions. Seems to play one song then switch to another, but Dolphin was doing that before the DTK rewrite. Will check it on console.

Super Monkey Ball 1 and 2 do use streaming audio according to every site, but I didn't see it hitting the AudioInt logs. They play fine still regardless.

Regressions (since 4.0)
Streaming seems globally louder than it was before the merge. Noticeable in Tony Hawk series, Super Monkey Ball, Dave Mirra Pro BMX2 and others. I can't tell if this is a bug for sure, but the music does seem too loud compared to the old music.
Pacman Fever sometimes plays a short bit of music early. I have heard it do this on console as well, but seems to do it more often on Dolphin. More research is needed. Audio design on the game is horrible.

@lioncash
Copy link
Member

Can you give a brief description on how it increases accuracy? I don't know much about the DTK/Audio emulation in general, so it would be sort of nice to know.

@phire
Copy link
Member

phire commented Jul 23, 2014

The save state version should be incremented, as a new variable has been added.

@booto
Copy link
Contributor Author

booto commented Jul 24, 2014

Resync'd with STATE_VERSION bump.

@booto
Copy link
Contributor Author

booto commented Jul 24, 2014

@lioncash

Before this PR, DTK would generate an AI interrupt when a track ended (it's not meant to) which helped to hide other problems with the actual mechanism that does generate interrupts.

AI_SAMPLE_COUNTER is a 32 bit register that counts the number of samples played via streaming. I suspect it's actually triggered by just a clock because the standard AI startup code requires it to tick over before initialising any tracks via DVDInterface.

The AIS-related interrupt is based on the value written in AI_INTERRUPT_TIMING. When AI_SAMPLE_COUNTER ticks over to the same value, the interrupt fires. To get an AI_INTERRUPT_TIMING value of 0x00000000 to cause an interrupt, I believe AI_SAMPLE_COUNTER would need to tick over from 0xFFFFFFFF to 0x00000000.

The previous code used AIINTVLD as an interrupt mask. YAGCD disagrees with this, and real bytecode also seems to disagree (AIINTVLD is set to 0, it sets up a SampleCount based interrupt & the title hangs if it doesn't fire). With the old code, this meant the interrupt wouldn't fire due to the SampleCounter, but it would just generate one at end-of-track inside DVDInterface and so no hang/visible problem manifested

DVD command 0xE1 is used to queue up tracks for use w/ streaming. It has a depth of one (i.e. the currently playing track and the next one). If a 'queue track' command is sent with position/length both zero, it means that streaming should stop after the end of the current track. There are some other semantics about the position of the streaming when this stop occurs, the code should reflect those. When a track loops, it's really just because the next track is itself. When a track's info moves from Next to Current, the value in Next is not discarded.

The offsets the DVD's streaming interface uses are always with respect to the start of the disk, and it retains values on track stop.

@booto
Copy link
Contributor Author

booto commented Jul 24, 2014

Fixed an issue where stopping AI via PSTAT whle a track was playing, but the track continued to advance (audio was silenced though). Stopping AI now pauses the stream, which seems to be the expected behaviour.

@lioncash
Copy link
Member

@booto Thanks for the explanation :)

@@ -251,6 +257,7 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base)
mmio->Register(base | AI_INTERRUPT_TIMING,
MMIO::DirectRead<u32>(&m_InterruptTiming),
MMIO::ComplexWrite<u32>([](u32, u32 val) {
DEBUG_LOG(AUDIO_INTERFACE, "AI_INTERRUPT_TIMING=%08x@%08x", val, PowerPC::ppcState.pc);

This comment was marked as off-topic.

This comment was marked as off-topic.

@delroth
Copy link
Member

delroth commented Jul 26, 2014

@booto: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.


LGTM, I'll let you consider if you want more testing to be done before merging this. I haven't really been following but the code looks good.

@dolphin-emu-bot allowmerge

dolphin-emu-bot added a commit that referenced this pull request Jul 26, 2014
DTK: Adjustments attempting to increase accuracy
@dolphin-emu-bot dolphin-emu-bot merged commit 30962ec into dolphin-emu:master Jul 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants