Skip to content

perf: avoid caching camera payloads during frame extraction - #689

Merged
kstonekuan merged 2 commits into
Hebbian-Robotics:mainfrom
Kaileshwar16:perf/687-frame-timestamps
Oct 5, 2026
Merged

kstonekuan merged 2 commits into
Hebbian-Robotics:mainfrom
Kaileshwar16:perf/687-frame-timestamps

Conversation

@Kaileshwar16

Copy link
Copy Markdown
Contributor

Summary

Episode.frames() and frames_at_indices() now collect timestamps from batches without
retaining the camera payloads. If the channel is already cached, they reuse its
timestamps.

The timestamp mapper also accepts NumPy arrays directly, avoiding an extra .tolist()
allocation.

Fixes #687.

Why

Both methods previously loaded and cached the entire camera channel just to access
timestamps. This could retain a large amount of encoded video during frame extraction.

This change preserves frame selection and returned timestamps. Tests cover unchanged log
times, cache preservation, and reuse of preloaded channels.

Validation

  • uv run --no-sync pytest -q tests/test_end_to_end.py tests/test_episode.py tests/ test_video.py tests/test_video_streaming.py tests/test_video_malformed_guards.py — 105
    passed.
  • uv run --no-sync pytest -q — 2,513 passed, 9 skipped, 1 failed.
  • The failure, test_legacy_cache_migration_merges_without_overwriting, also reproduces
    on unchanged upstream code.
  • uv run --no-sync ruff check --fix — passed.
  • uv run --no-sync ruff format — passed.
  • uv run --no-sync ty check — passed.
  • git diff --check — passed.

No benchmark was added, as agreed in the issue discussion.

Checklist

  • Added outcome-focused regression coverage.
  • Documentation reviewed; no public behavior or requirements changed.
  • Ran Ruff and type checks.
  • Ran the relevant tests and full suite; existing failure noted above.
  • No recordings, generated media, credentials, or runtime artifacts added.
  • Stored-data compatibility preserved.

@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors frame extraction to cache timestamps separately from payloads.

The PR appears safe to merge; the earlier repeated-read issue is fixed.

What we checked:

  • Cached times could go stale: Episode opens one reader and has no channel reload path. Both timestamp sources read from that reader.
Summary

Frame extraction now reads and caches camera timestamps without retaining the full encoded channel payload, while keeping frame selection and source times unchanged. The timestamp mapper also accepts NumPy arrays directly.

  • Episode.frames() and frames_at_indices() reuse timestamps from a cached channel or read them from batches.
  • Repeat extraction can reuse cached timestamps without reading camera batches again.
  • The timestamp mapper checks empty inputs by length and avoids converting arrays to lists.

Reviews (2) · Last reviewed commit: "perf: reuse camera timestamps across fra..."

Comment thread src/hflow/episode.py

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, merging. Thanks.

@kstonekuan
kstonekuan merged commit 6050356 into Hebbian-Robotics:main Oct 5, 2026
3 checks passed
@kstonekuan kstonekuan mentioned this pull request Oct 8, 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.

perf: avoid materializing full camera channel in Episode.frames() when only timestamps are needed

2 participants