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

Counters: Fix Hblank calculation for DVD videmodes #2239

Merged
merged 1 commit into from Feb 8, 2018

Conversation

Projects
None yet
8 participants
@ssakash
Member

ssakash commented Dec 29, 2017

Summary of changes:

  • Made the DVD variant video modes to use the timing algorithm used by all the analog video modes, previously it was using the one we had for HD/SDTV modes which was weird. (Refer commit message for more information)

@PCSX2/bug-squad
This change would influence PSX games mostly, I don't have any PSX games with me so tests are appreciated.

Counters: Fix Hblank calculation for DVD videmodes
Previously, the DVD variant NTSC/PAL modes used the horizontal blanking
interval calculation algorithm used by digital video modes, which
shouldn't be used and also rounding error check was neglected.

Added the DVD variant modes to the list in analog video mode finder
subroutine. This should impact timing/vertical synchronization in PSX
games significantly.
@MrCK1

This comment has been minimized.

Member

MrCK1 commented Dec 29, 2017

Can't recall if any of my PSX/PS2 games even trigger the DVD modes, I'll test this once it compiles anyway.

@ssakash

This comment has been minimized.

Member

ssakash commented Dec 29, 2017

Can't recall if any of my PSX/PS2 games even trigger the DVD modes, I'll test this once it compiles anyway.

I thought PSX games always use NTSC/PAL DVD modes?

@MrCK1

This comment has been minimized.

Member

MrCK1 commented Dec 29, 2017

I did a quick check with #2237 to help. It looks like you're right, the BIOS also uses it. I don't see anything different, I'll look around some more.

Edit: Everything looks fine to me. Anybody else is free to test.

@seapancake

This comment has been minimized.

seapancake commented Dec 29, 2017

Tested Rapid Reload (One the better games compatibility-wise and no difference, used #2237 build as well to confirm game uses DVD PAL.

@lightningterror

This comment has been minimized.

Member

lightningterror commented Dec 29, 2017

@RedPanda4552 can do some tests, he's got a huge collection.

@RedPanda4552

This comment has been minimized.

Contributor

RedPanda4552 commented Dec 29, 2017

*Growing. But yes, I can. Is this change for the CDVD Plugin mode, ISO mode or both?

@RedPanda4552

This comment has been minimized.

Contributor

RedPanda4552 commented Dec 29, 2017

Using out of box settings, except PSX titles marked as needing IOP interpreter and I used OGL SW renderer for all. Used ISO mode. For reference, here was my test pool by system:

PS2

  • Gran Turismo 4
  • Lego Star Wars The Original Trilogy
  • Ratchet and Clank Going Commando

PSX

  • Digimon World 2
  • Digimon World 3
  • Final Fantasy 8 (first disc only)
  • Hot Wheels Turbo Racing (IOP Interpreter)
  • Lego Rock Raiders (IOP Interpreter)
  • Toy Story 2 (IOP Interpreter)

#2237 Results
All PS2 gameplay was NTSC. The PS2 BIOS splash always ran as DVD NTSC.
All PSX gameplay was DVD NTSC.

#2239 Results
PS2 games appear unaffected. Lots of console output complaining about microVU1 and IPU, but I think that was due to SW renderer and not tweaking recompilers to match the games.

PSX Games:

  • FMV corruption unchanged.
  • EE hyperactivity unchanged.
  • SIO quirks and memcard speeds unchanged.
  • DMA3 Not Ready on PSX FMV's with -3 EE underclock (and sometimes on normal EE clock) unchanged.
  • Game load speeds (not to be confused with memcard load) seem unchanged.
  • Digimon World 3's frame repeating/strobing problem in HW renderers is unchanged.
  • Lego Rock Raiders' game speed/synchronization discrepancies unchanged. Though, ingame graphics corruption seems to have disappeared. Loading screen graphic corruption is unchanged, however.

I just sort of threw out every "glaring" issue I've seen in PSX mode. Is there anything in particular I should be looking for?

@ramapcsx2

This comment has been minimized.

Member

ramapcsx2 commented Jan 8, 2018

I'm out of the loop on what we consider "digital video mode".
All the PS2 video modes are analog, just with different timings.
Is the question here whether rounding should be applied?

@ramapcsx2

This comment has been minimized.

Member

ramapcsx2 commented Jan 14, 2018

I'm going to merge this, on the assumption that this patch represents the currently best accepted theory on how this timing mode works.

@ssakash

This comment has been minimized.

Member

ssakash commented Jan 14, 2018

I just sort of threw out every "glaring" issue I've seen in PSX mode. Is there anything in particular I should be looking for?

The last time we got this wrong, it caused skippy FMV and out of sync issues in few games (#1368 (comment))

Thanks for all the tests by the way. :)

I'm out of the loop on what we consider "digital video mode".

Not probably the accurate term but I just consider modes other than NTSC/PAL standard as digital video modes. I just use the classification between them to manage the timing code more accurately.

Is the question here whether rounding should be applied?

Not just rounding, there's also halving of the hblank and hrender in case of video modes other than NTSC/PAL, previously that was being applied while it shouldn't have been.

I'm going to merge this, on the assumption that this patch represents the currently best accepted theory on how this timing mode works.

Sure, I'm happy with the patch for now, seems like it fixed an issue (mentioned in pandubz's post) and hasn't caused any regressions so far.

@ramapcsx2

This comment has been minimized.

Member

ramapcsx2 commented Jan 15, 2018

halving of the hblank and hrender

vSyncInfoCalc() is just so full of speculation, it's hard to tell if any later timing changes do the right thing.
It should definitely be rewritten.

// Important! The hRender/hBlank timers should be 50/50 for best results.
// (this appears to be what the real EE's timing crystal does anyway)
u64 hBlank = Scanline / 2;
u64 hRender = Scanline - hBlank;

Yea, I find that hard to believe. Even if the comment says that it's true. I just don't know how we can test for it.
It'd be best to integrate a test for this with the video mode test homebrew.

@FlatOutPS2

This comment has been minimized.

Member

FlatOutPS2 commented Jan 15, 2018

I did a quick test with a number of PSX and PS2 games and noticed no issues or other changes. I specificially checked for choppy/out of sync FMVs as mentioned, but there seems to be no change whatsover.

@willkuer

This comment has been minimized.

Contributor

willkuer commented Jan 15, 2018

Don't you thinks it's kind of interesting that you changed something as important as timing by a non-negligible change as dividing or multiplying some part of the calculation by two and there is no major change? Just assume you divide your FSB's or one of your RAM modules' clockrate by two...

@ssakash

This comment has been minimized.

Member

ssakash commented Feb 8, 2018

@willkuer

Same here, I was expecting more games to be impacted by this but most of the games don't seem to care about the change as long as the blanking interval duration is similar. ¯_(ツ)_/¯

@ssakash ssakash merged commit 785fe6b into master Feb 8, 2018

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ssakash ssakash deleted the analogcheck branch Feb 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment