This fixes a bug where after saving and loading a custom preset on 480p input, the image would be shifted downwards because the input is sampled too early after vsync.
When we load a fixed output resolution on 480p, doPostPresetLoadSteps() first calls prepareSyncProcessor() which writes to SP_SDCS_VSSP_REG_L (and other registers in segment 5), then adjusts the registers by calling setCsVsStart and setCsVsStop. This controls the sync separator, and indirectly increases the number of scanlines we wait after vsync before sampling the input, which is is necessary because 480p video has a longer back porch after vsync than 480i video.
When we save the configuration as a custom preset, all values written by prepareSyncProcessor(), including those adjusted by setCsVsStop(), are saved to flash. The problem arises when we load this custom preset and call doPostPresetLoadSteps() again. This time, the function calls prepareSyncProcessor() and erases the changes to vertical sampling, but doesn't adjust the registers again by calling setCsVsStop() etc. This results in the input being sampled too soon and the output image shifting downwards.
As far as I can tell, it's actually unnecessary to call prepareSyncProcessor() when loading a custom preset, because in any saved preset the function has already been called, incorrect values replaced by calling setCsVsStop() etc., and all register values touched by prepareSyncProcessor() have been saved to the custom preset file. So to prevent prepareSyncProcessor() from writing incorrect register values when loading saved custom presets, don't call the function at all.
Fixes #401.
[ ] What's the relation between SP_SDCS_VSSP_REG_L/SP_SDCS_VSST_REG_L and IF_VB_SP/IF_VB_ST? How does the sync separator work? It's not really described in the Programming Guide PDF, and the Registers Definition doesn't supply context (and apparently spells vsync as vs. like versus).
[ ] Fix the PR and commit message once I figure out
[x] Verify that it is indeed unnecessary to call prepareSyncProcessor() when loading custom presets. If not, this PR is incorrect.
I double-checked that all writes either go through writeOneByte (on segment 5) or to registers located on segment 5, both below the maximum register address saved to preset files = 0x6F.
Hopefully there's no requirement that you overwrite loaded values and restore registers to an initial value, after loading a saved custom preset. I don't see any reason you'd need to, but I could be wrong.
[x] Merge #438 and rebase this branch on master
Unrelated: Can writeOneByte(0xF0, y) cause UReg::write() -> SegmentedSlave::setSeg() to falsely assume the segment is curSeg, and not bother writing the segment ID, when in fact the segment is y and writes are going to the wrong page?
This fixes a bug where after saving and loading a custom preset on 480p input, the image would be shifted downwards because the input is sampled too early after vsync.
When we load a fixed output resolution on 480p,
doPostPresetLoadSteps()
first callsprepareSyncProcessor()
which writes toSP_SDCS_VSSP_REG_L
(and other registers in segment 5), then adjusts the registers by callingsetCsVsStart
andsetCsVsStop
. This controls the sync separator, and indirectly increases the number of scanlines we wait after vsync before sampling the input, which is is necessary because 480p video has a longer back porch after vsync than 480i video.When we save the configuration as a custom preset, all values written by
prepareSyncProcessor()
, including those adjusted bysetCsVsStop()
, are saved to flash. The problem arises when we load this custom preset and calldoPostPresetLoadSteps()
again. This time, the function callsprepareSyncProcessor()
and erases the changes to vertical sampling, but doesn't adjust the registers again by callingsetCsVsStop()
etc. This results in the input being sampled too soon and the output image shifting downwards.As far as I can tell, it's actually unnecessary to call
prepareSyncProcessor()
when loading a custom preset, because in any saved preset the function has already been called, incorrect values replaced by callingsetCsVsStop()
etc., and all register values touched byprepareSyncProcessor()
have been saved to the custom preset file. So to preventprepareSyncProcessor()
from writing incorrect register values when loading saved custom presets, don't call the function at all.Fixes #401.
SP_SDCS_VSSP_REG_L/SP_SDCS_VSST_REG_L
andIF_VB_SP/IF_VB_ST
? How does the sync separator work? It's not really described in the Programming Guide PDF, and the Registers Definition doesn't supply context (and apparently spells vsync asvs.
like versus).prepareSyncProcessor()
when loading custom presets. If not, this PR is incorrect.writeOneByte
(on segment 5) or to registers located on segment 5, both below the maximum register address saved to preset files = 0x6F.Unrelated: Can
writeOneByte(0xF0, y)
causeUReg::write() -> SegmentedSlave::setSeg()
to falsely assume the segment iscurSeg
, and not bother writing the segment ID, when in fact the segment isy
and writes are going to the wrong page?