Skip to content

Commit

Permalink
Custom frame rate: improve message, respect also for NTSC-progressive
Browse files Browse the repository at this point in the history 
When a custom PAL/NTSC frame rate is used, PCSX2 respects it for PAL and NTSC,
but disrespected it for NTSC-progressive mode (used hardcoded 30/60). Fixed.

Also, when a custom rate was used, the console message displayed half the rate
which was configured. Now print the actual rate.
  • Loading branch information
@avih
avih committed Sep 16, 2015
1 parent ca5e9d8 commit a6737b8
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions pcsx2/Counters.cpp
View file
Expand Up @@ -173,6 +173,7 @@ static u64 m_iStart=0;
struct vSyncTimingInfo
{
Fixed100 Framerate; // frames per second (8 bit fixed)
GS_RegionMode RegionMode; // used to detect change (interlaced/progressive)
u32 Render; // time from vblank end to vblank start (cycles)
u32 Blank; // time from vblank start to vblank end (cycles)

Expand Down Expand Up @@ -278,18 +279,19 @@ u32 UpdateVSyncRate()
else if ( gsRegionMode == Region_NTSC_PROGRESSIVE )
{
isCustom = (EmuConfig.GS.FramerateNTSC != 59.94);
framerate = 30; // Cheating here to avoid a complex change to the below "vSyncInfo.Framerate != framerate" branch
framerate = EmuConfig.GS.FramerateNTSC / 2;
scanlines = SCANLINES_TOTAL_NTSC;
}

if( vSyncInfo.Framerate != framerate )
if (vSyncInfo.Framerate != framerate || vSyncInfo.RegionMode != gsRegionMode)
{
vSyncInfo.RegionMode = gsRegionMode;
vSyncInfoCalc( &vSyncInfo, framerate, scanlines );
Console.WriteLn( Color_Green, "(UpdateVSyncRate) Mode Changed to %s.", ( gsRegionMode == Region_PAL ) ? "PAL" :
( gsRegionMode == Region_NTSC ) ? "NTSC" : "Progressive Scan" );
( gsRegionMode == Region_NTSC ) ? "NTSC" : "NTSC Progressive Scan" );

if( isCustom )
Console.Indent().WriteLn( Color_StrongGreen, "... with user configured refresh rate: %.02f Hz", framerate.ToFloat() );
Console.Indent().WriteLn( Color_StrongGreen, "... with user configured refresh rate: %.02f Hz", 2 * framerate.ToFloat() );

hsyncCounter.CycleT = vSyncInfo.hRender; // Amount of cycles before the counter will be updated
vsyncCounter.CycleT = vSyncInfo.Render; // Amount of cycles before the counter will be updated
Expand Down

10 comments on commit a6737b8

@avih
Copy link
Contributor Author

@avih avih commented on a6737b8 Sep 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add to the commit message, but this also fixes a (minor) bug where it used 60fps instead of 59.94 target frame rate for NTSC-progressive.

Now it uses 59.94 by default also for NTSC Progressive, and also respects the custom NTSC rate in NTSC Progressive mode.

@bositman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm i remember a discussion some years back ( :P ) that on progressive mode, it was indeed 60 FPS and not 59.94? Not sure, maybe @ramapcsx2 knows more

@avih
Copy link
Contributor Author

@avih avih commented on a6737b8 Sep 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabest11 could you please review this patch? There could be nuances which I've missed, specifically, is there a reason for PCSX2 in NTSC Progressive mode to use 30/60 fps (before this patch) instead of 29.97/59.94 which the patch does? are the number of added scanlines affected by this 30 -> 29.97 change?

@ramapcsx2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only faintly remember there was some kind of problem with using 59.94.
It might have been fixed by now though.
Best thing to do: Have someone run Valkyrie Profile 2 in progressive scan mode while watching for glitches in the intro movies :)

@gabest11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what this does, not gsdx code.

@avih
Copy link
Contributor Author

@avih avih commented on a6737b8 Sep 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best thing to do: Have someone run Valkyrie Profile 2 in progressive scan mode while watching for glitches in the intro movies :)

OK, so you're our VP2 and nails polish guy (I don't have either...), care to run it for few minutes, report if everything is normal or if something weird happens?

Other than the console messages diff, the only difference with this patch is that NTSC progressive was hardcoded to 60, and now it's the "normal" NTSC rate (defaults to 59.94, but if you configure the NTSC rate to 60 and use an NTSC progressive mode, then it's identical to how it was before).

@avih
Copy link
Contributor Author

@avih avih commented on a6737b8 Sep 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabest11 I meant if you know anything about PS2 running in 60fps in NTSC progressive mode but 59.94 in other modes. My patch basically assumes that 60 was an error, and changes NTSC progressive to 59.94, and I'm wondering if I shouldn't have done that.

@gabest11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, 30/60 fps is certainly not for CRT televisions, on which PS2 runs. I mean, it is very unlikely that NTSC CRTs would run at anything other than 59.94 Hz.

@avih
Copy link
Contributor Author

@avih avih commented on a6737b8 Sep 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's what I think too. Thanks.

I think @ramapcsx2 says that with this new 59.94 rate for progressive, VP2 intro video has more audio issues than when it's 60, it could also be timing issues elsewhere, and I also think that progressive should not be 60 flat.

@ramapcsx2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, we have timing problems that all affect this. For this patch we can say it's good though :)
(Oh and NTSC TVs will happily accept anything from 59.7 to 60.3 or so :) )

Please sign in to comment.