[shobhitagnihotri69]

Summary

rgb_frames(frames_per_second=...) now configures FFmpeg's fps filter with round=up, ensuring that emitted frames correspond to the picture visible at or before each sampling tick rather than pictures recorded after it.

Why

FFmpeg's default fps rounding is round=near, which selects frames up to half a sample period after the output tick (e.g., 14 source frames late at 1 fps on 30 fps video). Downstream callers like mediapipe_hands attribute sampled frames via source_log_times_for_sampled_frames, which relies on round=up (the frame visible at-or-before each tick). Specifying fps={frames_per_second:g}:round=up aligns the extracted RGB pixel content with the assigned log times across all sample rates.

Refs #733

Validation

• Added test_rgb_frames_sampling_aligns_with_output_ticks in tests/test_ffmpeg.py testing sample rates 1.0, 3.0, 10.0, and 30.0 fps against a barcode-numbered video: all 4 passed.
• Ran uv run pytest tests/test_ffmpeg.py tests/test_mediapipe_hands.py tests/test_video.py tests/test_source_sampling.py tests/test_module_boundaries.py — 191 passed, 11 skipped.
• Ran uv run ruff check --fix, uv run ruff format, and uv run ty check — all passed without errors.

Checklist

• I added or updated outcome-focused tests for changed business logic.
• I updated documentation for changed behavior, flags, formats, or requirements.
• I ran uv run ruff check --fix, uv run ruff format, and uv run ty check.
• I ran the relevant pytest suite.
• I did not add recordings, generated media, credentials, private URLs, or runtime artifacts.
• I preserved stored-data compatibility or documented an explicit version change.

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The change affects sampled video measurements in a bounded workflow.

The PR appears safe to merge, with two non-blocking improvements to the regression test.

Findings

1.  Missing frames still pass ▶
2.  Between-frame sampling stays untested ▶

Summary

rgb_frames now sets FFmpeg's fps filter to round=up, matching the at-or-before rule used to assign source log times.

• Sampled RGB frames now match the source frame assigned to each output tick.

_{Reviews (1) · Last reviewed commit: "fix(measurements): align rgb_frames samp..." · Reviewed by Greptile}

[greptile-apps]

Missing frames still pass

The expected list uses len(shown), so an empty result or a correct but shortened prefix passes. This lets a change that drops sampled frames escape the regression test. Build the expected list from the fixture's known three-second duration and sample_fps instead.

Suggested change

	expected = [round(i * 30 / sample_fps) for i in range(len(shown))]
	expected = [round(i * 30 / sample_fps) for i in range(int(3 * sample_fps))]

[greptile-apps]

Between-frame sampling stays untested

Every tested rate divides 30 exactly, hiding the difference between round() and the required at-or-before rule. At 4 fps, the first sample after zero falls at source position 7.5: the expected frame is 7, but this expression gives 8. Add a non-dividing rate and use the floor-based rule from source_log_times_for_sampled_frames to protect this case.

[kstonekuan]

Could you change Fixes #733 to Refs #733? This fixes the offset, but the second point in the issue (the last good picture repeated across a lost keyframe) is still open, and merging would close it.