Skip to content

Refactor/s1 audiosync determinism#66

Merged
natashaannn merged 4 commits into
mainfrom
refactor/s1-audiosync-determinism
May 9, 2026
Merged

Refactor/s1 audiosync determinism#66
natashaannn merged 4 commits into
mainfrom
refactor/s1-audiosync-determinism

Conversation

@natashaannn
Copy link
Copy Markdown
Member

@natashaannn natashaannn commented May 9, 2026 •
edited
Loading

Summary

  • Implemented deterministic FFT peak selection in AudioSyncer to fix non-deterministic sync behavior when multiple correlation peaks have similar values
  • Added earliest-peak tie-break logic that prefers the first peak when multiple peaks are within 0.5 SNR threshold
  • Converted lag from floating-point seconds to frame-exact integer offsets at 30fps to eliminate precision drift
  • Updated validatePeak() to work correctly with new frame-based lag calculation
  • Added comprehensive unit tests covering tie-break scenarios, frame rounding, and determinism verification

How to review

  • scripts/sync/AudioSyncer.js — core implementation changes:
    • findBestLag(): new deterministic peak selection with candidate collection and earliest tie-break
    • validatePeak(): updated to handle frame-based lag calculations
    • Lag normalization: converts sample offsets to nearest frame at 30fps standard rate
  • scripts/tests/AudioSyncer.test.js — unit tests for all new functionality including edge cases
  • scripts/tests/AudioSyncer.determinism.test.js — dedicated determinism verification tests
  • docs/REFACTOR_ISSUE_INVENTORY.md — updated to mark issues Add hardware detection abstraction #22 and Add typed FFmpeg command builder #23 complete

Test plan

  • npm test passes (all Jest suites) ✅
  • Manual sync verification with real video/audio files ✅
    • Confirmed identical lag values across multiple runs
    • Verified frame-exact timing (no floating-point drift)
    • Deterministic peak selection working with real correlation data

Manual verification completed:

  • Sync produces identical lag values across multiple runs ✅
  • Output timing is frame-exact (no floating-point drift) ✅
  • Deterministic peak selection works with real correlation data ✅

Issues

Closes #12

natashaannn and others added 4 commits May 9, 2026 21:42
@natashaannn
refactor(sync): implement deterministic FFT peak selection and frame-…
f9fbf1a 
…exact lag

Fix non-deterministic FFT peak selection by implementing earliest-peak tie-break
when multiple peaks are within 0.5 SNR threshold. Convert lag from floating-
point seconds to integer frame offsets for reproducible sync results.

Changes:
- findBestLag: collect all near-maximum candidates and select earliest peak
- validatePeak: updated to work with frame-based lag calculation
- Lag normalization: convert to nearest frame at 30fps standard rate
- Added comprehensive unit tests for tie-break and frame rounding logic
- Added determinism verification tests

AC: #1, #2
@natashaannn
docs(inventory): mark issues #22 and #23 complete
6571fa7 
Both deterministic FFT peak selection and frame-exact lag offsets have been
implemented in the same commit, addressing both reproducibility and
integer storage requirements.
@natashaannn
fix(sync): address review findings from refactor/s1-audiosync-determi…
6ebde20 
…nism

- Fix validatePeak test to use deterministic correlation array instead of Math.random()
- Extract frameRate to module-level constant SYNC_FRAME_RATE
- Extract magic numbers as named constants PEAK_NEARNESS_THRESHOLD and RELIABILITY_SNR_THRESHOLD
- Remove stale lenA parameter from determinism test
- Add test case for near-equal peak appearing before global maximum

All tests now pass and provide better protection against regressions.
@natashaannn @claude
test(sync): fix validatePeak and frame-rounding test edge cases
6701b49 
- Move peak to index 67 in happy-path validatePeak test; lagSeconds=0.033
  at sampleRate=8000 round-trips to that index, not 50 as the comment claimed
- Expand frame-rounding correlation array to 1000 elements so samples=267
  and samples=-150 (idx=850) are in bounds; previously the bounds guard
  silently skipped placing the peak, leaving all-zeros and returning 0 frames
- Fix misleading comment on the mismatched-lag test to clarify it is an
  intentional negative case (lag ≠ peak → isReliable=false)
- Update review findings doc and SKILL.md pattern log

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@natashaannn natashaannn merged commit 10f0632 into main May 9, 2026
1 check passed
@natashaannn natashaannn deleted the refactor/s1-audiosync-determinism branch May 9, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deterministic FFT peak selection in AudioSyncer

1 participant

@natashaannn