Skip to content

Commit

Permalink
Remove closer for VideoRecorder
Browse files Browse the repository at this point in the history
  • Loading branch information
XuehaiPan committed Sep 26, 2021
1 parent a888dfa commit 78e1edb
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 23 deletions.
20 changes: 3 additions & 17 deletions gym/wrappers/monitoring/tests/test_video_recorder.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
import gc
import json
import os
import shutil
import tempfile
import time
import numpy as np

import gym
from gym.wrappers.monitoring.video_recorder import VideoRecorder, video_recorder_closer
from gym.wrappers.monitoring.video_recorder import VideoRecorder


class BrokenRecordableEnv(object):
Expand All @@ -30,17 +26,12 @@ def test_record_simple():
env.reset()
rec.capture_frame()
proc = rec.encoder.proc
with video_recorder_closer.lock:
num_registered = len(video_recorder_closer.closeables)

assert proc.poll() is None # subprocess is running
assert num_registered >= 1

rec.close()

assert proc.poll() is not None # subprocess is terminated
with video_recorder_closer.lock:
assert len(video_recorder_closer.closeables) == num_registered - 1
assert not rec.empty
assert not rec.broken
assert os.path.exists(rec.path)
Expand All @@ -57,24 +48,19 @@ def record():

rec_path = rec.path
proc = rec.encoder.proc
with video_recorder_closer.lock:
num_registered = len(video_recorder_closer.closeables)

assert proc.poll() is None # subprocess is running
assert num_registered >= 1

# The function ends without an explicit `rec.close()` call
# The Python interpreter will implicitly do `del rec` on garbage cleaning
return rec_path, proc, num_registered
return rec_path, proc

rec_path, proc, num_registered = record()
rec_path, proc = record()

gc.collect() # do explicit garbage collection for test
time.sleep(5) # wait for subprocess exiting

assert proc.poll() is not None # subprocess is terminated
with video_recorder_closer.lock:
assert len(video_recorder_closer.closeables) == num_registered - 1
assert os.path.exists(rec_path)
f = open(rec_path)
assert os.fstat(f.fileno()).st_size > 100
Expand Down
6 changes: 0 additions & 6 deletions gym/wrappers/monitoring/video_recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,12 @@
import numpy as np

from gym import error, logger
from gym.utils import closer


def touch(path):
open(path, "a").close()


video_recorder_closer = closer.Closer()


class VideoRecorder(object):
"""VideoRecorder renders a nice movie of a rollout, frame by frame. It
comes with an `enabled` option so you can still use the same code
Expand All @@ -42,7 +38,6 @@ def __init__(self, env, path=None, metadata=None, enabled=True, base_path=None):
self._async = env.metadata.get("semantics.async")
self.enabled = enabled
self._closed = False
self._recorder_id = video_recorder_closer.register(self)

# Don't bother setting anything else if not enabled
if not self.enabled:
Expand Down Expand Up @@ -190,7 +185,6 @@ def close(self):
self.write_metadata()

# Stop tracking this for autoclose
video_recorder_closer.unregister(self._recorder_id)
self._closed = True

def write_metadata(self):
Expand Down

0 comments on commit 78e1edb

Please sign in to comment.