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

Aspect Ratio/VI Scaling FIx #2765

Merged
merged 2 commits into from Aug 1, 2015
Merged

Aspect Ratio/VI Scaling FIx #2765

merged 2 commits into from Aug 1, 2015

Conversation

mirrorbender
Copy link
Contributor

I've been working on this for the past few days. Basically the goal is to correctly display games by emulating the VI scaling and the change in aspect ratio caused by Analog TV sets. See issue 8751 for more info. There is also this thread about it. Any advice is appreciated.

@phire
Copy link
Member

phire commented Jul 21, 2015

You might want to take a look at #2686, which fixes many other issues with VI emulation, removing a lot of the hard coded constants and fixing field timings.

Maybe you should build this on top of that PR, or just keep in mind how the two are going to interact when merged.


int VIWidth = 640;
int VIHeight = 480;
bool VMode = false;

This comment was marked as off-topic.

This comment was marked as off-topic.

@phire
Copy link
Member

phire commented Jul 21, 2015

BTW, I was thinking we could probably detect if a GameCube game uses a 4:3ish or 16:9ish aspect ratio based on the projection matrix it sets up.

That way we could make auto option smarter and remove the need for the user to switch between Force 4:3 and Force 16:9

@mirrorbender
Copy link
Contributor Author

These changes should be completely compatible with #2686. It doesn't rely on any of the hard coded values that were used previously. "Guessing" the aspect ratios sounds like it is a little beyond my abilities at this point. I'll submit my improvements when I make them

@phire
Copy link
Member

phire commented Jul 21, 2015

Regarding the math.
Currently you have too many magic numbers. At the very least they need to be documented.
I think I see where 702.0f and 710.85f come from, but where do 768.0f, 648.0f, 1025.0f and 853.33f come from?
It's also not very nice to have the video backends aware of PAL vs NTSC.

I suspect the math could be cleaner if you calculated it as separate horizontal and vertical scaling, because VI only effects the horizontal scaling.

@mirrorbender
Copy link
Contributor Author

Where should I document the math? In comments?

You need to be aware of PAL vs NTSC in order to know the dimensions of the active frame and scale accordingly. Unless you just want me to do all of that in VideoInterface and just have that spit out a number to RenderBase. Let me know what you want me to do.

@phire
Copy link
Member

phire commented Jul 21, 2015

Do it in VideoInterface and spit one number out into RenderBase. If needed, RenderBase can tell VideoInterface if it want's a number for widescreen or not.

@@ -28,6 +28,8 @@
#include "Core/Movie.h"
#include "Core/FifoPlayer/FifoRecorder.h"

#include "Core/HW/VideoInterface.h" //For Aspect Ratio Correction/Scaling Emulation

This comment was marked as off-topic.

This comment was marked as off-topic.

@mirrorbender
Copy link
Contributor Author

Do you guys know if it's okay to use/change the g_aspect_wide variable in Core.h? It was useful for me in communicating widescreen information to VideoInterface.

@phire
Copy link
Member

phire commented Jul 21, 2015

No, changing it would mess up the auto aspect ratio option.

Instead, pass use16_9 into VideoInterface::GetAspectRatio as a parameter, like so:

Ratio = (WinWidth / WinHeight) / VideoInterface::GetAspectRatio(use16_9);

@mirrorbender
Copy link
Contributor Author

Ah okay, didn't think of that solution

@phire
Copy link
Member

phire commented Jul 21, 2015

I tested with Rogue Squadron , massive improvement in the cut-scenes, but the math seems to be off still.

Auto:
gswe64-4

Analog:
gswe64-5

Notice the planet still isn't quite round.

In game it's slightly worse, Compared to auto it's slightly narrower/taller, yet actual hardware has it letterboxed.

gswe64-1

@mirrorbender
Copy link
Contributor Author

It's always possible that my math might be wrong, but it's also possible the the game's programming is slightly off. I've had this discussion on the forums already, and the truth is that every game is potentially a little bit off. My math is based on the NTSC/PAL specifications, so if an individual game is a little off from the standard, then they will still be a little off here. I'm pretty sure my math is right, especially for standard NTSC

@bb010g
Copy link
Contributor

bb010g commented Jul 21, 2015

@phire commented on Jul 21, 2015, 2:37 PM PDT:

In game it's slightly worse, Compared to auto it's slightly narrower/taller, yet actual hardware has it letterboxed.

Dynamically switching aspect ratios in-game?


//For VI Scaling and Aspect Ratio Correction
float GetAspectRatio(bool);
bool GetVideoMode();

This comment was marked as off-topic.

This comment was marked as off-topic.

@phire
Copy link
Member

phire commented Jul 21, 2015

@bb010g Yeah, it manipulates the VI registers to implement letter-boxing.

@mirrorbender
Copy link
Contributor Author

If you guys could check my math and the reasoning behind it that would be great. I'm pretty damn sure it's right, but of course the real test is convincing others. If the math is right then it is right, and any remaining geometry errors are the fault of the game.

if (xfbAddr)
g_video_backend->Video_BeginField(xfbAddr, fbWidth, fbStride, fbHeight);


This comment was marked as off-topic.

@JMC47
Copy link
Contributor

JMC47 commented Jul 28, 2015

OpenGL and D3D, I tried XFB on/off. Any ideas?

@phire
Copy link
Member

phire commented Jul 28, 2015

progressive mode.

@JMC47
Copy link
Contributor

JMC47 commented Jul 28, 2015

@phire caught the issue, I had progressive scan enabled. Why would that cause this?

@@ -436,6 +436,56 @@ u32 GetXFBAddressBottom()
return m_XFBInfoBottom.FBB;
}

float GetAspectRatio(bool wide)
{
int height = (2 * m_VerticalTimingRegister.ACV);

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@JMC47
Copy link
Contributor

JMC47 commented Jul 29, 2015

Regardless, I wanted to test the widescreen in Wii games and see if they matched like GameCube games. It does, so, it's nice. Just fix the bug and I'm happy via testing.

{
m_target_height = 1;
}
if(m_target_width < 1)

This comment was marked as off-topic.

@mirrorbender
Copy link
Contributor Author

Would it be helpful if I gave some of you guys write access to my fork of dolphin? So you can make whatever little fixes you want without having to wait for me to see your comments and change them myself?

@bb010g
Copy link
Contributor

bb010g commented Jul 29, 2015

There's always pull requests on your repo.

@JMC47
Copy link
Contributor

JMC47 commented Jul 29, 2015

The comments and such are a normal part of the Pull Request process that you handle to get code quality up to the standards of the emulator.

@JMC47
Copy link
Contributor

JMC47 commented Jul 30, 2015

@lioncash did the code review. If you could amend the changes he pointed out, we could move toward squashing commits and getting this ready to merge soon.

@RisingFog
Copy link
Member

@dolphin-emu-bot rebuild

@phire phire changed the title [Work in progress] Aspect Ratio/VI Scaling FIx Aspect Ratio/VI Scaling FIx Aug 1, 2015
phire added a commit that referenced this pull request Aug 1, 2015
@phire phire merged commit fc4ba3a into dolphin-emu:master Aug 1, 2015
@M-a-r-k
Copy link

M-a-r-k commented Aug 1, 2015

The pixel aspect calculation in GetAspectRatio() is a bit off (in my opinion), in comparison to the industry-standard pixel aspect ratios. See http://www.lurkertech.com/lg/pixelaspect_sgi.html

Now my GameCube hardware knowledge is minimal, but I assume the GC output pixel clock is 13.5MHz (for 480i/576i). The industry-standard square-pixel sampling rates are 135/11 MHz for NTSC and 14.75 MHz for PAL.

Thus NTSC 13.5 MHz pixels have an aspect ratio of 10:11, and PAL 13.5MHz pixels have an aspect ratio of 59:54.

For PAL 4:3 you have:

    pixelAR = 768.0f / 702.0f;

That's 128:117, which is very close to 59:54; it's (768/767)*(59/54), about a 0.13% difference. That line could be changed to

    pixelAR = 14.75f / 13.5f;

Similarly for NTSC:

    pixelAR = 648.0f / 710.85f;

would become

    pixelAR = (135.0f / 11.0f) / 13.5f;

(or better just write as 10.0f / 11.0f)
The percentage difference between the two there is larger, about 0.274%

If it were me, I'd also have one if (wide) statement, like this (comments deleted for brevity):

if (m_DisplayControlRegister.FMT == 1)
    pixelAR = 14.75f / 13.5f;
else
    pixelAR = 10.0f / 11.0f;

if (wide)
    pixelAR  *= 4.0f / 3.0f;

@phire
Copy link
Member

phire commented Aug 1, 2015

The problem is, there is no standard defining pixel aspect ratios.

The page you reference derives pixel aspect ratio by comparing the BT.601 standard's 13.5mhz sampling rate to the "Industry standard square pixel" sampling rates of 12 + 27/99mhz and 14.75mhz the then goes on to admit nobody knows where those "Industry standard" sample rates come from. I've found at least two pages on the internet unsuccessfully trying to re-derive those sample rates.
They seem to be as arbitrary as anything else on the subject of ntsc/pal aspect ratios.

The numbers in this PR were derived from this page

My own efforts to derive a number from the standards when the SMPTE 170m-2004 standard was really quite vague about where the active line ended and the horizontal blanking period started.

Though I did manage to re-derive the 710.85px by 486px number from the BT.470-6 standard.

@M-a-r-k
Copy link

M-a-r-k commented Aug 1, 2015

It's true that no single/canonical square-pixel sampling rate can be derived from the various SD video standards, because the standards allow some leeway in the length of the active line.

The "industry-standard" figures may have been derived at/settled on arbitrarily (well not arbitrarily; see below), but if they are what everyone (hardware manufacturers) uses in practice, then I think it's best to follow them. Modern displays will be calibrated to that spec, and DVDs etc. will be authored assuming that. Then Dolphin's output should look the same as on a real GC shown on a monitor which is calibrated/adjusted to the industry-standard ratios.

Plus, 10:11 is a much "nicer" ratio than 4320:4739 :)

