Skip to content

Stream camera video frames instead of buffering episodes - #1000

Merged
alexmillane merged 1 commit into
mainfrom
alex/feature/incremental_video_encoding
Aug 3, 2026
Merged

Stream camera video frames instead of buffering episodes#1000
alexmillane merged 1 commit into
mainfrom
alex/feature/incremental_video_encoding

Conversation

@alexmillane

@alexmillane alexmillane commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Encode camera videos incrementally so host RAM no longer scales with episode length.

Detailed description

  • Why: CameraObsVideoRecorder stored every frame of an episode before encoding, costing num_envs × episode_length × H × W × C. For robolab tasks (3× 1280x720 droid cameras, 1000-step episodes, 50Hz) that is 8.3 GB per env per episode.
  • What: frames now stream to a per-(env, camera) FFMPEG_VideoWriter as they arrive.
  • Preformance impact: measured on canned_food_in_bin, peak host RAM drops 8.32 GB → 0.72 GB. Wall clock unchanged.

Before and after experiment.

peak_ram_recording_comparison ram_traces_recording

@alexmillane alexmillane left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Self review 1.

Comment thread isaaclab_arena/tests/test_camera_observation_video_recorder.py Outdated
Comment thread isaaclab_arena/tests/test_camera_observation_video_recorder.py Outdated
Comment thread isaaclab_arena/tests/test_camera_observation_video_recorder.py Outdated
Comment thread isaaclab_arena/video/camera_observation_video_recorder.py Outdated
Comment thread isaaclab_arena/video/camera_observation_video_recorder.py Outdated
Comment thread isaaclab_arena/video/camera_observation_video_recorder.py Outdated
Comment thread isaaclab_arena/video/camera_observation_video_recorder.py Outdated
CameraObsVideoRecorder held every frame of an episode in memory before
encoding, costing num_envs x episode_length x H x W x C bytes. Frames are
now written to a per-(env, camera) ffmpeg encoder as they arrive.

Measured on canned_food_in_bin with 3x 1280x720 cameras: peak host RAM at
6 envs drops 60.5 GB -> 13.1 GB, with wall clock unchanged.

Signed-off-by: alex <amillane@nvidia.com>
@alexmillane
alexmillane force-pushed the alex/feature/incremental_video_encoding branch from 78173ad to 443177f Compare August 1, 2026 14:38
@alexmillane
alexmillane marked this pull request as ready for review August 1, 2026 20:50
@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces per-episode raw camera-frame buffering with incremental encoding through one ffmpeg writer per environment and camera, substantially reducing peak host memory.

  • Opens each episode’s encoder lazily when its first recordable frame arrives.
  • Finalizes writers at environment termination or truncation.
  • Closes and deletes files belonging to partial episodes during wrapper shutdown.
  • Updates unit tests to verify streaming, writer lifecycle, episode naming, and partial-file removal.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified.

Episode counters are sampled when each current episode’s writer opens, completed streams are finalized on reset, partial streams are removed on shutdown, and the updated tests cover the principal lifecycle transitions.

Important Files Changed

Filename Overview
isaaclab_arena/video/camera_observation_video_recorder.py Replaces frame lists and end-of-episode bulk encoding with lazily opened per-stream ffmpeg writers, including completion and partial-episode cleanup.
isaaclab_arena/tests/test_camera_observation_video_recorder.py Reworks recorder tests around a writer stand-in and adds direct assertions for incremental frame delivery and encoder lifecycle behavior.

Sequence Diagram

sequenceDiagram
  participant Env
  participant Recorder as CameraObsVideoRecorder
  participant Writer as FFMPEG_VideoWriter
  Env->>Recorder: step() observations
  alt Environment is active
    Recorder->>Writer: lazily open episode writer
    Recorder->>Writer: write_frame(frame)
  else Environment reset
    Recorder->>Writer: close and finalize completed episode
  end
  alt Wrapper closes during partial episode
    Recorder->>Writer: close
    Recorder->>Recorder: delete partial mp4
  end
Loading

Reviews (1): Last reviewed commit: "Stream camera frames to the encoder inst..." | Re-trigger Greptile

Comment thread isaaclab_arena/video/camera_observation_video_recorder.py
@arena-review-bot

Copy link
Copy Markdown
Contributor

🤖 Isaac Lab-Arena Review Bot

Summary

This PR switches CameraObsVideoRecorder from buffering an entire episode of raw frames to streaming each frame straight into a per-(env, camera) FFMPEG_VideoWriter, cutting peak host RAM dramatically (measured 8.32 GB → 0.72 GB on canned_food_in_bin). The change lands in the right layer, preserves the episode-index lockstep (now derived at encoder-open time instead of the old -1-at-flush), correctly reads the counter via self.unwrapped, and keeps partial-episode deletion on close(). The tests were rewritten cleanly to validate streaming (frames reach the encoder as they arrive, partial files removed, per-env episode numbering) plus a real-ffmpeg smoke test. Solid, well-scoped change.

Findings

🔵 Improvement — camera_observation_video_recorder.py:175 — Streaming keeps num_envs × num_cameras ffmpeg processes open for the whole episode (vs. one encode at a time before). Likely fine given the single-thread/low-CPU note, but worth confirming the process/FD count holds at the larger env counts you will actually run.

Test Coverage

Good. Tests are CPU-only (no Isaac Sim), so the inner/outer sim pattern does not apply; the moviepy encoder is replaced by a counting stand-in and there is a skipif(ffmpeg missing) real-encode test. New behavior is covered: frames stream rather than buffer, partial episodes leave no file, empty episodes still advance the index, and post-reset frames are excluded. The streaming rewrite is a behavior-preserving refactor and the suite tracks it closely.

Verdict

Ship it

@alexmillane
alexmillane enabled auto-merge (squash) August 3, 2026 09:45
@alexmillane
alexmillane merged commit 8abe2af into main Aug 3, 2026
47 of 51 checks passed
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