Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 13, 2025

This PR fixes a memory leak in VecVideoRecorder where recorded_frames stayed in memory due to references held by the MoviePy clip object. The fix applies the same solution as implemented in Farama-Foundation/Gymnasium#1444.

Problem

When VecVideoRecorder stops recording and saves a video, the recorded_frames list would remain in memory because MoviePy's ImageSequenceClip maintains references to the frame data even after the video is written. This caused a memory leak during long training sessions with video recording enabled.

Solution

Added explicit del statements in the _stop_recording() method to properly clean up references:

def _stop_recording(self) -> None:
    """Stop current recording and saves the video."""
    assert self.recording, "_stop_recording was called, but no recording was started"

    if len(self.recorded_frames) == 0:  # pragma: no cover
        logger.warn("Ignored saving a video as there were zero frames to save.")
    else:
        from moviepy.video.io.ImageSequenceClip import ImageSequenceClip

        clip = ImageSequenceClip(self.recorded_frames, fps=self.frames_per_sec)
        clip.write_videofile(self.video_path)
        del clip  # ← Added this line

    del self.recorded_frames  # ← Added this line
    self.recorded_frames = []
    self.recording = False

The fix ensures that:

  1. The MoviePy clip object is explicitly deleted after video writing
  2. The old recorded_frames list is deleted before assigning a new empty list
  3. Memory is properly released for garbage collection

Testing

  • All existing tests continue to pass
  • Added comprehensive memory cleanup verification tests
  • Verified that recorded_frames are properly cleared after video saves
  • Confirmed no regressions in video recording functionality

This change is backward compatible and only affects the internal memory management of the video recorder.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Applied same fix from Gymnasium PR #1444 - add del statements to properly clean up
moviepy clip and recorded_frames list to prevent memory leaks during video recording.

Co-authored-by: araffin <1973948+araffin@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] apply the same edits as @Farama-Foundation/Gymnasium/pull/1444 for SB3 vec video recorder, don't forget to update the changelog too. Fix memory leak in VecVideoRecorder by properly deleting references Sep 13, 2025
@Copilot Copilot AI requested a review from araffin September 13, 2025 13:05
Copilot finished work on behalf of araffin September 13, 2025 13:05
@araffin araffin marked this pull request as ready for review September 13, 2025 17:10
@araffin araffin merged commit 440ccdd into master Sep 13, 2025
4 checks passed
@araffin araffin deleted the copilot/fix-3e76bfba-bb6e-464c-bacc-855adadac401 branch September 13, 2025 17:31
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