Continue video clip indices after existing files

Previously, VideoRecorder started every process at clip index zero. Recording into an existing output directory could overwrite clip_0000.mp4 and any later clips from a previous play or replay run.

Initialize the recorder index from the highest existing file matching the configured prefix. Fresh or empty directories still start at zero.

Author: diegoferigo-rai

CONTRIBUTORS.md

@@ -83,6 +83,7 @@ Guidelines for modifications:
 * Daniela Hasenbring
 * Dhananjay Shendre
 * Dhyan Thakkar
+* Diego Ferigo
 * Dongxuan Fan
 * Dorsa Rohani
 * Ege Sekkin

source/isaaclab/changelog.d/video-recorder-continue-indices.rst

@@ -0,0 +1,4 @@
+Fixed
+^^^^^
+
+* Fixed video recording overwriting existing clips when a new process writes to a non-empty output directory.

source/isaaclab/isaaclab/envs/utils/video_recorder.py

@@ -14,6 +14,7 @@
 
 import logging
 import os
+import re
 from typing import TYPE_CHECKING
 
 import numpy as np
@@ -78,7 +79,7 @@ def __init__(self, cfg: VideoRecorderCfg, env: object):
         self._frames: list[np.ndarray] = []
         self._step_count = 0
         self._frames_step_count = 0
-        self._clip_index = 0
+        self._clip_index = self._next_clip_index()
         self._recording = False
         # Set to True after the first unrecoverable frame-capture error so that
         # subsequent steps do not propagate the exception or repeat the log message.
@@ -300,6 +301,21 @@ def _effective_output_dir(self) -> str:
     def _clip_path(self, index: int) -> str:
         return os.path.join(self._effective_output_dir(), f"{self.cfg.output_filename_prefix}_{index:04d}.mp4")
 
+    def _next_clip_index(self) -> int:
+        return max(self._existing_clip_indices(), default=-1) + 1
+
+    def _existing_clip_indices(self) -> list[int]:
+        output_dir = self._effective_output_dir()
+        if not os.path.isdir(output_dir):
+            return []
+
+        pattern = re.compile(rf"^{re.escape(str(self.cfg.output_filename_prefix))}_(?P<index>\d+)\.mp4$")
+        return [
+            int(match.group("index"))
+            for filename in os.listdir(output_dir)
+            if (match := pattern.match(filename)) is not None
+        ]
+
     def _close_clip(self) -> None:
         if not self._frames:
             self._recording = False
@@ -343,7 +359,9 @@ def _maybe_delete_old_clips(self) -> None:
         if self.cfg.keep_last_n_clips is None:
             return
         cutoff = self._clip_index - self.cfg.keep_last_n_clips
-        for index in range(max(0, cutoff)):
+        for index in self._existing_clip_indices():
+            if index >= cutoff:
+                continue
             path = self._clip_path(index)
             try:
                 os.remove(path)

source/isaaclab/test/envs/test_video_recorder.py

@@ -106,6 +106,28 @@ def test_init_raises_import_error_when_moviepy_missing():
             VideoRecorder(_cfg(), _make_env())
 
 
+def test_init_continues_clip_index_after_existing_files(tmp_path):
+    output_dir = tmp_path / "videos"
+    output_dir.mkdir()
+    (output_dir / "clip_0000.mp4").touch()
+    (output_dir / "clip_0007.mp4").touch()
+    (output_dir / "clip_final.mp4").touch()
+    (output_dir / "other_0008.mp4").touch()
+
+    recorder = VideoRecorder(_cfg(output_dir=str(output_dir), output_filename_prefix="clip"), _make_env())
+
+    assert recorder._clip_index == 8
+
+
+def test_init_starts_clip_index_at_zero_for_empty_output_dir(tmp_path):
+    output_dir = tmp_path / "videos"
+    output_dir.mkdir()
+
+    recorder = VideoRecorder(_cfg(output_dir=str(output_dir)), _make_env())
+
+    assert recorder._clip_index == 0
+
+
 # ---------------------------------------------------------------------------
 # Trigger logic
 # ---------------------------------------------------------------------------
@@ -419,24 +441,48 @@ def test_keep_last_n_clips_prunes_old_clips():
         _cfg(output_dir="/tmp/test_prune", keep_last_n_clips=2),
         _make_env(),
     )
+    recorder._clip_index = 3
     removed = []
 
     def fake_remove(path):
         removed.append(path)
 
-    mock_clip = MagicMock()
-    with patch("isaaclab.envs.utils.video_recorder.ImageSequenceClip", return_value=mock_clip):
-        with patch("isaaclab.envs.utils.video_recorder.os.makedirs"):
+    with patch("isaaclab.envs.utils.video_recorder.os.path.isdir", return_value=True):
+        with patch(
+            "isaaclab.envs.utils.video_recorder.os.listdir",
+            return_value=["clip_0000.mp4", "clip_0001.mp4", "clip_0002.mp4"],
+        ):
             with patch("isaaclab.envs.utils.video_recorder.os.remove", side_effect=fake_remove):
-                for i in range(3):
-                    recorder._frames = [_FRAME.copy()]
-                    recorder._recording = True
-                    recorder._close_clip()
+                recorder._maybe_delete_old_clips()
 
     # After 3 clips with keep_last_n_clips=2, clip index 0 should be removed.
     assert any("_0000.mp4" in p for p in removed), f"Expected clip 0 to be removed, got: {removed}"
 
 
+def test_keep_last_n_clips_prunes_only_existing_sparse_clips():
+    """Sparse clip indices must not trigger one deletion attempt per missing index."""
+
+    recorder = VideoRecorder(
+        _cfg(output_dir="/tmp/test_sparse_prune", keep_last_n_clips=2),
+        _make_env(),
+    )
+    recorder._clip_index = 10_000
+    removed = []
+
+    def fake_remove(path):
+        removed.append(path)
+
+    with patch("isaaclab.envs.utils.video_recorder.os.path.isdir", return_value=True):
+        with patch(
+            "isaaclab.envs.utils.video_recorder.os.listdir",
+            return_value=["clip_0001.mp4", "clip_9997.mp4", "clip_9998.mp4", "other_0000.mp4"],
+        ):
+            with patch("isaaclab.envs.utils.video_recorder.os.remove", side_effect=fake_remove):
+                recorder._maybe_delete_old_clips()
+
+    assert removed == ["/tmp/test_sparse_prune/clip_0001.mp4", "/tmp/test_sparse_prune/clip_9997.mp4"]
+
+
 # ---------------------------------------------------------------------------
 # Minor 14: partial-clip close() flush
 # ---------------------------------------------------------------------------