feat(audio): add --ntsc_audio_rate for NTSC-locked PCM output#1046
Merged
Conversation
Add an opt-in --ntsc_audio_rate flag that outputs analog audio locked to NTSC line timing (2.8 samples/line = 1470 samples/frame, ~44055.944 Hz) instead of the default 44100 Hz, which drifts at 1471.47 samples/frame against NTSC video. This reuses the existing negative-freq convention in downscale_audio (a negative value is a multiple of the horizontal line frequency). The multiple 2.8 is fractional, so two plumbing fixes were needed: - core.py: stop truncating analog_audio with int() for negative values, so the fractional HSYNC multiple survives. - core.py: resolve the negative rate to its real nominal value in build_json so metadata reports the true sample rate (44055.944...) rather than the -2.8 sentinel. PAL is already frame-locked at 44100 Hz (1764 samples/frame), so the flag is NTSC-only and warns + no-ops on PAL. Default behavior is unchanged for backward compatibility. Add tests/test_audio_rate.py covering the negative-freq mapping, and document the new flag in docs/user-guide/command.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Full disclosure: Claude did all the work; I reviewed code changes, they looked minimal/safe. We (Claude and me!) did manual testing to confirm that the .pcm output file size changes as expected when this flag is passed.
Add an opt-in --ntsc_audio_rate flag that outputs analog audio locked to NTSC line timing (2.8 samples/line = 1470 samples/frame, ~44055.944 Hz) instead of the default 44100 Hz, which drifts at 1471.47 samples/frame against NTSC video.
This reuses the existing negative-freq convention in downscale_audio (a negative value is a multiple of the horizontal line frequency). The multiple 2.8 is fractional, so two plumbing fixes were needed:
PAL is already frame-locked at 44100 Hz (1764 samples/frame), so the flag is NTSC-only and warns + no-ops on PAL. Default behavior is unchanged for backward compatibility.
Add tests/test_audio_rate.py covering the negative-freq mapping, and document the new flag in docs/user-guide/command.md.
Checklist
Description
Adds an opt-in
--ntsc_audio_rateflag that outputs analog audio locked to NTSCline timing — exactly 2.8 samples/line (1470 samples/frame, 735/field), i.e.
~44055.944 Hz — instead of the default 44100 Hz. Off by default; existing
behavior is unchanged.
Motivation
Primary motivation is to improve disc audio stacking.
The default 44100 Hz PCM rate does not divide evenly into NTSC frame timing:
44100 / (30000/1001 fps) ≈ 1471.47 samples/frame, so the audio sample grid
slowly drifts relative to the video. 44055.944 Hz = 2.8 × the NTSC horizontal
line frequency (4.5 MHz / 286 = 15734.2657 Hz), giving exactly 1470 samples per
frame and perfect, drift-free A/V alignment. PAL already divides evenly
(44100 / 25 fps = 1764 samples/frame), so it needs no change.
Related Issues
N/A
Changes Made
--ntsc_audio_rateCLI flag (default off, backward compatible). NTSC-only;warns and no-ops on PAL.
freqconvention indownscale_audio(a negativevalue is a multiple of the horizontal line frequency) by mapping the flag to
freq = -2.8.core.py: preserve fractional negative rates (no longerint()-truncateanalog_audio), so-2.8survives end-to-end.core.pybuild_json: report the resolved nominal sample rate(
44055.944055944055) in the.tbc.jsoninstead of the-2.8sentinel.tests/test_audio_rate.pyand document the flag indocs/user-guide/command.md.Testing
pytest) — 5 passed.-s 2000 -l 30). Default →44144 stereo samples (1471.47/frame);
--ntsc_audio_rate→ exactly 44100stereo samples (1470.000/frame). JSON
sampleRatereported44100vs44055.944055944055.freq→ samples-per-line mapping(
-2.8→ 1470/frame,-2.8→ 44055.944 Hz, default44100→ 1471/frame).Screenshots (if applicable)
N/A — no UI changes.
Additional Notes
.tbc.jsonsampleRatecan now be a non-integer float when this flag isused. Downstream ld-decode-tools should accept a float
sampleRate(handledseparately).