[Bug Report] VideoRecorder breaks with older version of MoviePy

[chrisnota]

Describe the bug

I was hitting the following error with VideoRecorder:

Moviepy - Building video /path/to/video/best-episode-0.mp4.
Moviepy - Writing video /path/to/video/best-episode-0.mp4

Process Process-1:
Traceback (most recent call last):
  File "/path/to/python/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/path/to/python/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/path/to/project/scripts/common/recorder.py", line 76, in record_video_worker
    video_recorder.stop_recording()
  File "/path/to/python/site-packages/gymnasium/wrappers/rendering.py", line 407, in stop_recording
    clip.write_videofile(path, logger=moviepy_logger)
  File "/path/to/python/site-packages/decorator.py", line 235, in fun
    return caller(func, *(extras + args), **kw)
  File "/path/to/python/site-packages/moviepy/decorators.py", line 54, in requires_duration
    return f(clip, *a, **k)
  File "/path/to/python/site-packages/decorator.py", line 235, in fun
    return caller(func, *(extras + args), **kw)
  File "/path/to/python/site-packages/moviepy/decorators.py", line 135, in use_clip_fps_by_default
    return f(clip, *new_a, **new_kw)
  File "/path/to/python/site-packages/decorator.py", line 235, in fun
    return caller(func, *(extras + args), **kw)
  File "/path/to/python/site-packages/moviepy/decorators.py", line 22, in convert_masks_to_RGB
    return f(clip, *a, **k)
  File "/path/to/python/site-packages/moviepy/video/VideoClip.py", line 300, in write_videofile
    ffmpeg_write_video(self, filename, fps, codec,
  File "/path/to/python/site-packages/moviepy/video/io/ffmpeg_writer.py", line 213, in ffmpeg_write_video
    with FFMPEG_VideoWriter(filename, clip.size, fps, codec = codec,
  File "/path/to/python/site-packages/moviepy/video/io/ffmpeg_writer.py", line 88, in __init__
    '-r', '%.02f' % fps,
TypeError: must be real number, not NoneType

I noticed my moviepy was on an older 1.0 version. Upgrading to latest fixed the issue.

I think VideoRecorder may not be compatible with some or all moviepy 1.0 versions. We should bump the minimum version number in the pyproject.yaml.

Code example

video_recorder: RecordVideo = env.wrap(
            lambda env: RecordVideo(
                env,
                os.path.join(task.agent_path, "videos"),
                episode_trigger=lambda episode: True,
                disable_logger=True,
                fps=50,
                name_prefix=task.name,
            )
        )
        # run the environment until the episode finishes

System info

Name: gymnasium
Version: 1.0.0
Ubuntu 22.04

Additional context

This is probably what caused #882, which was closed because the user wasn't able to reproduce (possibly after inadvertently upgrading?)

Checklist

• I have checked that there is no similar issue in the repo

[pseudo-rnd-thoughts]

Very helpful @chrisnota
Do you know what the minimum version of moviepy will work?
Is there an easy gymnasium side fix for moviepy v1.0?

[chrisnota]

Probably, but I'm not sure. Very busy with some deadlines currently, but I could take a deeper look later next week? Just wanted to make sure this got logged.

[chrisnota]

This is really a separate issue, and is really a bug on moviepy, but it seems like there are some interesting memory leaks with moviepy. I ran this code, and got the following plot:

from moviepy.video.io.ImageSequenceClip import ImageSequenceClip
import time
import numpy as np
import psutil
import gc
import matplotlib.pyplot as plt


def get_memory_usage():
    """Return the memory usage in MB."""
    process = psutil.Process()
    mem_info = process.memory_info()
    return mem_info.rss / (1024 * 1024)  # Convert to MB

# Store memory measurements
memory_usage = []
start_memory = get_memory_usage()
print(f"Initial memory usage: {start_memory:.2f} MB")

n = 200
for i in range(n):
    print(f"Iteration {i+1}/{n}")
    
    # Measure memory before
    before_mem = get_memory_usage()
    
    # Create clip
    frames = [np.random.random((100, 100, 3))] * 255
    clip = ImageSequenceClip(frames, fps=50)
    
    # Explicitly close and delete the clip
    # clip.close()
    # del clip
    # del frames
    
    # Force garbage collection
    # gc.collect()
    
    # Measure memory after
    after_mem = get_memory_usage()
    memory_usage.append(after_mem)
    
    print(f"Memory usage: {after_mem:.2f} MB (Change: {after_mem - before_mem:.2f} MB)")
    
    time.sleep(0.1)

# Analyze for memory leaks
final_memory = memory_usage[-1]
print(f"\nMemory Analysis:")
print(f"Starting memory: {start_memory:.2f} MB")
print(f"Final memory: {final_memory:.2f} MB")
print(f"Difference: {final_memory - start_memory:.2f} MB")

# Calculate memory growth rate
if len(memory_usage) > 1:
    memory_growth = (memory_usage[-1] - memory_usage[0]) / len(memory_usage)
    print(f"Average memory growth per iteration: {memory_growth:.4f} MB")
    print(f"Memory leak detected: {'Yes' if memory_growth > 0.5 else 'No'}")

# Plot memory usage
plt.figure(figsize=(10, 6))
plt.plot(range(1, len(memory_usage) + 1), memory_usage)
plt.title('Memory Usage Over Time')
plt.xlabel('Iteration')
plt.ylabel('Memory Usage (MB)')
plt.grid(True)
plt.savefig('memory_usage.png')
plt.close()

print("Memory usage plot saved as 'memory_usage.png'")

Using clip.close() helps a little:

However, regardless of whether you have closed the clip, calling gc.collect() after each iteration totally solves this issue:

So I'm going to suggest calling gc.collect() on stop_recording(). Still hoping to put together a PR for this.

[pseudo-rnd-thoughts]

Thanks @chrisnota, that sounds like a good plan for now.
Have you reported this upstream to moviepy?

[pseudo-rnd-thoughts]

@chrisnota Running your script on my MacOS with pip install --upgrade moviepy on Python 3.10 I don't appear to get the same memory leak results.

Default without clip.close, del clip, del frames or gc.collect()

Memory Analysis:
Starting memory: 51.94 MB
Final memory: 70.50 MB
Difference: 18.56 MB
Average memory growth per iteration: 0.0926 MB
Memory leak detected: No

Could you clarify what OS and MoviePy version your using?

[chrisnota]

I'm on ubuntu 22.04, moviepy==2.1.2

[timote-MOREAUX]

Hi, I am coming from IsaacLab were the leak was noticed too, see isaac-sim/IsaacLab#1996. I traced back the issue to Gymnasium then MoviePy. I found an easy solution in Gymnasium that doesn't require to call the garbage collector. After checking MoviePy source code, it appears that not much of VideoRecorder is used and the lower level class FFMPEG_VideoWriter could be used directly:

def stop_recording(self):
    """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:
        logger.warn("Ignored saving a video as there were zero frames to save.")
    else:
        try:
            from moviepy.video.io.ffmpeg_writer import FFMPEG_VideoWriter
        except ImportError as e:
            raise error.DependencyNotInstalled(
                'MoviePy is not installed, run `pip install "gymnasium[other]"`'
            ) from e

        
        path = os.path.join(self.video_folder, f"{self._video_name}.mp4")
        
        with FFMPEG_VideoWriter(
            filename=path,
            size=self.recorded_frames[0].shape[:2][::-1],
            fps=self.frames_per_sec
        ) as writer:

            for frame in self.recorded_frames:
                writer.write_frame(frame)

    self.recorded_frames = []
    self.recording = False
    self._video_name = None

I tested it and it completely fixed the issue. Before the patch:

After using FFMPEG_VideoWriter:

Also, I may have found the issue in MoviePy and have a potential solution too, though updating it would be complicated as the dependencies are already conflicting thus, the version used is outdated. I will still report the issue to them.

[pseudo-rnd-thoughts]

@timote-MOREAUX Investigating this on Ubuntu in #1377 with memray and a couple of other tools, I found that numpy was actuall the culprit.
I think whats happening is that we need to store a list of rgb array to then include in the rendering.
However, numpy / python doesn't have time to free the numpy arrays after they've been created into the video.
This means that numpy starts increasing the heap size iteratively and doesn't decrease aggressively enough.

Just adding gc.collect() after each video is saved fixed the issue for me.
Though there might be a deeper MoviePy issue

[timote-MOREAUX]

@pseudo-rnd-thoughts I dug deeper in MoviePy to be certain of the cause, you can check the report I made there with the bug report mentioned above.

The numpy array isn't freed because of circular referencing in ImageSequenceClip and it has nothing to do with any Numpy incorrect implementation. The array should have been freed when ImageSequenceClip is de-referenced. The array is referenced by the ImageSequenceClip.sequence variable which is in the circle and holds onto the frames. Thus the variable cannot be automatically collected and needs either to be deleted to break the circular referencing (Implementing a close() method) or to be collected by force calling gc.collect(). I tested both solutions.

I think that using gc.collect() there is a bad practice and should be avoided. The best patch would be to avoid circular reference at all in the class but I didn't checked the code enough to see if it could be refactored easily.