Skip to content

fix: use wall-clock timing for SVG and ffmpeg framerate - #6

Closed
xav-ie wants to merge 2 commits into
agentstation:mainfrom
xav-ie:fix/svg-wall-clock-timing
Closed

xav-ie wants to merge 2 commits into
agentstation:mainfrom
xav-ie:fix/svg-wall-clock-timing

Conversation

@xav-ie

@xav-ie xav-ie commented Feb 26, 2026

Copy link
Copy Markdown

Summary

  • Capture wall-clock timestamps during recording so SVG animation duration matches real elapsed time, even when CanvasToImage runs slower than the target framerate
  • Derive the actual capture rate (totalFrames / wallDuration) and pass it to ffmpeg as the input framerate so MP4 duration matches too

Context

When CanvasToImage is slower than the target frame interval, fewer frames are captured than framerate × duration predicts. Previously the SVG used frame-index-based timing, causing animations to play back too fast. This fix uses real wall-clock timestamps instead.

Test plan

  • Existing tests pass (go test ./...)
  • Added TestSVGGenerator_SlowCaptureTimestamp to verify non-uniform timestamps produce correct animation timing
  • Each commit compiles individually (bisectable)

Replace time.After with time.Ticker and track actual elapsed time
so SVG animation duration and keyframe percentages reflect real
wall-clock time. When CanvasToImage is slower than the target frame
interval, frames are captured at a lower rate but timestamps remain
accurate, preventing compressed playback in the SVG output.
Derive the real capture rate (totalFrames / wallDuration) and pass it
to ffmpeg as the input framerate. This ensures MP4 video duration
matches the wall-clock recording time when CanvasToImage runs slower
than the target framerate.
@jackspirou

Copy link
Copy Markdown
Member

Closing as superseded by the merged timing implementation.
Main preserves visible elapsed time across SVG, GIF, MP4, and WebM output.
It includes the final visible hold, excludes Hide intervals, and repeats raster states at their recorded timestamps.
This also handles raster-only recordings, without an SVG-dependent average frame rate.

The reviewed PR head rendered a 2.5-second visible hold as a 1-second SVG.
The replacement rendered that hold as 2.5 seconds.
The merged implementation includes regression tests for hidden spans, slow capture, raster timing, and capture failures.

@jackspirou jackspirou closed this Oct 8, 2026
@jackspirou

Copy link
Copy Markdown
Member

Thank you, @xav-ie, for identifying the recording-clock problem and adding a regression test. Your proposal established the user benefit we must preserve: SVG and video playback must follow visible elapsed recording time, including slow capture. The current implementation uses visible frame timestamps and raster repetition. I am auditing those contracts against your original proposal, including hidden intervals and capture failures, rather than treating PR closure as proof that the problem is solved. I will post the final coverage evidence here.

@jackspirou

Copy link
Copy Markdown
Member

The timing benefit audit is complete. The current implementation preserves visible elapsed time rather than nominal capture count. The slow-capture regression and nonuniform raster-state regression pass with the race detector. Hidden spans, the final visible hold, SVG-only capture, and capture/encoder failures also have passing coverage.

A fresh native capture with 2.5 seconds visible and 1.5 seconds hidden produced a 2.50-second SVG and 2.52-second MP4, GIF, and WebM outputs. A second raster-only capture with PlaybackSpeed 2 and LoopOffset 25 produced 1.28-second outputs in all three formats, against a 1.25-second target. The small differences are encoder frame quantization. Chrome and real ffmpeg were used.

This covers the original slow-capture and video timing value. No timing benefit was discarded when this PR closed. Thank you again for identifying the issue and supplying its first regression test.

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.

2 participants

⌂ 换网址