When saving Pass Through to a preset, no output appears until I select fixed resolution Pass Through

[nyanpasu64]

While testing my preset system, I've created a preset which activates Pass Through/Bypass for 480p inputs (with full-sized image), and upscales 15KHz inputs to 480p (unfortunately with horizontally shrunken image) using the full GBS-C processing pipeline. However, after a system reboot, attempting to load this 480p preset produces no output with a 480p input (supposed to activate bypass mode):

<reset>
Activity detected, input: Component

2345678
Format change: 3 <stable>
preferencesv2.txt opened
loading from preset slot B: /preset_ntsc_480p.B
ADC offset: R:44 G:43 B:42
clock gen reset: 81000000
ABHT: large diff
retry
Active FrameTime Lock enabled, adjusting external clock gen frequency

preset applied: bypass (custom) for EDTV 60Hz

ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
give up

Loading other presets/fixed resolutions, then the Bypass preset again, does not resolve this problem. Only clicking the Pass Through button fixes the preset until the next reboot. Even then, clicking "load preset" still produces odd output:

user command 3 at settings source 10, custom slot 66, status 21
preferencesv2.txt opened
loading from preset slot B: /preset_ntsc_480p.B
ADC offset: R:44 G:43 B:42
clock gen reset: 81000000
ABHT: large diff
retry
Active FrameTime Lock enabled, adjusting external clock gen frequency

preset applied: bypass (custom) for EDTV 60Hz

ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
retry
ABHT: large diff
give up

Nowhere does it say pass-through on, which appears when pressing "Pass Through" normally (printed by setOutModeHdBypass). Was saving Bypass into a preset never an intended mode of operation?

I'd say "maybe this bug occurs because loading presets only restores a register dump, and doesn't properly setup Bypass", but I think "restore a register dump" is not strictly true because saving and loading a fixed resolution as a preset shifts 480p sources down (#401).

Reproduced on master e61abf586467d945914e7226cfc101371a87317b.

[ramapcsx2]

Hm, maybe it helps if I explain what I remember about this..

ABHT: means automatic best htotal ;p

Passthrough mode is indeed supposed to be not a preset / does not have a register dump attached to it, at least as far as I remember. This isn't a must though. If there are problems with it trying to be universal no matter the register state, this can be changed.

You found that the "pass-through on" message is missing. I would look at the code path that normally leads to that message. The problem should be nearby.

[nyanpasu64]

(Note: less polished post.)

You found that the "pass-through on" message is missing. I would look at the code path that normally leads to that message. The problem should be nearby.

Ouch... "pass-through on" is printed in setOutModeHdBypass(), but this function is called in:

• Enable Pass Through:
  • case 'K': setOutModeHdBypass()
• After reboot or source change:

void runSyncWatcher() {
    ...
    boolean wantPassThroughMode = uopt->presetPreference == (OutputBypass=10);
    if (!wantPassThroughMode) {
        applyPresets(detectedVideoMode);
    } else {
        ...setOutModeHdBypass();
    }

Problem is, when you load a preset from the GUI, case '3': // load custom preset will unconditionally assign uopt->presetPreference = OutputCustomized; then applyPresets(rto->videoStandardInput); (the wrong function). And when you switch inputs or reboot the GBS-C, runSyncWatcher() will see that you loaded a custom preset rather than fixed bypass, and unconditionally applyPresets rather than setOutModeHdBypass.

How do we fix this? Probably each custom preset should save in a variable or chip register (I don't know how), what output resolution or bypass mode it contains. And every method of applying a custom preset (loading preset, applying preset on source change, and anything else calling applyPresets I missed) should check if the preset being loaded is passthrough, then apply setOutModeHdBypass instead.

Is it safe to change applyPresets to unconditionally check if the bypass flag is set, and call setOutModeHdBypass? Maybe. I don't know.

Thoughts on architecture

I'm scared of how stateful the code is, and how easy it is to perform duplicated work, or (even worse) forget to initialize or update variables.

My preference (the most effective approach I've tried for making large-scale complex state tractable) is for state transitions and responding to timers/inputs, to occur in a unified fashion, where you have a separation between "source of truth" and derived cached state, and a "state transaction" object tracks all changes made to "source of truth" (eg. with a bitflag) and recomputes the cached state changed. I'm not sure how that would apply in this case; perhaps source state includes input source, input resolution, the frame sync timer ticking, the currently loaded fixed/custom preset. And both output preset changes and input source changes would trigger the same code which evaluates the current input source and output mode, and pick the right output mode.

What's the advantage over code which performs the necessary actions directly when a single piece of input state changes? I think it's mostly organizational/human, since it's easier for me to understand state changes when all access to mutable state occurs through one class which logs what state was changed, and a single method checks which state was changed and recomputes all outputs dependent on it. The real advantage is to avoid duplicated output recomputation or error-prone manual state recomputation when multiple sources of truth are edited at once.

I think such a system would help tracking state which must be saved/loaded in sync with output presets, for example motion adaptive deinterlacing settings (if we alter them for blooming scanlines, need to revert them when disabling scanlines or enabling deinterlacing, and don't want to find more unused chip registers to hide state in), or "is preset set to bypass".

Note that I very much do not fully understand the code as is. Someone who wrote the code to this point and knows why things were designed this way, and/or has studied NTSC/PAL/progressive input modes, chip state, how to properly configure it, and when/how the GBS-C configures it, is very much more qualified to rearchitect the code's state management. (I lack a full "theory of the program" in the Naur sense)

Could applyPresets serve as a "function called when input or output mode changes" (the second-best thing to a centralized output recomputation system)? Currently, it's called whenever the input mode changes between 15 and 31 KHz, but not between 240p and 480i, and not when uopt->presetPreference == OutputBypass. Again I've probably missed some cases... holy fuck, 26 call sites?!

[ramapcsx2]

Hehe, well, that's certainly a lofty goal, to redesign the architecture in that way :p It comes with many little problems, as you already noticed, and probably needs a full rewrite of the entire thing in the end.

A rewrite isn't a full remedy guarantee though, as many states are dynamic and have alterations applied.. It will be tricky to even try and unify it all, while not doing extra work that will hinder performance. Don't forget that some alterations need to happen in real time, and it's not feasible to maybe download the entire register state for a checkup.

I would try to work with what's there, solve the new problems each at a time. Check whether the bypass mode has a good indicator in the TiVo registers (or add a flag in unused space), and you can use that to help decide what should happen in applyPresets() :)