This obviously isn't directly relevant to Dolphin, but here's how (I think) 135/11 MHz was decided for the NTSC square-pixel sampling rate.

The requirements are:

  • you want the number of samples in the active line to be close to 648 (= 486 × 4/3).

From a hardware design perspective you also want:

  • a whole number of samples per entire line (i.e. including blanking/sync)
  • the sampling clock to be a "nice" rational multiple of the NTSC subcarrier frequency

NTSC subcarrier (Fsc) is 315/88 MHz.

There are Fsc×455/2 scanlines per second, i.e. the period of each scanline is
1/(Fsc×455/2) = 572/9 usec.

From the NTSC spec, the non-active line time is min 0.165H, max 0.18H. So the active line time is min 0.82H, max 0.835H. We want approximately 648 samples in the active line time. What sampling rate would achieve that?

0.82H = 11726/225 usec = 52.1155... usec
0.835H = 23881/450 usec = 53.0688... usec

For 0.82H active line, rate needed would be 648/(11726/225) MHz, giving 32400/41 = 790.244 samples per line approx
For 0.835H active line, rate needed would be 648/(23881/450) MHz, giving 129600/167 = 776.048 samples per line approx.

So the rate we decide to use will have between 777 and 790 samples per line (whole number).

Whole number of samples per line: n = Fsamp × 572/9000000
Sampling clock a rational multiple of Fsc: Fsamp = x/y × Fsc

So n = x/y × Fsc × 572/9000000 = x/y × 315/88 × 572/9 = x/y × 455/2
So x/y = 2n/455

We want to choose n so that x/y is "nice" (small numerator and denominator). Just plug in all possible values:
n = 777 ==> x/y = 222/65
n = 778 ==> x/y = 1556/455
n = 779 ==> x/y = 1558/455
n = 780 ==> x/y = 24/7 <-- WE HAVE A WINNER!
n = 781 ==> x/y = 1562/455
n = 782 ==> x/y = 1564/455
n = 783 ==> x/y = 1566/455
n = 784 ==> x/y = 224/65
n = 785 ==> x/y = 314/91
n = 786 ==> x/y = 1572/455
n = 787 ==> x/y = 1574/455
n = 788 ==> x/y = 1576/455
n = 789 ==> x/y = 1578/455
n = 790 ==> x/y = 316/91

The only value for n which gives a nice multiplier is 780. So use Fsc × 24/7 = (315/88) × (24/7) = 135/11 MHz for the square-pixel sampling rate.

@mirrorbender
Copy link
Contributor Author

So if I'm following what you are saying right, you are proposing we use use 10:11 as the standard pixel aspect ratio because it's the place within the margin of error for which the numbers are prettiest. That's certainly a valid approach. It is not any more correct than the current numbers, as they are both within the margin of error.

Essentially, you are jumping through the additional hoop of analog->digital conversion, as opposed to using the purely analog standard to determine the ideal PAR. Based on what I've read, the active line width in the ITU.470 spec is 63.55555... - 10.9 (plus or minus 0.2) microseconds. That would mean that the "standard" is 52.65555... and deviation from that is "error". Your proposed standard introduces a tiny bit of error (well within the allowed margin of error, to be fair) to account for realities of digital television design. Dolphin doesn't need to do that. To Dolphin, it's all just floating point math, it doesn't matter how pretty the numbers are. It's basically a question of this: are we trying to reproduce the display of an analog TV or are we trying to reproduce the display of a digital TV which is trying to reproduce the display of an analog TV?

Of course, this disagreement is over about 2 pixels worth of difference in width, so we also need to ask ourselves how much it actually matters.

@M-a-r-k
Copy link

M-a-r-k commented Aug 2, 2015

Well quite. But anyway...

I don't know whether the DVD or MPEG-2 specifications recommend/suggest a pixel aspect ratio, but 10:11 is used by at least Adobe Encore and Sony Vegas (based on quickly Googling). And DVD/MPEG-2 was quite prevalent when the GameCube was introduced. It just seems (to me at least) better to use the same ratio "everyone else" uses rather than something else. Perhaps cut scenes in (some) games would have been authored with software which uses that ratio???

Again this isn't related to Dolphin, but 135/11 MHz for square-pixel sampling is an even nicer multiple of the 4×Fsc sampling rate used by D-2 video (which is basically digitized NTSC composite video); 6/7 there. That may be another reason why 135/11 MHz was settled upon by equipment manufacturers.

@mirrorbender
Copy link
Contributor Author

I just did some comparisons between screenshots using the two different PAR's. The difference is basically imperceptible without measuring pixels, even when you are specifically looking for it. If you really want to change it, create your own pull request for it and I'll endorse it as being functionally the same while being much easier to read.

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