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

Fix 2 DTK bugs #8349

Merged
merged 1 commit into from Sep 15, 2019

Conversation

@Pokechu22
Copy link
Contributor

commented Sep 4, 2019

Bug 1: subcommand 1 of 0xE1 does not actually reset the current position; it just stops playback. 0xE2 returns the same value it would have before stopping playback (other than reporting that the stream is stopped).

Bug 2: subcommand 3 of 0xE2 was incorrectly dividing the length by 4; while this makes sense for positions (which are multiplied by 4 earlier to get a byte offset), it is not correct for lengths.

@Pokechu22

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2019

Hardware test, which should at least work on the Wii.

@lioncash

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

It'd probably be beneficial if the hardware test was integrated with our hwtests in some capacity

Fix 2 DTK bugs
Bug 1: subcommand 1 of 0xE1 does not actually reset the current position; it just stops playback.  0xE2 returns the same value it would have before stopping playback (other than reporting that the stream is stopped).

Bug 2: subcommand 3 of 0xE2 was incorrectly dividing the length by 4; while this makes sense for positions (which are multiplied by 4 earlier to get a byte offset), it is not correct for lengths.

@Pokechu22 Pokechu22 force-pushed the Pokechu22:dtk-bugfix branch from a0d86bd to 3efa3d7 Sep 15, 2019

@Pokechu22

This comment has been minimized.

Copy link
Contributor Author

commented Sep 15, 2019

I'll probably PR the hwtests repository later with a more complete set of automated tests for my bigger DI pull request; for now the test is manual so it probably doesn't need to be added.

@BhaaLseN
Copy link
Member

left a comment

Changes seem sensible. Fine by me if the hwtest agrees. Untested.

@JosJuice JosJuice merged commit 82fd7f5 into dolphin-emu:master Sep 15, 2019

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.