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

VI: derive field timing from VI registers #2686

Merged
merged 4 commits into from
Sep 2, 2015

Conversation

booto
Copy link
Contributor

@booto booto commented Jul 2, 2015

Rather than hard coding field timing for PAL/NTSC/Progressive this should derive it from the values set in the VI registers.

This PR also adds an option to enable/disable the forcing of interlaced to progressive fields. The option is not exposed via the UI. It defaults to ON and when on forces interlaced video modes to emit full progressive fields. This restores the old force-progressive hack but makes it optional via INI edit.

PR created to trigger builds for testing.

Some problems:

  • % in title bar seems to be slightly off.
  • Interlaced signal gives the one-line-jitter problem (again)
  • save state version has been bumped, might need to be updated later on

@mimimi085181
Copy link
Contributor

Seems to fix this issue: https://code.google.com/p/dolphin-emu/issues/detail?id=7734 (Baten Kaitos Eternal Wings and the Lost Ocean Flickering in PAL version)

@JMC47
Copy link
Contributor

JMC47 commented Jul 2, 2015

Sonic Mega Collection: Blue Sphere has severe flickering in this; works in Master.

@JMC47
Copy link
Contributor

JMC47 commented Jul 2, 2015

FIXES NES PAL GAME AUDIO.

@JMC47
Copy link
Contributor

JMC47 commented Jul 2, 2015

Virtual XFB is bugged on Super Smash Bros. (VC PAL), Real XFB and no XFB works

ripclassic

@JMC47
Copy link
Contributor

JMC47 commented Jul 2, 2015

Fixes the Wind Waker PAL50 Hang.

@JMC47
Copy link
Contributor

JMC47 commented Jul 2, 2015

Metroid Prime 3 no longer shits itself in Real/VirtualXFB

@JMC47
Copy link
Contributor

JMC47 commented Jul 2, 2015

Metroid Prime 2/3 no longer suffer from the black bar glitch with XFB enabled.

@Linktothepast
Copy link
Contributor

Actually it seems that all the abnormal speed issues with virtual console games are fixed by this one with no need for progressive scan either in the snes and genesis games that needed it for normal speed. Neo geo games also work fine with no speed issues.
https://code.google.com/p/dolphin-emu/issues/detail?id=7039
https://code.google.com/p/dolphin-emu/issues/detail?id=6466
The above issues are totally fixed.

@shuffle2
Copy link
Contributor

shuffle2 commented Jul 3, 2015

nice. can NTSC_FIELD_RATE + etc be removed now?

@mimimi085181
Copy link
Contributor

Ok, i did some more tests with Baten Kaitos. First, the flickering goes away(on master) when using an AR code to force the game to play at in 60/30 instead of 50/25 Hz mode. Also the small black bar at the bottom goes away in 60/30 Hz mode. I kinda expected this, because the NTSC version does not have flickering.

The screen wobbles with virtual xfb with this pr. That is already known, but it randomly decides to flicker as well. When it flickers, it flickers above the screen in 50 Hz mode, and below the screen in 60 Hz mode. This can only be seen in windowed mode, see screenshots:
https://drive.google.com/file/d/0ByQgqzsPdirYZEhxNW55X09KUzA/view?usp=sharing
https://drive.google.com/file/d/0ByQgqzsPdirYYnFnVHRKNFFJUjQ/view?usp=sharing

Could the randomness be related to the game changing between 25/50 Hz modes?

Could anything be related to the game using still pictures. I mean, the game does something weird with the nintendo logo at startup. If the window is resized, the logo does not change accordingly and stays at the same x/y position in the window at the same size. On master, the fps is 0(vps 50) at that time, and the logo does not flicker like the rest does.

@booto booto force-pushed the field-timing branch 2 times, most recently from 465f5bf to 8e74842 Compare July 7, 2015 11:45
54000000UL,
};

static u64 s_ticks_last_line_start = 0;

This comment was marked as off-topic.

@shuffle2
Copy link
Contributor

shuffle2 commented Jul 7, 2015

Could the randomness be related to the game changing between 25/50 Hz modes?

If the game changes modes (or does other things which could produce random/ugly output for a small time period), they should disable VI during that period, which results in a black screen since nothing gets drawn out. I'm not sure if this works correctly on dolphin.

@@ -66,7 +66,7 @@ static Common::Event g_compressAndDumpStateSyncEvent;
static std::thread g_save_thread;

// Don't forget to increase this after doing changes on the savestate system
static const u32 STATE_VERSION = 44; // Last changed in PR 2464
static const u32 STATE_VERSION = 45; // Last changed in PR 2464

This comment was marked as off-topic.

@booto booto force-pushed the field-timing branch 2 times, most recently from 53bcfcf to 4ea30db Compare July 7, 2015 18:29
@booto
Copy link
Contributor Author

