Fix(record): save camera recording on episode reset by yuecideng · Pull Request #208 · DexForce/EmbodiChain

yuecideng · 2026-03-30T16:20:46Z

Description

This PR refactors record_camera_data to fix the issue where the final episode's camera frames were never saved to video.

Previously, video saving was triggered inside __call__ by detecting that elapsed_steps == 0 (start of a new episode). This meant the last episode's frames were always lost because the reset happened before the next step() call could trigger the save.

Changes:

record.py: Extract video saving and buffer clearing into a new save_and_clear() method on record_camera_data. Store _save_path from config params in __init__. Remove the save/clear block from __call__. Override save_and_clear() as no-op in record_camera_data_async (which has its own saving logic).
embodied_env.py: In _initialize_episode, discover any record_camera_data functor instances in the event manager and call save_and_clear() before reset events are applied.

Dependencies: None

Type of change

Bug fix (non-breaking change which fixes an issue)
Enhancement (non-breaking change which improves an existing functionality)
New feature (non-breaking change which adds functionality)
Breaking change (existing functionality will not work without user modification)
Documentation update

Screenshots

N/A

Checklist

I have run the black . command to format the code base.
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
Dependencies have been updated, if applicable.

🤖 Generated with Claude Code

…all__ Extract video saving and buffer clearing logic from record_camera_data.__call__ into a new save_and_clear() method. Call this method from EmbodiedEnv._initialize_episode to ensure frames are saved before the episode is reset, fixing the issue where the final episode's frames were never saved. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors camera recording so buffered frames are saved when an episode resets (instead of relying on detecting the next step()), preventing the final episode’s frames from being dropped.

Changes:

Add save_and_clear() to record_camera_data and persist _save_path from config.
Remove “save-on-reset-detection” logic from record_camera_data.__call__.
Call save_and_clear() from EmbodiedEnv._initialize_episode() before applying reset events.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`embodichain/lab/gym/envs/managers/record.py`	Introduces `save_and_clear()` and refactors recording/saving responsibilities.
`embodichain/lab/gym/envs/embodied_env.py`	Triggers recorder finalization at episode initialization/reset time.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-30T16:24:56Z

embodichain/lab/gym/envs/managers/record.py

@@ -142,15 +158,6 @@ def __call__(
        max_env_num: int = 16,
        save_path: str = "./outputs/videos",
    ):


save_path is still part of the functor call signature, but it is no longer used (saving now uses self._save_path captured in __init__). This makes runtime overrides of save_path ineffective and is inconsistent with record_camera_data_async (which still uses the passed save_path). Either remove save_path from the signature/params (and update configs accordingly) or set self._save_path = save_path inside __call__ so both code paths respect the same input.

Suggested change

):

):

# Ensure per-call save_path overrides the instance-level setting used by save_and_clear

self._save_path = save_path

Copilot · 2026-03-30T16:24:56Z

embodichain/lab/gym/envs/managers/record.py

        max_env_num: int = 16,
        save_path: str = "./outputs/videos",
    ):
-        # TODO: the current implementation will lost the final episode frames recording.
-        # Check if the frames should be saved for the current episode
-        if env.elapsed_steps.sum().item() == 0 and len(self._frames) > 0:
-            video_name = f"episode_{self._current_episode}_{self._name}"
-            images_to_video(self._frames, save_path, video_name, fps=20)
-
-            self._current_episode += 1
-            self._frames = []
-
        self.camera.update(fetch_only=self.camera.is_rt_enabled)
        data = self.camera.get_data()
        rgb = data["color"]


max_env_num is intended to limit how many env frames are used, but the current tiling logic later computes num_frames using max(rgb.shape[0], max_env_num), which won’t cap anything when rgb.shape[0] > max_env_num. Consider switching that computation to min(...) so max_env_num reliably bounds the grid size and avoids unnecessary work.

Copilot · 2026-03-30T16:24:57Z

embodichain/lab/gym/envs/embodied_env.py

+        # Save recorded camera data before resetting
+        if self.cfg.events and self.event_manager is not None:
+            from embodichain.lab.gym.envs.managers.record import record_camera_data
+
+            for mode_cfgs in self.event_manager._mode_functor_cfgs.values():
+                for functor_cfg in mode_cfgs:
+                    if isinstance(functor_cfg.func, record_camera_data):
+                        functor_cfg.func.save_and_clear()


This change introduces new episode-finalization behavior (calling save_and_clear() during resets), but there are no tests asserting that recordings are saved/cleared at the right time. Given BaseEnv.step() resets only a subset of envs on dones, adding a unit test around _initialize_episode(reset_ids=...) would help prevent regressions (e.g., ensure save/clear is only triggered on full resets or the intended condition).

Copilot · 2026-03-30T16:24:57Z

embodichain/lab/gym/envs/embodied_env.py

+        # Save recorded camera data before resetting
+        if self.cfg.events and self.event_manager is not None:
+            from embodichain.lab.gym.envs.managers.record import record_camera_data
+
+            for mode_cfgs in self.event_manager._mode_functor_cfgs.values():
+                for functor_cfg in mode_cfgs:
+                    if isinstance(functor_cfg.func, record_camera_data):
+                        functor_cfg.func.save_and_clear()


_initialize_episode() is called for partial resets as well (see BaseEnv.step() auto-resets only dones envs). Calling save_and_clear() here unconditionally will clear the shared frame buffer even when only a subset of envs is being reset, which can truncate videos and drop frames for envs that are still mid-episode. Consider only calling save_and_clear() when resetting all envs (e.g., env_ids is None or len(env_ids)==self.num_envs), or rework the recorder to buffer/save per-env when partial resets occur.

Suggested change

# Save recorded camera data before resetting

if self.cfg.events and self.event_manager is not None:

from embodichain.lab.gym.envs.managers.record import record_camera_data

for mode_cfgs in self.event_manager._mode_functor_cfgs.values():

for functor_cfg in mode_cfgs:

if isinstance(functor_cfg.func, record_camera_data):

functor_cfg.func.save_and_clear()

# Save recorded camera data before resetting.

# Only clear the shared recorder buffer when all environments are being reset.

if self.cfg.events and self.event_manager is not None:

from embodichain.lab.gym.envs.managers.record import record_camera_data

# A global reset occurs when env_ids is None, or when the number of

# environments to process matches the total number of environments.

if env_ids is None or len(env_ids_to_process) == self.num_envs:

for mode_cfgs in self.event_manager._mode_functor_cfgs.values():

for functor_cfg in mode_cfgs:

if isinstance(functor_cfg.func, record_camera_data):

functor_cfg.func.save_and_clear()

Copilot AI review requested due to automatic review settings March 30, 2026 16:20

Copilot started reviewing on behalf of yuecideng March 30, 2026 16:21 View session

yuecideng added the gym robot learning env and its related features label Mar 30, 2026

Copilot AI reviewed Mar 30, 2026

View reviewed changes

yuecideng merged commit ae030c5 into main Mar 31, 2026
9 checks passed

yuecideng deleted the fix/record-camera-save-on-reset branch March 31, 2026 01:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix(record): save camera recording on episode reset#208

Fix(record): save camera recording on episode reset#208
yuecideng merged 1 commit intomainfrom
fix/record-camera-save-on-reset

yuecideng commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    ):
+    ):
+        # Ensure per-call save_path overrides the instance-level setting used by save_and_clear
+        self._save_path = save_path

Conversation

yuecideng commented Mar 30, 2026

Description

Type of change

Screenshots

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants