Split chroma.py into a chroma package - #353
Conversation
Pure rename of chroma.py to vhsdecode/chroma/qam.py plus an __init__ that re-exports the externally used names, so importers and git blame are both undisturbed. Follow-up commits split the SECAM FM helpers and the shared utilities/orchestrators out of qam.py.
Moves the SECAM-specific code (method 1 restoration, blanking regeneration, line identification/parity flywheel, and the the ME-SECAM servo's under-carrier pair measurement verbatim into vhsdecode/chroma/secam.py. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Moves the format-agnostic pieces (chroma_to_u16, the pre-TBC
colour-under filter, and the process_chroma/decode_chroma per-field
orchestrators) verbatim into vhsdecode/chroma/common.py, leaving qam.py
with the burst measurement and heterodyne up-conversion pipeline. The
import graph is a simple DAG: common -> {qam, secam}. get_burst_area
stays in qam.py since the burst machinery there uses it. No behavior
change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eshaz
left a comment
eshaz
left a comment
There was a problem hiding this comment.
It would make more sense to split the process_chroma functions into their respective color systems. Since the color system is known in the Field classes, the function calls can be hard coded there without the extra conditionals. For example, if I am processing a SECAM field, then I'd call process_chroma_secam from chroma/secam.py and the same for QAM.
At some point, an inheritance pattern would make sense here to organize the shared functionality for all the different system types: PAL, NTSC, MESECAM, SECAM. This should probably be done in a future PR though.
There was a problem hiding this comment.
This file isn't needed since none of these functions are expected to be used outside of vhs-decode.
There was a problem hiding this comment.
It looks like there's still SECAM and QAM logic in this common file. Only the shared filtering and other utilities that are agnostic of the color system should live here.
2 participants
Description below. If you merge, don't squash it. The commits contain a file move and it would be nice to retain
git blamefor the originalchroma.pyauthors.Checklist
Description
This splits
chroma.pyinto a Python package (no behavior changes):vhsdecode/chroma/__init__.pyre-exports the previously-public names, so external imports (field.py, process.py, tests) are unchangedqam.pykeeps burst measurement, track-phase rotation, heterodyne up-conversion. Renamed from the originalchroma.py, keeping full git blame back to 2021.secam.pyhas SECAM VHS method 1 recording restoration: ×4 phase multiply, SECAM III bell regeneration, blanking regeneration, line identification / parity flywheel.common.pyhaschroma_to_u16, the pre-TBC colour-under filter, and the process_chroma/decode_chroma orchestratorsMotivation
chroma.pyhas grown to ~2,100 lines mixing three concerns: burst-measurement/heterodyne up-conversion pipeline used by PAL, NTSC, PAL-M, ME-SECAM, the SECAM method 1 FM restoration chain, and a handful of format-agnostic utilities plus the top-level per-field orchestrators. @eshaz suggested splitting it up in a #tape-decode discussion in the Domesday86 Discord.Additional Notes
qam.pystill has a couple functions used for ME-SECAM, which is afterall a heterodyne format, just FM instead of actual QAM.Testing
pytest --output-on-failure)