booto commented Jul 7, 2015

@shuffle2 Yes, I've removed NTSC_FIELD_RATE et al.

@MayImilae
Copy link
Contributor

Is this ready to go? It would be great in the progress report!

@JMC47
Copy link
Contributor

JMC47 commented Jul 26, 2015

I think it needs to have the progressive hack put into it... It has a screen shaking issue in interlaced mode.

@booto booto force-pushed the field-timing branch 2 times, most recently from 48658a9 to fb83ddc Compare July 27, 2015 12:38
@booto
Copy link
Contributor Author

booto commented Jul 28, 2015

Progressive hack added (see updated description in first comment).

Only issue I'm still aware of is the Baten Kaitos problem @mimimi085181 mentioned. Could we get some clear repro steps with settings/etc?

@magumagu
Copy link
Contributor

"Force Progressive Output" isn't a useful option to expose in the UI. I mean, it's useful in theory, but there isn't any point without actual interlacing support.

@phire
Copy link
Member

phire commented Jul 29, 2015

I have to agree with magumagu, the option doesn't even do what it says, the main force progressive hack stays enabled.

@booto
Copy link
Contributor Author

booto commented Jul 29, 2015

I'm wondering if progressive has actually ever worked properly? I'm assuming I've made a silly mistake somewhere, but here's what I've found:

In current master (567d0b2...)
I've enabled progressive mode, VI BeginField logs confirm it's in progressive. I'm on Windows, and added a GetTickCount() value to each of those lines. This is a timestamp from an arbitrary start point in milliseconds.

Here's a filtered extract of the log:

25:30:871 HW\VideoInterface.cpp:541 W[VI]: (VI->BeginField): Address: 002D92A0 | WPL 38 | STD 40 | ACV 464 | Field Progressive | WindowsTickCount: 14056688
25:30:895 HW\VideoInterface.cpp:541 W[VI]: (VI->BeginField): Address: 00243280 | WPL 38 | STD 40 | ACV 464 | Field Progressive | WindowsTickCount: 14056719
25:30:932 HW\VideoInterface.cpp:541 W[VI]: (VI->BeginField): Address: 00243280 | WPL 38 | STD 40 | ACV 464 | Field Progressive | WindowsTickCount: 14056750
25:30:968 HW\VideoInterface.cpp:541 W[VI]: (VI->BeginField): Address: 002D92A0 | WPL 38 | STD 40 | ACV 464 | Field Progressive | WindowsTickCount: 14056782
25:30:997 HW\VideoInterface.cpp:541 W[VI]: (VI->BeginField): Address: 00243280 | WPL 38 | STD 40 | ACV 464 | Field Progressive | WindowsTickCount: 14056813
25:31:028 HW\VideoInterface.cpp:541 W[VI]: (VI->BeginField): Address: 00243280 | WPL 38 | STD 40 | ACV 464 | Field Progressive | WindowsTickCount: 14056844

The milliseconds between frames: 31, 31, 32, 31, 31.

This corresponds to 1000/31 ~ 30-ish Hz. For Progressive (480p) we'd expect 60Hz...

Looking at the code,

Progressive is not interlaced so m_DisplayControlRegister.NIN is 1, so fields is 2 (??!).
See: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Core/HW/VideoInterface.cpp#L441

Progressive frames are emitted at line 1 of the scanout.
See: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Core/HW/VideoInterface.cpp#L560

The number of lines "virtually scanned out" is s_lineCount * fields.
See: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Core/HW/VideoInterface.cpp#L584

It looks like the second field is being counted through, but nothing being scanned out.

A similar log generated in this PR:

51:18:328 HW\VideoInterface.cpp:547 W[VI]: (VI->BeginField): Address: 002D92A0 | WPL 38 | STD 40 | EQ 12 | PRB 48 | ACV 464 | PSB 38 | Field Odd | WindowsTickCount 15604156
51:18:344 HW\VideoInterface.cpp:547 W[VI]: (VI->BeginField): Address: 00243280 | WPL 38 | STD 40 | EQ 12 | PRB 48 | ACV 464 | PSB 38 | Field Even | WindowsTickCount 15604171
51:18:360 HW\VideoInterface.cpp:547 W[VI]: (VI->BeginField): Address: 002D92A0 | WPL 38 | STD 40 | EQ 12 | PRB 48 | ACV 464 | PSB 38 | Field Odd | WindowsTickCount 15604187
51:18:377 HW\VideoInterface.cpp:547 W[VI]: (VI->BeginField): Address: 00243280 | WPL 38 | STD 40 | EQ 12 | PRB 48 | ACV 464 | PSB 38 | Field Even | WindowsTickCount 15604202
51:18:394 HW\VideoInterface.cpp:547 W[VI]: (VI->BeginField): Address: 002D92A0 | WPL 38 | STD 40 | EQ 12 | PRB 48 | ACV 464 | PSB 38 | Field Odd | WindowsTickCount 15604218
51:18:410 HW\VideoInterface.cpp:547 W[VI]: (VI->BeginField): Address: 00243280 | WPL 38 | STD 40 | EQ 12 | PRB 48 | ACV 464 | PSB 38 | Field Even | WindowsTickCount 15604234

The milliseconds between frames: 15, 16, 15, 16, 16.

This corresponds to 1000/16 ~ 60-ish Hz, in the ballpark of what's expected.

@JMC47
Copy link
Contributor

JMC47 commented Jul 29, 2015

Progressive Scan + XFB outputs only half the frames in master (and this PR.) Perhaps related?

Try playing F-Zero GX with it, it's full of sadness. Same with Wind Waker.

@JMC47
Copy link
Contributor

JMC47 commented Jul 29, 2015

Did a lot more testing and determined that the framedrop bug for Progressive Scan + XFB is fixed in single core within this Pull Request, but dualcore is broken still. @phire I think this is the only thing blocking virtual/hybrid XFB from moving toward default.

Testing wise, LGTM.

@JMC47
Copy link
Contributor

JMC47 commented Jul 30, 2015

SyncGPU + RealXFB + Progressive works as well, as per @phire's request to test it.

@booto
Copy link
Contributor Author

booto commented Jul 30, 2015

@magumagu @phire The last sync removed the option from the UI.

@JMC47
Copy link
Contributor

JMC47 commented Jul 30, 2015

I found out that the Dualcore + RealXFB problems were due to it maxing out my GPU. Damn. There are literally no more problems/regressions with this, full LGTM this time.

@JMC47
Copy link
Contributor

JMC47 commented Jul 30, 2015

As I say that, I found a regression. Sonic Mega Collection games, such as Sonic And Knuckles suffer from constant flickering.

@phire
Copy link
Member

phire commented Jul 30, 2015

Oh, one final think we need to solve before merging is the mystery of why FifoCI is rendering double frames.

I didn't think Fifoplayer touched VI at all.

@BhaaLseN
Copy link
Member

Does the Fifo-Log include all those fields? Maybe they were actually interlaced or something, which now works correctly?

@degasus
Copy link
Member

degasus commented Jul 30, 2015

fifo logs don't have the VI at all, only the XFB copy. That's why XFB is disabled on replaying fifo logs. So I wounder what's wrong here. I guess it's because of our (crappy) frame replay on avi dump: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/AVIDump.cpp#L231 We try to get a constant FPS there. Maybe this is wrong with the new VI code.

@phire
Copy link
Member

phire commented Jul 30, 2015

While FifoPlayer has a very slightly different idea about cycles per frame.

@JMC47
Copy link
Contributor

JMC47 commented Jul 30, 2015

Couple of things I noticed.

Virtual XFB on OpenGL still has stuff showing up below the bottom of the screen. D3D isn't doing it, so, I can't ponder a guess as to why.

Sonic Mega Collection is still flickering. That's not good. At least Master + PR NoXFB are the same (flickering), but for some reason there's a regression on Virtual and RealXFB causing them to flicker too.

@magumagu
Copy link
Contributor

magumagu@f93c87e contains a corrected implementation of the ForceProgressive option.

@JMC47
Copy link
Contributor

JMC47 commented Aug 4, 2015

Fixes Hydroventure (Fluidity PAL) in PAL50 mode.

@booto booto force-pushed the field-timing branch 2 times, most recently from df4d6dc to 187ab67 Compare August 10, 2015 06:37
@JMC47
Copy link
Contributor

JMC47 commented Aug 10, 2015

Virtual XFB is 100% broken in the latest version of the PR. Jitter bug is still there in RealXFB.

Edit: Virtual XFB is broken only with force progressive off.

@booto
Copy link
Contributor Author

booto commented Aug 12, 2015

Force-progressive being disabled while interlaced output is being emitted is expected to be broken at the moment. Earlier, it would force the effect of 'force progressive' to happen anyway. Now it actually tries to send the correct interlaced parameters to the code that scans out the image, but that code doesn't handle it properly (...yet). This is the effect of magumagu's patch mentioned above.

@JMC47
Copy link
Contributor

JMC47 commented Aug 12, 2015

Fair enough then.

@degasus
Copy link
Member

degasus commented Sep 1, 2015

LGTM

@JMC47
Copy link
Contributor

JMC47 commented Sep 2, 2015

LGTM. Sonic may need interlacing support, and this fixes a ton of other games.

@booto booto changed the title [WIP] VI: derive field timing from VI registers VI: derive field timing from VI registers Sep 2, 2015
phire added a commit that referenced this pull request Sep 2, 2015
VI: derive field timing from VI registers
@phire phire merged commit ecbb83f into dolphin-emu:master Sep 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet