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

add support for progressive + pal60 video mode forcing (for Launch Disc) #378

Merged
merged 3 commits into from
Dec 25, 2024

Conversation

pyorot
Copy link
Contributor

@pyorot pyorot commented Nov 17, 2024

as discussed in #376; this is my sketch for the solution:

  1. determine render mode from system settings (CONF_GetProgressiveScan, VIDEO_HaveComponentCable and CONF_GetEuRGB60).
  2. set render mode register within the VIDEO_Configure wrapper function, determined from render mode.
  3. remove gamecube-disc-launch SYS_SetVideoMode call cos undocumented and not used in non-gc-disc paths (note: i'm unsure if this function supports the value 5 for PAL60 and don't want it to undo the register setting in step 2).
  4. add video mode setting to title-launch path (i didn't spot earlier it wasn't being called because the called function has "Title" in its name so i assumed it would be lol)

edit: 3 needs to stay and 4 doesn't fix issue #376... in fact, it rebreaks it after having been fixed by a different change (#379) lol. 1 and 2 seem like they would make disc loading more robust and correct (e.g. starting pal discs in progressive) tho so i've left them up here

this still doesn't support stuff like mpal and so isn't totally libogc-compliant but that's why we have the libogc dev on-hand to sort it out fully :3

not tested, not compiled, not checked by a language server even (i wasn't expecting to be doing any c++ :p)

@pyorot
Copy link
Contributor Author

pyorot commented Nov 17, 2024

i may be interested in compiling this and running it on my boot2-hacked wii at some point, so any tips for setting up libogc and everything would be appreciated

@DacoTaco
Copy link
Owner

did you try this change? did this fix your issue?

sorry i didn't get back to you sooner, my life is a bit busy atm

@pyorot
Copy link
Contributor Author

pyorot commented Nov 17, 2024

no worries. i haven't tested or compiled the code yet, and may give it a shot sometime soon, but wanted to open it up for comments in the meantime. i'd have to get the dev setup working and that kind of thing

i think my issue is caused more by the old code being functionally handicapped rather than wrong. it always sets interlaced for cross-region so even routes that do force video, like wii disc loads, should theoretically do that – e.g. ntsc-u disc in a pal wii will always start in 480i. as a european, i only have my cross-region "backups" in wad form to test unfort :)

@DacoTaco
Copy link
Owner

i would suggest to first test this change then, to compile you need devkitPRO's devkitPPC and libogc and that should be it (its all under then wii-dev package collection).
i can already see a few issues that i remember were critical, but i wont be doing any code review until its confirmed working for your case tbh

@DacoTaco DacoTaco marked this pull request as draft November 18, 2024 05:35
@pyorot
Copy link
Contributor Author

pyorot commented Nov 18, 2024

i would suggest to first test this change then, to compile you need devkitPRO's devkitPPC and libogc and that should be it (its all under then wii-dev package collection). i can already see a few issues that i remember were critical, but i wont be doing any code review until its confirmed working for your case tbh

fair, it was at least very easy to set up the toolchain so thanks for enabling that

i am testing rn, current patch doesn’t work and i’ve traced the bug to the title region detection logic. SetVideoModeForTitle isn’t being called at all. there’s a very similar bug in usbloader gx it seems, where “detect disc” starts 480i but “force ntsc” (NOT “force ntsc 480p”) starts 480p despite the code setting 480i lol. to be continued…

@pyorot
Copy link
Contributor Author

pyorot commented Nov 18, 2024

i got stuck tryna figure out the mystery in usbloader gx that made Disc Default boot 480i yet both Force 480i and Force 480p boot 480p (lol), so i decided to isolate out the patches that fixed the blocking issue of broken region detection logic, and accidentally fixed the whole thing :p. new patchset up at #379

i'm gonna rebase the patches here onto the ones there and then leave this as a draft for when someone who has cross-region discs can fix up the broken stuff there

edit: calling SetVideoModeForTitle after ShutdownVideo in the title-launch path actually switches it from 480p to 480i lolwat. i'll delete that patch for now...

@pyorot pyorot changed the title add support for progressive + pal60 video mode forcing; force video mode in title launches add support for progressive + pal60 video mode forcing (for Launch Disc) Nov 18, 2024
@DacoTaco
Copy link
Owner

is this still applicable? this will need some rebasing though :)

@pyorot
Copy link
Contributor Author

pyorot commented Nov 21, 2024

is this still applicable? this will need some rebasing though :)

the first commit by itself is just tidiness/correctness i guess, always setting that video mode address to the pending viTVMode Format (0–5) rather than just 0 or 1. the second i feel like should fix cross-region discs always booting interlaced but i've not verified that actually happens. i do know someone i could ask to check it tho

how does it look to you just statically?

@DacoTaco
Copy link
Owner

is this still applicable? this will need some rebasing though :)

the first commit by itself is just tidiness/correctness i guess, always setting that video mode address to the pending viTVMode Format (0–5) rather than just 0 or 1. the second i feel like should fix cross-region discs always booting interlaced but i've not verified that actually happens. i do know someone i could ask to check it tho

how does it look to you just statically?

much better now with the rebase, the PR now contains a single change which is a lot easier to process. it looks ok, but does it actually work? are you able to test it for channels and discs?

@pyorot
Copy link
Contributor Author

pyorot commented Nov 25, 2024

much better now with the rebase, the PR now contains a single change which is a lot easier to process.

yeah it’s a funny thing with hacking, you have no idea what will work so you pile up 6 patches in one draft pr then turns out u only need some of them. i at least try to do one commit per change so one can click thru the commits. then i cherry-pick them out at the end

it looks ok, but does it actually work? are you able to test it for channels and discs?

there’ve been developments with wii swiss booter last few days so i haven’t yet reached out to a guy i know maintaining guides for these things. i don’t have any cross-region discs but i was thinking of sending him the latest priiloader dev build to try booting ntsc-j gamecube SMS on a pal wii, which should answer the question of if this pr is needed

with channels, all that code is not even run cos setting the video mode (whether with this patch or not) seems to force interlaced for reasons i can’t fathom. so ig it’s just about if it helps with discs

btw, is there a way to test priiloader w/o installing it? i’ve been enjoying doing “make run” with wiiload lately heh

@DacoTaco
Copy link
Owner

much better now with the rebase, the PR now contains a single change which is a lot easier to process.

yeah it’s a funny thing with hacking, you have no idea what will work so you pile up 6 patches in one draft pr then turns out u only need some of them. i at least try to do one commit per change so one can click thru the commits. then i cherry-pick them out at the end

it looks ok, but does it actually work? are you able to test it for channels and discs?

there’ve been developments with wii swiss booter last few days so i haven’t yet reached out to a guy i know maintaining guides for these things. i don’t have any cross-region discs but i was thinking of sending him the latest priiloader dev build to try booting ntsc-j gamecube SMS on a pal wii, which should answer the question of if this pr is needed

with channels, all that code is not even run cos setting the video mode (whether with this patch or not) seems to force interlaced for reasons i can’t fathom. so ig it’s just about if it helps with discs

btw, is there a way to test priiloader w/o installing it? i’ve been enjoying doing “make run” with wiiload lately heh

i would send it for testing ye. im looking to either fixup & merge this PR or close it because it doesnt bring anything to the table.

as for testing, sure. make run should run priiloader just fine. keep in mind that if you have autoboot settings it will run those as well so make sure to disable those when testing!
when run from HBC some access rights errors might pop up (like title names not being read), but overal it should run fine

@pyorot
Copy link
Contributor Author

pyorot commented Nov 26, 2024

nice one, i messaged him to drop by if he gets a sec to test so we’ll see :)

@pyorot
Copy link
Contributor Author

pyorot commented Nov 26, 2024

keep in mind that if you have autoboot settings it will run those as well so make sure to disable those when testing!

BRO i have asked so many stupid questions this week i’m sorry looool it was 100% this

@DacoTaco DacoTaco force-pushed the master branch 2 times, most recently from 33953e8 to 2de7903 Compare December 23, 2024 15:51
Copy link
Owner

@DacoTaco DacoTaco left a comment

Choose a reason for hiding this comment

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

minor change before merge :)

@DacoTaco DacoTaco changed the base branch from master to release December 24, 2024 09:40
@DacoTaco DacoTaco changed the base branch from release to master December 24, 2024 09:58
@DacoTaco
Copy link
Owner

to be merged in main -> next update is 0.11 because previous patch related to this was merged in main

@pyorot
Copy link
Contributor Author

pyorot commented Dec 24, 2024

quick summary of where we are with video modes:

for cross-region disc, video is always re-configured by priiloader
for cross-region title, video is always shut down (so the title can reinit it)

re-configuring ntsc-u vc sm64 to 480p causes it to switch from 480p to 480i lol, hence the above

the re-configuring code detects the software’s region as NTSC(-U) / NTSC-J / PAL, and accordingly sets video mode to NTSC or PAL, with extra conditions:

  • if settings progressive and component cable detected: 480p
  • (PAL only:) else if settings PAL60: 480i
  • else: 480i (if NTSC) or 576i (PAL)

this stuff is quite confusing amid all the CONF_ VIDEO_ SYS_ enums and weirdly expansive array of viTVmodes, so it’d be good to get a reference documentation written somewhere for what the correct flowchart is cos i imagine most programs will want to do the same thing

this looks good to me except i’m unsure when PALM would be set and don't understand why the enum says 528 for PAL rather than 576. but it’s a start 👍

@pyorot pyorot marked this pull request as ready for review December 24, 2024 16:18
@DacoTaco DacoTaco merged commit a9de725 into DacoTaco:master Dec 25, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants