Skip to content

fix: keep preview effects moving through VFR frame holds - #855

Open
SHN2004 wants to merge 1 commit into
webadderallorg:mainfrom
SHN2004:fix/vfr-preview-clock
Open

fix: keep preview effects moving through VFR frame holds#855
SHN2004 wants to merge 1 commit into
webadderallorg:mainfrom
SHN2004:fix/vfr-preview-clock

Conversation

@SHN2004

@SHN2004 SHN2004 commented Sep 2, 2026

Copy link
Copy Markdown

Description

Drive editor playback updates from requestAnimationFrame() and the continuous video.currentTime media clock instead of waiting for requestVideoFrameCallback().

This keeps the playhead, cursor, zoom, wallpaper, and other time-based preview effects moving while a variable-frame-rate source holds a video frame.

Motivation

requestVideoFrameCallback() only runs when Chromium presents a new decoded frame. Sparse/VFR ScreenCaptureKit recordings can hold one frame for seconds, which previously froze the entire editor clock until the next frame arrived.

Closes #854.

Type of Change

  • New Feature
  • Bug Fix
  • Refactor / Code Cleanup
  • Documentation Update
  • Other (please specify)

Related Issue(s)

Screenshots / Video

Not applicable; the regression is covered by the playback scheduler test.

Testing Guide

  • npx vitest --run src/components/video-editor/videoPlayback/videoEventHandlers.test.ts
  • npm test — 1,036 tests passed
  • npx tsc --noEmit
  • npx biome lint src/components/video-editor/videoPlayback/videoEventHandlers.ts src/components/video-editor/videoPlayback/videoEventHandlers.test.ts
  • npx biome format src/components/video-editor/videoPlayback/videoEventHandlers.ts src/components/video-editor/videoPlayback/videoEventHandlers.test.ts

Checklist

  • I have performed a self-review of my code.
  • I have added any necessary screenshots or videos.
  • I have linked related issue(s); no changelog update is needed for this focused fix.

Summary by CodeRabbit

  • Bug Fixes
    • Improved video playback time updates by consistently synchronizing editor state with the video’s current playback position.
    • Improved pause and disposal behavior by reliably canceling pending animation-frame updates.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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: Team

Run ID: 2a785557-5ca5-4384-9345-6aa5a20fac87

📥 Commits

Reviewing files that changed from the base of the PR and between b952b3e and 1e52d39.

📒 Files selected for processing (2)
  • src/components/video-editor/videoPlayback/videoEventHandlers.test.ts
  • src/components/video-editor/videoPlayback/videoEventHandlers.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The video event handlers now poll video.currentTime with requestAnimationFrame. Presented-frame callback handling and related cancellation logic were removed. Tests now validate animation-frame scheduling, time updates, and cancellation.

Changes

Video playback clock

Layer / File(s) Summary
Use animation-frame playback updates
src/components/video-editor/videoPlayback/videoEventHandlers.ts
The handler always schedules updates with requestAnimationFrame, cancels animation frames, and reads playback time from video.currentTime.
Update playback clock tests
src/components/video-editor/videoPlayback/videoEventHandlers.test.ts
Tests now validate currentTime-based updates and cancelAnimationFrame calls. Presented-frame callback tests and mock members were removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 1e52d

This localized playback fix keeps preview effects advancing during variable-frame-rate frame holds without changing public interfaces, dependencies, or deployment behavior. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary fix: keeping preview effects advancing during variable-frame-rate frame holds.
Description check ✅ Passed The description includes the purpose, motivation, bug-fix classification, linked issue, testing guidance, and checklist status. It appropriately marks screenshots as not applicable.
Linked Issues check ✅ Passed The changes satisfy issue #854 by replacing frame-presentation-driven updates with requestAnimationFrame polling and video.currentTime, allowing editor effects and the timeline to advance during VFR f…
Out of Scope Changes check ✅ Passed The changes are limited to playback scheduling and its tests. They directly support the linked issue and do not introduce unrelated functionality.
Full details: Linked Issues check

Explanation

The changes satisfy issue #854 by replacing frame-presentation-driven updates with requestAnimationFrame polling and video.currentTime, allowing editor effects and the timeline to advance during VFR frame holds.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

creatorliao pushed a commit to creatorliao/Recordly that referenced this pull request Sep 6, 2026
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.

[Bug]: Editor preview freezes between frames in variable-frame-rate recordings

1 participant