viTVMode access macros + bugfixes

pyorot commented 1 week ago

A fix related to libogc video_types.h. Thruout the code, the whole of GXRModeObj.viTVMode was being erroneously compared to VI_NTSC, which is a VI_TVMODE_FMT rather than a full VI_TVMode. This patch adds two macros to invert the VI_TVMODE(fmt, mode) transformation and recover the FMT and the MODE (this should be upstreamed to libogc), and then applies the macros thruout the codebase. The changes are pretty much:

rmode->viTVMode == VI_NTSC → VI_TVMODE_FMT(rmode->viTVMode) == VI_NTSC (bugfix)
rmode->viTVMode & VI_NON_INTERLACE → VI_TVMODE_MODE(rmode->viTVMode) == VI_NON_INTERLACE (code readability; old code relied on hack that VI_NON_INTERLACE was the only enum out of three that was odd).

Consequently, VideoRegionMatches now works, so mismatched regions will actually call ShutdownVideo() in the title loading codepath, which fixes #376.

(finally)

The second commit in this patchset just fixes a compiler warning by replacing a switch(bool) with an if-else.

Both commits tested by loading NTSC-U VC Super Mario 64 on a PAL Wii in 480p. However, there are a couple of bugs to look out for:

Now, the video takes a few more seconds to initialise and misses more of the intro of SM64.
The names of the titles are all still showing up as ????????, which I think is a regression on the master branch.

My old attempt at fixing #376 is at #378, but it'll probably remain a draft since I don't have the means to test the other changes.

~~Currently it still boots NTSC-U SM64 at 480i but I believe the mistake is that I replaced the ShutdownVideo() call with SetVideoModeForTitle rather than just adding the latter~~ (edit: nvm lol, SetVideoModeForTitle actually switches it from 480p to 480i 🤦‍♂️).
The code indicates the rather alarming bug that cross-region discs will get forced into interlaced tho, which is an obstacle to replacing GeckoOS with Priiloader for general-purpose speedrunning (I'm doing all this to streamline recommended setups).
It also just seems handicapped in a few other ways, like say that the memory edit to set VI mode is videoMode > SYS_VIDEO_NTSC ? 0x00000001 : 0x00000000; rather than VI_TVMODE_FMT(rmode->viTVMode).

pyorot commented 1 week ago

^ actually sorry yeah i wanted to submit this readability patch as well, also tested in the same way as the others

DacoTaco commented 1 week ago

so this is tested? are you getting 480p now in SM64? im getting confused because of all the PR's and comments tbh lol.

also, the ?? name is, again, because priiloader is loaded without the right nand access. if you install it, and hold reset, it should show the title names. its not a regression and is pure because of access rights.

pyorot commented 1 week ago

so this is tested? are you getting 480p now in SM64?

yes, thanks for the patience

im getting confused because of all the PR's and comments tbh lol.

each commit is a change that'll either fix something or be turned into a comment justifying something that looks wrong in the code, so that it doesn't get inadvertently changed later. this pr has finished changes and the other one has stuff that needs testing or further explanation, yet i don't have the equipment or knowledge myself. for example, i asked for more info on SYS_SET_VIDEO in one of the review threads

also, the ?? name is, again, because priiloader is loaded without the right nand access. if you install it, and hold reset, it should show the title names. its not a regression and is pure because of access rights.

in both cases, the user should be told this (i patched ios 36 cos i was told to for example). but also, i checked just now and am getting ??s on installed priiloader no matter what i do, so there's something else missing i think

DacoTaco commented 1 week ago

in both cases, the user should be told this (i patched ios 36 cos i was told to for example). but also, i checked just now and am getting ??s on installed priiloader no matter what i do, so there's something else missing i think but priiloader doesn't use IOS36 unless its the installer and it was started without the flag for HBC to not reload IOS (aka, you were missing the meta.xml).

the installed version should work, unless the content is moved to SD. i'd suggest looking into the priiloader log to see why it failed to read the title name

pyorot commented 6 days ago

but priiloader doesn't use IOS36 unless its the installer and it was started without the flag for HBC to not reload IOS (aka, you were missing the meta.xml).

yeah ik, one could argue that that's a skill issue if the user deletes meta.xml haha, but i'd recommend adding a warning to the installer

the installed version should work, unless the content is moved to SD. i'd suggest looking into the priiloader log to see why it failed to read the title name

where is this log?

DacoTaco commented 6 days ago

the installed version should work, unless the content is moved to SD. i'd suggest looking into the priiloader log to see why it failed to read the title name

where is this log?

if you enable it in the settings, it logs to the primary external device (SD or USB)

pyorot commented 6 days ago

the installed version should work, unless the content is moved to SD. i'd suggest looking into the priiloader log to see why it failed to read the title name

where is this log?

if you enable it in the settings, it logs to the primary external device (SD or USB)

oh lol, i thought i needed a usb gecko and was print-debugging with PRINT_FORMAT 👌. i'll get back to this ?? issue later

pyorot commented 6 days ago

all tested (same way) and ready to go. prii.log isn't appearing on my usb drive unfort. my sd broke today while working on this, bit of plastic flew off and the wii ain't havin it anymore lol

edit: got my sd card working kinda and priiloader still isn't logging to either, with one plugged in at a time and both usb slots tested

DacoTaco commented 6 days ago

all tested (same way) and ready to go. prii.log isn't appearing on my usb drive unfort. my sd broke today while working on this, bit of plastic flew off and the wii ain't havin it anymore lol

edit: got my sd card working kinda and priiloader still isn't logging to either, with one plugged in at a time and both usb slots tested

is the setting enabled that dumps the debug log to file?

pyorot commented 5 days ago

is the setting enabled that dumps the debug log to file?

yeah, “dump gecko output” or similarly named

DacoTaco commented 4 days ago

i have setup my development environments (serial connection, obs, devkitPPC/libogc) on my new system and will be testing some stuff tomorrow :)

DacoTaco / priiloader

viTVMode access macros + bugfixes #379