fix(recording): prevent double-count of VideoStartGate trim in mic_start_time#1966
Conversation
…prevent double-count in editor mic offset
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| // Without this, the editor's mic_offset = display_start - mic_start equals | ||
| // trim_duration and causes it to skip that same duration a second time. | ||
| let trim_duration = Duration::from_nanos( | ||
| trim_samples as u64 * 1_000_000_000 / sample_rate as u64, |
There was a problem hiding this comment.
Nice fix. Minor tweak: consider reusing the existing samples_to_nanos() helper here to avoid a potential divide-by-zero and to keep overflow/rounding behavior consistent.
| trim_samples as u64 * 1_000_000_000 / sample_rate as u64, | |
| let trim_duration = | |
| Duration::from_nanos(samples_to_nanos(trim_samples as u64, sample_rate)); |
| // metadata reflects the first committed sample, preventing the editor | ||
| // from double-counting the same gap as a skip offset. | ||
| let trim_duration = Duration::from_nanos( | ||
| expected_trim as u64 * 1_000_000_000 / info.sample_rate as u64, |
There was a problem hiding this comment.
Same thought in the test: using samples_to_nanos() keeps the conversion logic centralized and avoids info.sample_rate == 0 panicking in the future.
| expected_trim as u64 * 1_000_000_000 / info.sample_rate as u64, | |
| let trim_duration = Duration::from_nanos( | |
| samples_to_nanos(expected_trim as u64, info.sample_rate), | |
| ); |
|
/tip $150 |
|
@algora-pbc /tip $150 |
|
hey @zcesur, I think there might be an issue with @algora-pbc on this PR, the tip command was posted but the bot didn't respond. could you have a look when you get a chance? |
|
Please visit Algora to complete your tip via Stripe. |
|
/tip $220 |
|
Please visit Algora to complete your tip via Stripe. |
|
🎉🎈 @ManthanNimodiya has been awarded $220 by Cap! 🎈🎊 |
2 participants
Root Cause
When VideoStartGate trims leading samples from the first audio frame to align it with the first video frame, the returned AudioFrame kept the original buffer capture timestamp instead of the adjusted timestamp of the first committed sample.
This flowed into first_timestamp → mic_start_time in project metadata.
In the editor, mic_offset = display_start − mic_start = trim_duration, so the renderer skipped trim_duration into the audio file, but those samples were already removed by the gate.
Same gap counted twice: audio arrives late by up to one buffer period (~35 ms at 48 kHz / 1680-sample buffers).
Fix
Advance the frame timestamp by trim_samples / sample_rate before returning UseFrame, so mic_start_time reflects the first committed sample (≈ video start) and mic_offset collapses to ~0.
No change to the Passthrough path or the stale-startup audio_timing_repair_offset mechanism.
Added a timestamp assertion to the existing apply_gate_audio_leads_trims_front unit test; all 190 unit tests pass.
Greptile Summary
This PR fixes a double-counting bug where
VideoStartGatetrimmed leading audio samples to align with the first video frame but kept the original buffer-capture timestamp on the returnedAudioFrame, causing the editor to also skip ahead bytrim_durationbecausemic_offset = display_start − mic_startequalled the already-removed gap. The fix advancesframe.timestampbytrim_samples * 1e9 / sample_ratenanoseconds before constructing the trimmedAudioFrame, collapsingmic_offsetto near-zero.apply_video_start_gate): computestrim_durationfrom the integer trim-sample count and adds it toframe.timestampusing the existingTimestamp + Durationtrait impl, which handles every platform variant (Instant,SystemTime,PerformanceCounter,MachAbsoluteTime) correctly.apply_gate_audio_leads_trims_front): adds an assertion that the returned frame's timestamp equalsstart + trim_duration, comparing viasigned_duration_since_secsto avoid directTimestampequality. The formula mirrors the production code's integer round-trip, which is exact for the 20 ms / 48 kHz values in the test.Confidence Score: 5/5
Safe to merge — the change is a narrow, self-contained timestamp correction with no impact on the Passthrough or DropFrame paths.
The arithmetic is correct, division by zero is guarded by the existing zero-sample early return, integer overflow is not a concern given the 500 ms alignment limit, and the Timestamp + Duration trait handles all platform variants. The new unit-test assertion validates the same computation end-to-end.
No files require special attention.
Important Files Changed
trim_samples / sample_rateafter the VideoStartGate front-trim so thatmic_start_timein metadata reflects the first committed sample; adds a matching assertion to the existing unit test.Reviews (1): Last reviewed commit: "fix(recording): advance gate frame times..." | Re-trigger Greptile