fix(timestamp): eliminate f64 timebase and master clock drift in long recordings (#2144) by cocvn157 · Pull Request #4152 · CapSoftware/Cap

cocvn157 · 2026-11-13T03:44:59Z

Problem

During longer recordings (or on machines with high system uptime where raw hardware tick counters approach or exceed $2^{53}$), the cursor gradually drifts out of sync with the video track:

f64 tick/nanosecond conversions in platform timestamps (macos.rs & win.rs):
- MachAbsoluteTimestamp (crates/timestamp/src/macos.rs) converted raw mach_absolute_time() ticks and Duration offsets using f64 ((diff as f64 * freq) as u64, (self.0 as f64 + ticks) as u64) and queried mach_timebase_info on every call. On Apple Silicon (numer = 125, denom = 3), numer / denom is a repeating fraction (41.666...) and casting raw tick counters through f64 loses low-order tick precision once uptime ticks exceed the 53-bit f64 mantissa.
- PerformanceCounterTimestamp (crates/timestamp/src/win.rs) similarly used rhs.as_secs_f64() * freq as f64 in Add<Duration> / Sub<Duration> and (self.0 - other.0) as f64 / (perf_freq() as f64) in signed_duration_since_secs, losing sub-tick and sub-microsecond precision on long recordings.
Lossy f64 seconds roundtrip on every frame in MasterClock::remap_raw_ns (master_clock.rs & lib.rs):
- Every captured video and audio frame in the recording pipeline is remapped to the master timeline via MasterClock::remap_raw_ns(source_ts) (crates/recording/src/output_pipeline/core.rs).
- Previously, MasterClock::remap_raw_ns called source_ts.signed_duration_since_secs(self.timestamps) (converting integer nanoseconds/ticks into f64 seconds) and then multiplied back by 1_000_000_000.0 in f64 (seconds_to_ns_saturating), whereas cursor events record elapsed time directly from Instant::elapsed().

Solution

crates/timestamp/src/macos.rs:
- Cache mach_timebase_info (numer, denom) in a OnceLock<(u128, u128)>.
- Replace all f64 tick/nanosecond conversions in duration_since, checked_duration_since, duration_since_unix_epoch, Add<Duration>, and Sub<Duration> with rounded u128 ratio arithmetic (ticks_to_nanos_with_ratio / nanos_to_ticks_with_ratio) and integer saturating_add / saturating_sub.
- Add signed_duration_since_nanos(&self, other: Self) -> i128 and derive signed_duration_since_secs from it.
crates/timestamp/src/win.rs:
- Replace f64 tick conversions with rounded i128 / u128 integer ratio math (counter_diff_to_nanos / duration_to_counter_ticks).
- Add signed_duration_since_nanos(&self, other: Self) -> i128 and update signed_duration_since_secs to use exact integer nanosecond conversion rather than (self.0 - other.0) as f64 / (perf_freq() as f64).
crates/timestamp/src/lib.rs & crates/timestamp/src/master_clock.rs:
- Add Timestamp::signed_duration_since_nanos(&self, start: Timestamps) -> i128 across all platform variants (Instant, SystemTime, PerformanceCounter, MachAbsoluteTime).
- Update MasterClock::remap_raw_ns to use source_ts.signed_duration_since_nanos(self.timestamps) directly and saturate to i64, eliminating the integer -> f64 seconds -> f64 * 1e9 -> i64 roundtrip on every frame.
Tests:
- Added unit tests in macos.rs, win.rs, and master_clock.rs verifying exact Duration::ZERO identity at (1 << 53) + 1 ticks, fractional frame intervals (16_666_667 ns), Apple Silicon 125/3 ratio rounding, and multi-hour nanosecond remapping in MasterClock::remap_raw_ns.

Confidence Score: 2/5

Do not merge until the missing dependencies and broken recording APIs are fixed and the comments policy is satisfied.

Findings

New dependencies are missing ▶
Recording clock calls break ▶
Capture timestamp calls break ▶
Comments repeat the code ▶

Fix with agent prompt

### Issue 1
crates/timestamp/src/lib.rs:11-12
`serde` and `specta` are imported without being declared in `crates/timestamp/Cargo.toml`. Neither is available as a direct dependency, so `cap-timestamp` fails to compile on every platform. Add both dependencies or remove the new imports and derives.

### Issue 2
crates/timestamp/src/master_clock.rs:29-31
The replacement `MasterClock` removes APIs that the recording pipeline still uses. `core.rs` and `audio_mixer.rs` still import `SourceClockState` and `SourceClockOutcome`, call `MasterClock::new(timestamps, rate)`, and use sample-count methods. These missing symbols and incompatible calls prevent the recording crate from compiling.

Keep the existing clock APIs and apply the integer conversion within them, or migrate all callers in this PR.

### Issue 3
crates/timestamp/src/lib.rs:90
`Timestamp::duration_since` now returns `Option<Duration>`, but existing callers still use its result as a `Duration`. For example, `AudioMixer::detect_laggard_source` calls `.checked_sub(...)` on it. The rewrite also removes `Timestamp::from_cpal`, `checked_duration_since`, timestamp arithmetic, and `PerformanceCounterTimestamp::from_100ns` while capture code still calls them.

Preserve these APIs while adding nanosecond conversion, or update their callers together. Otherwise, the recording crate cannot compile.

### Issue 4
crates/timestamp/src/lib.rs:105-106
The new documentation comments only restate the method name and return value. `AGENTS.md` requires comments to capture non-obvious context and explicitly bans comments that paraphrase parameter names or narrate the code. Remove these comments or explain a non-obvious timing constraint instead. The same pattern appears above `signed_duration_since_secs` and elsewhere in the rewrite. This repository requirement must be satisfied before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Replaces floating-point timestamp conversions with integer arithmetic and adds exact nanosecond remapping.

Mac and Windows timestamps use rounded integer clock conversions.
The recording clock measures active time around pauses.
The master clock maps source times directly and drops per-source cadence smoothing.

_{Reviews (1) · Last reviewed commit: "fix(timestamp): eliminate f64 timebase a..." · Reviewed by Greptile}

CapSoftware#2144) - Convert MachAbsoluteTimestamp and PerformanceCounterTimestamp tick/duration arithmetic to rounded 128-bit integer ratio math with cached timebase info - Add integer-exact signed_duration_since_nanos across MachAbsoluteTimestamp, PerformanceCounterTimestamp, and Timestamp - Route MasterClock::remap_raw_ns directly through signed_duration_since_nanos instead of roundtripping through f64 seconds on every video/audio frame - Add unit tests covering high-uptime tick counters (> 2^53), Apple Silicon 125/3 timebases, and multi-hour MasterClock nanosecond remapping

vercel · 2026-11-13T03:45:06Z

@cocvn157 is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

greptile-apps · 2026-11-13T03:48:11Z

+use serde::{Deserialize, Serialize};
+use specta::Type;


New dependencies are missing

serde and specta are imported without being declared in crates/timestamp/Cargo.toml. Neither is available as a direct dependency, so cap-timestamp fails to compile on every platform. Add both dependencies or remove the new imports and derives.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/timestamp/src/lib.rs Line: 11-12 Comment: **New dependencies are missing** `serde` and `specta` are imported without being declared in `crates/timestamp/Cargo.toml`. Neither is available as a direct dependency, so `cap-timestamp` fails to compile on every platform. Add both dependencies or remove the new imports and derives. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-11-13T03:48:11Z

+impl MasterClock {
+    pub fn new() -> Self {
+        Self::from_timestamps(Timestamps::now())


Recording clock calls break

The replacement MasterClock removes APIs that the recording pipeline still uses. core.rs and audio_mixer.rs still import SourceClockState and SourceClockOutcome, call MasterClock::new(timestamps, rate), and use sample-count methods. These missing symbols and incompatible calls prevent the recording crate from compiling.

Keep the existing clock APIs and apply the integer conversion within them, or migrate all callers in this PR.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/timestamp/src/master_clock.rs Line: 29-31 Comment: **Recording clock calls break** The replacement `MasterClock` removes APIs that the recording pipeline still uses. `core.rs` and `audio_mixer.rs` still import `SourceClockState` and `SourceClockOutcome`, call `MasterClock::new(timestamps, rate)`, and use sample-count methods. These missing symbols and incompatible calls prevent the recording crate from compiling. Keep the existing clock APIs and apply the integer conversion within them, or migrate all callers in this PR. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-11-13T03:48:11Z

-            #[cfg(windows)]
-            Self::PerformanceCounter(counter) => {
-                counter.checked_duration_since(start.performance_counter)
+    pub fn duration_since(&self, start: Timestamps) -> Option<Duration> {


Capture timestamp calls break

Timestamp::duration_since now returns Option<Duration>, but existing callers still use its result as a Duration. For example, AudioMixer::detect_laggard_source calls .checked_sub(...) on it. The rewrite also removes Timestamp::from_cpal, checked_duration_since, timestamp arithmetic, and PerformanceCounterTimestamp::from_100ns while capture code still calls them.

Preserve these APIs while adding nanosecond conversion, or update their callers together. Otherwise, the recording crate cannot compile.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/timestamp/src/lib.rs Line: 90 Comment: **Capture timestamp calls break** `Timestamp::duration_since` now returns `Option<Duration>`, but existing callers still use its result as a `Duration`. For example, `AudioMixer::detect_laggard_source` calls `.checked_sub(...)` on it. The rewrite also removes `Timestamp::from_cpal`, `checked_duration_since`, timestamp arithmetic, and `PerformanceCounterTimestamp::from_100ns` while capture code still calls them. Preserve these APIs while adding nanosecond conversion, or update their callers together. Otherwise, the recording crate cannot compile. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-11-13T03:48:11Z

+    /// Returns the signed elapsed duration in nanoseconds since `start`.
+    /// Negative if `self` is earlier than `start`.


Comments repeat the code

The new documentation comments only restate the method name and return value. AGENTS.md requires comments to capture non-obvious context and explicitly bans comments that paraphrase parameter names or narrate the code. Remove these comments or explain a non-obvious timing constraint instead. The same pattern appears above signed_duration_since_secs and elsewhere in the rewrite. This repository requirement must be satisfied before merging.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/timestamp/src/lib.rs Line: 105-106 Comment: **Comments repeat the code** The new documentation comments only restate the method name and return value. `AGENTS.md` requires comments to capture non-obvious context and explicitly bans comments that paraphrase parameter names or narrate the code. Remove these comments or explain a non-obvious timing constraint instead. The same pattern appears above `signed_duration_since_secs` and elsewhere in the rewrite. This repository requirement must be satisfied before merging. **Context Used:** AGENTS.md ([source](https://github.com/capsoftware/cap/blob/main/AGENTS.md)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

algora-pbc Bot added the 🙋 Bounty claim label Nov 13, 2026

greptile-apps Bot reviewed Nov 13, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(timestamp): eliminate f64 timebase and master clock drift in long recordings (#2144) - #4152

fix(timestamp): eliminate f64 timebase and master clock drift in long recordings (#2144)#4152
cocvn157 wants to merge 1 commit into
CapSoftware:mainfrom
cocvn157:fix/cursor-video-sync-long-recordings-2144

cocvn157 commented Nov 13, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

vercel Bot commented Nov 13, 2026

Uh oh!

greptile-apps Bot Nov 13, 2026

Uh oh!

greptile-apps Bot Nov 13, 2026

Uh oh!

greptile-apps Bot Nov 13, 2026

Uh oh!

greptile-apps Bot Nov 13, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		/// Returns the signed elapsed duration in nanoseconds since `start`.
		/// Negative if `self` is earlier than `start`.

Conversation

cocvn157 commented Nov 13, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Confidence Score: 2/5

Findings

Summary

Uh oh!

vercel Bot commented Nov 13, 2026

Uh oh!

greptile-apps Bot Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cocvn157 commented Nov 13, 2026 •

edited by greptile-apps Bot

Loading