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

[DVD] Ignore buffer when seeking backwards #3673

Merged
merged 1 commit into from Feb 26, 2016

Conversation

@mmastrac
Copy link
Contributor

mmastrac commented Feb 25, 2016

This fixes Shrek 2, but needs some testing on other titles that might have broken with the previous signedness fix.

The buffer definitely shouldn't be used for large backwards seeks, but with this patch we're also ignoring it for small backwards seeks. This may be a reasonable assumption.

Review on Reviewable

@JosJuice

This comment has been minimized.

Copy link
Contributor

JosJuice commented Feb 25, 2016

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@JosJuice

This comment has been minimized.

Copy link
Contributor

JosJuice commented Feb 25, 2016

Based on us talking in IRC, it seems like the code before PR #3632 was actually working correctly, but it was non-obvious that it was working correctly because it relied on underflow. PR #3632 made the behavior wrong. This PR brings back the correct behavior, written in a way that's easier to understand.

@mmastrac

This comment has been minimized.

Copy link
Contributor Author

mmastrac commented Feb 25, 2016

Do not merge yet -- I'm going to try and reason through this.

@mmastrac mmastrac force-pushed the mmastrac:dvd_nobackwardsbuffer branch from abb5c4e to f1a5d6c Feb 25, 2016
Unfortunately this fix re-breaks Arc Rise Fantasia, so we must restore its INI as well.
@JosJuice

This comment has been minimized.

Copy link
Contributor

JosJuice commented Feb 25, 2016

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@JosJuice

This comment has been minimized.

Copy link
Contributor

JosJuice commented Feb 26, 2016

All of PR #3632's known effects (Arc Rise Fantasia not crashing, Shrek 2 crashing, SpongeBob's Boating Bash crashing, faster loading times in general) have now been tested with this PR and found to be the same as before PR #3632.

delroth added a commit that referenced this pull request Feb 26, 2016
[DVD] Ignore buffer when seeking backwards
@delroth delroth merged commit 1b37b39 into dolphin-emu:master Feb 26, 2016
11 of 12 checks passed
11 of 12 checks passed
pr-osx-x64 Auto build in progress
Details
code-review/reviewable Review complete: all files reviewed, all discussions resolved
Details
default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on the Buildbot.
Details
pr-android Build succeeded on the Buildbot.
Details
pr-deb-dbg-x64 Build succeeded on the Buildbot.
Details
pr-deb-x64 Build succeeded on the Buildbot.
Details
pr-freebsd Build succeeded on the Buildbot.
Details
pr-ubu-nogui-x64 Build succeeded on the Buildbot.
Details
pr-ubu-x64 Build succeeded on the Buildbot.
Details
pr-win-dbg-x64 Build succeeded on the Buildbot.
Details
pr-win-x64 Build succeeded on the Buildbot.
Details
@mmastrac mmastrac deleted the mmastrac:dvd_nobackwardsbuffer branch Feb 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.