Skip to content

fix(webcam): stabilize frame sync#646

Merged
webadderall merged 2 commits into
mainfrom
fix/webcam-stutter-sync
Jun 2, 2026
Merged

fix(webcam): stabilize frame sync#646
webadderall merged 2 commits into
mainfrom
fix/webcam-stutter-sync

Conversation

@webadderall
Copy link
Copy Markdown
Collaborator

@webadderall webadderall commented Jun 2, 2026

Problem

  • webcam sources could stutter because preview sync was issuing unnecessary corrective seeks and export cache refresh checks were judging webcam freshness against the main clip time instead of the webcam target time

Fix

  • add a shared webcam seek-decision helper so preview sync avoids repeated corrective seeks while the webcam element is already seeking
  • keep preview timeline tracking stable so normal playback is not misclassified as repeated timeline jumps
  • compare export webcam cache freshness against the expected webcam target time, including offset webcam timelines

Validation

  • npm exec -- vitest --run src/components/video-editor/videoPlayback/webcamSync.test.ts src/lib/exporter/frameRenderer.test.ts src/lib/exporter/modernFrameRenderer.test.ts
  • npm exec -- biome check src/components/video-editor/VideoPlayback.tsx src/components/video-editor/videoPlayback/webcamSync.ts src/components/video-editor/videoPlayback/webcamSync.test.ts src/lib/exporter/frameRenderer.ts src/lib/exporter/frameRenderer.test.ts src/lib/exporter/modernFrameRenderer.test.ts

Notes

  • Biome still reports existing exhaustive-deps warnings in VideoPlayback.tsx that predate this change.

Summary by CodeRabbit

  • Bug Fixes

    • Improved webcam playback synchronization during video editing by refining the logic for when corrective seeks occur, reducing unnecessary sync adjustments.
    • Enhanced webcam time offset handling in the exporter to better align webcam frames with edited content.
  • Tests

    • Added test coverage for webcam synchronization decision logic and time offset export scenarios.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e67f2338-03fe-4765-9dd7-b868f840758d

📥 Commits

Reviewing files that changed from the base of the PR and between 70f5714 and 5fee014.

📒 Files selected for processing (1)
  • src/components/video-editor/videoPlayback/webcamSync.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/video-editor/videoPlayback/webcamSync.ts

📝 Walkthrough

Walkthrough

This PR extracts webcam seek logic into a new shouldSeekWebcamMedia helper, adds unit tests, integrates it into playback sync, and updates the frame renderer to compare cached/live webcam times against an expected media target time (including timeOffsetMs).

Changes

Webcam Seek Decision Helper Refactor

Layer / File(s) Summary
Webcam seek helper definition and unit tests
src/components/video-editor/videoPlayback/webcamSync.ts, src/components/video-editor/videoPlayback/webcamSync.test.ts
shouldSeekWebcamMedia is added with JSDoc and tests. It suppresses seeks while the element is seeking, detects timeline jumps (null previous time or >0.25s discontinuity), and triggers seeks when webcam current time deviates from the desired target beyond adaptive thresholds (0.35s when playing, 0.01s otherwise). Three tests cover seeking suppression, drift-triggered seeking, and no-op when target already matches.
Playback synchronization integration
src/components/video-editor/VideoPlayback.tsx
syncWebcamMedia now calls shouldSeekWebcamMedia (passing desired time, play/seeking state, previous/current timeline time, and webcam current time) to decide when to set webcamVideo.currentTime, replacing the previous inline heuristic.
Export frame renderer cache refresh using target time
src/lib/exporter/frameRenderer.ts, src/lib/exporter/frameRenderer.test.ts
drawWebcamOverlay calculates expectedWebcamTargetTime with getWebcamMediaTargetTimeSeconds (accounting for webcam duration and timeOffsetMs) and compares cached and live webcam times against this expected time. A new test validates cache refresh/draw when webcam.timeOffsetMs is configured.
Test formatting cleanup
src/lib/exporter/modernFrameRenderer.test.ts
Whitespace/formatting changes in the webcam readiness timeout test; behavior unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • webadderallorg/Recordly#519: Updates exporter webcam timing/caching logic using an expected webcam target time and affects webcam sync/cache decisions.
  • webadderallorg/Recordly#394: Modifies syncWebcamMedia drift and seek behavior; related to the playback sync changes here.

Suggested labels

Checked

Suggested reviewers

  • meiiie

Poem

🐰 I hop on frames and count each beat,
I nibble drifts till times align neat.
One helper now tells cameras to seek,
Silent and swift — no more timing peek.
Hooray for synced video, neat and sweet!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description includes Problem, Fix, Validation, and Notes sections explaining the changes, but omits several required template sections: Motivation, Type of Change, Related Issue(s), Screenshots/Video, Testing Guide, and Checklist. Complete the PR description by adding the missing template sections: explicitly mark Type of Change (Bug Fix), link any related issues, include Testing Guide details, and complete the Checklist section.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(webcam): stabilize frame sync' clearly and concisely describes the main change: fixing webcam frame synchronization stuttering by stabilizing sync logic.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/webcam-stutter-sync

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@webadderall
Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 2, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@webadderall webadderall merged commit 05fdfa4 into main Jun 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant