diff --git a/moviepy/Clip.py b/moviepy/Clip.py index efdfb22c6..59dc5732f 100644 --- a/moviepy/Clip.py +++ b/moviepy/Clip.py @@ -60,7 +60,7 @@ def __init__(self): def copy(self): """ Shallow copy of the clip. - Returns a shwallow copy of the clip whose mask and audio will + Returns a shallow copy of the clip whose mask and audio will be shallow copies of the clip's mask and audio if they exist. This method is intensively used to produce new clips every time @@ -288,6 +288,7 @@ def set_duration(self, t, change_end=True): of the clip. """ self.duration = t + if change_end: self.end = None if (t is None) else (self.start + t) else: @@ -489,3 +490,24 @@ def generator(): return tqdm(generator(), total=nframes) return generator() + + def close(self): + """ + Release any resources that are in use. + + Implementation note for subclasses: + * Memory-based resources can be left to the garbage-collector. + * However, any open files should be closed, and subprocesses should be terminated. + * Be wary that shallow copies are frequently used. Closing a Clip may affect its copies. + * Therefore, should NOT be called by __del__(). + """ + pass + + # Support the Context Manager protocol, to ensure that resources are cleaned up. + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() + diff --git a/moviepy/audio/io/AudioFileClip.py b/moviepy/audio/io/AudioFileClip.py index 5c9042475..8b86b8920 100644 --- a/moviepy/audio/io/AudioFileClip.py +++ b/moviepy/audio/io/AudioFileClip.py @@ -44,12 +44,27 @@ class AudioFileClip(AudioClip): buffersize See Parameters. + Lifetime + -------- + + Note that this creates subprocesses and locks files. If you construct one of these instances, you must call + close() afterwards, or the subresources will not be cleaned up until the process ends. + + If copies are made, and close() is called on one, it may cause methods on the other copies to fail. + + However, coreaders must be closed separately. + Examples ---------- >>> snd = AudioFileClip("song.wav") + >>> snd.close() >>> snd = AudioFileClip("song.mp3", fps = 44100, bitrate=3000) - >>> snd = AudioFileClip(mySoundArray,fps=44100) # from a numeric array + >>> second_reader = snd.coreader() + >>> second_reader.close() + >>> snd.close() + >>> with AudioFileClip(mySoundArray,fps=44100) as snd: # from a numeric array + >>> pass # Close is implicitly performed by context manager. """ @@ -59,28 +74,26 @@ def __init__(self, filename, buffersize=200000, nbytes=2, fps=44100): AudioClip.__init__(self) self.filename = filename - reader = FFMPEG_AudioReader(filename,fps=fps,nbytes=nbytes, + self.reader = FFMPEG_AudioReader(filename,fps=fps,nbytes=nbytes, buffersize=buffersize) - - self.reader = reader self.fps = fps - self.duration = reader.duration - self.end = reader.duration + self.duration = self.reader.duration + self.end = self.reader.duration - self.make_frame = lambda t: reader.get_frame(t) - self.nchannels = reader.nchannels + self.make_frame = lambda t: self.reader.get_frame(t) + self.nchannels = self.reader.nchannels def coreader(self): """ Returns a copy of the AudioFileClip, i.e. a new entrance point to the audio file. Use copy when you have different clips watching the audio file at different times. """ - return AudioFileClip(self.filename,self.buffersize) + return AudioFileClip(self.filename, self.buffersize) + - def __del__(self): - """ Close/delete the internal reader. """ - try: - del self.reader - except AttributeError: - pass + def close(self): + """ Close the internal reader. """ + if self.reader: + self.reader.close_proc() + self.reader = None diff --git a/moviepy/audio/io/ffmpeg_audiowriter.py b/moviepy/audio/io/ffmpeg_audiowriter.py index 4e6ca8b71..7ec56be6a 100644 --- a/moviepy/audio/io/ffmpeg_audiowriter.py +++ b/moviepy/audio/io/ffmpeg_audiowriter.py @@ -125,11 +125,27 @@ def write_frames(self,frames_array): def close(self): - self.proc.stdin.close() - if self.proc.stderr is not None: - self.proc.stderr.close() - self.proc.wait() - del self.proc + if self.proc: + self.proc.stdin.close() + self.proc.stdin = None + if self.proc.stderr is not None: + self.proc.stderr.close() + self.proc.stdee = None + # If this causes deadlocks, consider terminating instead. + self.proc.wait() + self.proc = None + + def __del__(self): + # If the garbage collector comes, make sure the subprocess is terminated. + self.close() + + # Support the Context Manager protocol, to ensure that resources are cleaned up. + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() diff --git a/moviepy/audio/io/readers.py b/moviepy/audio/io/readers.py index 8e4935ad0..d0bdb6923 100644 --- a/moviepy/audio/io/readers.py +++ b/moviepy/audio/io/readers.py @@ -148,7 +148,7 @@ def close_proc(self): for std in [ self.proc.stdout, self.proc.stderr]: std.close() - del self.proc + self.proc = None def get_frame(self, tt): @@ -245,4 +245,5 @@ def buffer_around(self,framenumber): def __del__(self): + # If the garbage collector comes, make sure the subprocess is terminated. self.close_proc() diff --git a/moviepy/config.py b/moviepy/config.py index e102253f8..5a8451607 100644 --- a/moviepy/config.py +++ b/moviepy/config.py @@ -45,10 +45,9 @@ def try_cmd(cmd): else: success, err = try_cmd([FFMPEG_BINARY]) if not success: - raise IOError(err.message + - "The path specified for the ffmpeg binary might be wrong") - - + raise IOError( + str(err) + + " - The path specified for the ffmpeg binary might be wrong") if IMAGEMAGICK_BINARY=='auto-detect': if os.name == 'nt': @@ -65,8 +64,10 @@ def try_cmd(cmd): else: success, err = try_cmd([IMAGEMAGICK_BINARY]) if not success: - raise IOError(err.message + - "The path specified for the ImageMagick binary might be wrong") + raise IOError( + "%s - The path specified for the ImageMagick binary might be wrong: %s" % + (err, IMAGEMAGICK_BINARY) + ) diff --git a/moviepy/video/VideoClip.py b/moviepy/video/VideoClip.py index 28caf7d19..6ad8de976 100644 --- a/moviepy/video/VideoClip.py +++ b/moviepy/video/VideoClip.py @@ -250,6 +250,7 @@ def write_videofile(self, filename, fps=None, codec=None, >>> from moviepy.editor import VideoFileClip >>> clip = VideoFileClip("myvideo.mp4").subclip(100,120) >>> clip.write_videofile("my_new_video.mp4") + >>> clip.close() """ name, ext = os.path.splitext(os.path.basename(filename)) diff --git a/moviepy/video/compositing/CompositeVideoClip.py b/moviepy/video/compositing/CompositeVideoClip.py index 30172b782..8d0012ed9 100644 --- a/moviepy/video/compositing/CompositeVideoClip.py +++ b/moviepy/video/compositing/CompositeVideoClip.py @@ -75,9 +75,11 @@ def __init__(self, clips, size=None, bg_color=None, use_bgclip=False, if use_bgclip: self.bg = clips[0] self.clips = clips[1:] + self.created_bg = False else: self.clips = clips self.bg = ColorClip(size, col=self.bg_color) + self.created_bg = True @@ -117,6 +119,18 @@ def playing_clips(self, t=0): actually playing at the given time `t`. """ return [c for c in self.clips if c.is_playing(t)] + def close(self): + if self.created_bg and self.bg: + # Only close the background clip if it was locally created. + # Otherwise, it remains the job of whoever created it. + self.bg.close() + self.bg = None + if hasattr(self, "audio") and self.audio: + self.audio.close() + self.audio = None + + + def clips_array(array, rows_widths=None, cols_widths=None, diff --git a/moviepy/video/io/VideoFileClip.py b/moviepy/video/io/VideoFileClip.py index a44ae2a19..a5e300c40 100644 --- a/moviepy/video/io/VideoFileClip.py +++ b/moviepy/video/io/VideoFileClip.py @@ -12,7 +12,9 @@ class VideoFileClip(VideoClip): A video clip originating from a movie file. For instance: :: >>> clip = VideoFileClip("myHolidays.mp4") - >>> clip2 = VideoFileClip("myMaskVideo.avi") + >>> clip.close() + >>> with VideoFileClip("myMaskVideo.avi") as clip2: + >>> pass # Implicit close called by contex manager. Parameters @@ -61,6 +63,14 @@ class VideoFileClip(VideoClip): Read docs for Clip() and VideoClip() for other, more generic, attributes. + + Lifetime + -------- + + Note that this creates subprocesses and locks files. If you construct one of these instances, you must call + close() afterwards, or the subresources will not be cleaned up until the process ends. + + If copies are made, and close() is called on one, it may cause methods on the other copies to fail. """ @@ -74,7 +84,6 @@ def __init__(self, filename, has_mask=False, # Make a reader pix_fmt= "rgba" if has_mask else "rgb24" - self.reader = None # need this just in case FFMPEG has issues (__del__ complains) self.reader = FFMPEG_VideoReader(filename, pix_fmt=pix_fmt, target_resolution=target_resolution, resize_algo=resize_algorithm, @@ -110,14 +119,15 @@ def __init__(self, filename, has_mask=False, fps = audio_fps, nbytes = audio_nbytes) - def __del__(self): - """ Close/delete the internal reader. """ - try: - del self.reader - except AttributeError: - pass + def close(self): + """ Close the internal reader. """ + if self.reader: + self.reader.close() + self.reader = None try: - del self.audio + if self.audio: + self.audio.close() + self.audio = None except AttributeError: pass diff --git a/moviepy/video/io/ffmpeg_reader.py b/moviepy/video/io/ffmpeg_reader.py index 077c4053f..28b100aa5 100644 --- a/moviepy/video/io/ffmpeg_reader.py +++ b/moviepy/video/io/ffmpeg_reader.py @@ -103,9 +103,6 @@ def initialize(self, starttime=0): self.proc = sp.Popen(cmd, **popen_params) - - - def skip_frames(self, n=1): """Reads and throws away n frames """ w, h = self.size @@ -155,7 +152,7 @@ def get_frame(self, t): Note for coders: getting an arbitrary frame in the video with ffmpeg can be painfully slow if some decoding has to be done. - This function tries to avoid fectching arbitrary frames + This function tries to avoid fetching arbitrary frames whenever possible, by moving between adjacent frames. """ @@ -186,15 +183,11 @@ def close(self): self.proc.stdout.close() self.proc.stderr.close() self.proc.wait() - del self.proc - - def __del__(self): - self.close() - if hasattr(self,'lastread'): + self.proc = None + if hasattr(self, 'lastread'): del self.lastread - def ffmpeg_read_image(filename, with_mask=True): """ Read an image file (PNG, BMP, JPEG...). diff --git a/moviepy/video/io/ffmpeg_writer.py b/moviepy/video/io/ffmpeg_writer.py index 9200905f1..f4a2fa6f0 100644 --- a/moviepy/video/io/ffmpeg_writer.py +++ b/moviepy/video/io/ffmpeg_writer.py @@ -122,7 +122,7 @@ def __init__(self, filename, size, fps, codec="libx264", audiofile=None, # This was added so that no extra unwanted window opens on windows # when the child process is created if os.name == "nt": - popen_params["creationflags"] = 0x08000000 + popen_params["creationflags"] = 0x08000000 # CREATE_NO_WINDOW self.proc = sp.Popen(cmd, **popen_params) @@ -178,12 +178,21 @@ def write_frame(self, img_array): raise IOError(error) def close(self): - self.proc.stdin.close() - if self.proc.stderr is not None: - self.proc.stderr.close() - self.proc.wait() + if self.proc: + self.proc.stdin.close() + if self.proc.stderr is not None: + self.proc.stderr.close() + self.proc.wait() - del self.proc + self.proc = None + + # Support the Context Manager protocol, to ensure that resources are cleaned up. + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() def ffmpeg_write_video(clip, filename, fps, codec="libx264", bitrate=None, preset="medium", withmask=False, write_logfile=False, @@ -198,24 +207,22 @@ def ffmpeg_write_video(clip, filename, fps, codec="libx264", bitrate=None, logfile = None verbose_print(verbose, "[MoviePy] Writing video %s\n"%filename) - writer = FFMPEG_VideoWriter(filename, clip.size, fps, codec = codec, + with FFMPEG_VideoWriter(filename, clip.size, fps, codec = codec, preset=preset, bitrate=bitrate, logfile=logfile, audiofile=audiofile, threads=threads, - ffmpeg_params=ffmpeg_params) - - nframes = int(clip.duration*fps) + ffmpeg_params=ffmpeg_params) as writer: - for t,frame in clip.iter_frames(progress_bar=progress_bar, with_times=True, - fps=fps, dtype="uint8"): - if withmask: - mask = (255*clip.mask.get_frame(t)) - if mask.dtype != "uint8": - mask = mask.astype("uint8") - frame = np.dstack([frame,mask]) + nframes = int(clip.duration*fps) - writer.write_frame(frame) + for t,frame in clip.iter_frames(progress_bar=progress_bar, with_times=True, + fps=fps, dtype="uint8"): + if withmask: + mask = (255*clip.mask.get_frame(t)) + if mask.dtype != "uint8": + mask = mask.astype("uint8") + frame = np.dstack([frame,mask]) - writer.close() + writer.write_frame(frame) if write_logfile: logfile.close() diff --git a/setup.py b/setup.py index 15f18cc73..4af454d06 100644 --- a/setup.py +++ b/setup.py @@ -61,7 +61,7 @@ def run_tests(self): # Define the requirements for specific execution needs. requires = ['decorator==4.0.11', 'imageio==2.1.2', 'tqdm==4.11.2', 'numpy'] -optional_reqs = ['scikit-image==0.13.0', 'scipy==0.19.0', 'matplotlib==2.0.0'] +optional_reqs = ['scikit-image==0.13.0', 'scipy>=0.19.0,<=0.19.1', 'matplotlib>=2.0.0,<=2.0.2'] documentation_reqs = ['pygame==1.9.3', 'numpydoc>=0.6.0', 'sphinx_rtd_theme>=0.1.10b0', 'Sphinx>=1.5.2'] + optional_reqs test_reqs = ['pytest>=2.8.0', 'nose', 'sklearn', 'pytest-cov', 'coveralls'] \ diff --git a/tests/test_ImageSequenceClip.py b/tests/test_ImageSequenceClip.py index c188049cd..202c78733 100644 --- a/tests/test_ImageSequenceClip.py +++ b/tests/test_ImageSequenceClip.py @@ -22,9 +22,9 @@ def test_1(): durations.append(i) images.append("media/python_logo_upside_down.png") - clip = ImageSequenceClip(images, durations=durations) - assert clip.duration == sum(durations) - clip.write_videofile("/tmp/ImageSequenceClip1.mp4", fps=30) + with ImageSequenceClip(images, durations=durations) as clip: + assert clip.duration == sum(durations) + clip.write_videofile("/tmp/ImageSequenceClip1.mp4", fps=30) def test_2(): images=[] @@ -37,7 +37,7 @@ def test_2(): #images are not the same size.. with pytest.raises(Exception, message='Expecting Exception'): - ImageSequenceClip(images, durations=durations) + ImageSequenceClip(images, durations=durations).close() if __name__ == '__main__': diff --git a/tests/test_PR.py b/tests/test_PR.py index 0c4e2849d..f9ab93dca 100644 --- a/tests/test_PR.py +++ b/tests/test_PR.py @@ -9,8 +9,11 @@ from moviepy.video.tools.interpolators import Trajectory from moviepy.video.VideoClip import ColorClip, ImageClip, TextClip + sys.path.append("tests") -from test_helper import TMP_DIR, TRAVIS +from test_helper import TMP_DIR, TRAVIS, FONT + + def test_download_media(capsys): """Test downloading.""" @@ -34,11 +37,11 @@ def test_PR_339(): return # In caption mode. - TextClip(txt='foo', color='white', font="Liberation-Mono", size=(640, 480), - method='caption', align='center', fontsize=25) + TextClip(txt='foo', color='white', font=FONT, size=(640, 480), + method='caption', align='center', fontsize=25).close() # In label mode. - TextClip(txt='foo', font="Liberation-Mono", method='label') + TextClip(txt='foo', font=FONT, method='label').close() def test_PR_373(): result = Trajectory.load_list("media/traj.txt") @@ -65,16 +68,16 @@ def test_PR_424(): warnings.simplefilter('always') # Alert us of deprecation warnings. # Recommended use - ColorClip([1000, 600], color=(60, 60, 60), duration=10) + ColorClip([1000, 600], color=(60, 60, 60), duration=10).close() with pytest.warns(DeprecationWarning): # Uses `col` so should work the same as above, but give warning. - ColorClip([1000, 600], col=(60, 60, 60), duration=10) + ColorClip([1000, 600], col=(60, 60, 60), duration=10).close() # Catch all warnings as record. with pytest.warns(None) as record: # Should give 2 warnings and use `color`, not `col` - ColorClip([1000, 600], color=(60, 60, 60), duration=10, col=(2,2,2)) + ColorClip([1000, 600], color=(60, 60, 60), duration=10, col=(2,2,2)).close() message1 = 'The `ColorClip` parameter `col` has been deprecated. ' + \ 'Please use `color` instead.' @@ -90,26 +93,27 @@ def test_PR_458(): clip = ColorClip([1000, 600], color=(60, 60, 60), duration=10) clip.write_videofile(os.path.join(TMP_DIR, "test.mp4"), progress_bar=False, fps=30) + clip.close() def test_PR_515(): # Won't actually work until video is in download_media - clip = VideoFileClip("media/fire2.mp4", fps_source='tbr') - assert clip.fps == 90000 - clip = VideoFileClip("media/fire2.mp4", fps_source='fps') - assert clip.fps == 10.51 + with VideoFileClip("media/fire2.mp4", fps_source='tbr') as clip: + assert clip.fps == 90000 + with VideoFileClip("media/fire2.mp4", fps_source='fps') as clip: + assert clip.fps == 10.51 def test_PR_528(): - clip = ImageClip("media/vacation_2017.jpg") - new_clip = scroll(clip, w=1000, x_speed=50) - new_clip = new_clip.set_duration(20) - new_clip.fps = 24 - new_clip.write_videofile(os.path.join(TMP_DIR, "pano.mp4")) + with ImageClip("media/vacation_2017.jpg") as clip: + new_clip = scroll(clip, w=1000, x_speed=50) + new_clip = new_clip.set_duration(20) + new_clip.fps = 24 + new_clip.write_videofile(os.path.join(TMP_DIR, "pano.mp4")) def test_PR_529(): - video_clip = VideoFileClip("media/fire2.mp4") - assert video_clip.rotation == 180 + with VideoFileClip("media/fire2.mp4") as video_clip: + assert video_clip.rotation == 180 if __name__ == '__main__': diff --git a/tests/test_TextClip.py b/tests/test_TextClip.py index b07605c7b..ffd14b462 100644 --- a/tests/test_TextClip.py +++ b/tests/test_TextClip.py @@ -3,9 +3,9 @@ import pytest from moviepy.video.fx.blink import blink from moviepy.video.VideoClip import TextClip -from test_helper import TMP_DIR, TRAVIS sys.path.append("tests") +from test_helper import TMP_DIR, TRAVIS def test_duration(): #TextClip returns the following error under Travis (issue with Imagemagick) @@ -15,12 +15,14 @@ def test_duration(): return clip = TextClip('hello world', size=(1280,720), color='white') - clip.set_duration(5) + clip = clip.set_duration(5) # Changed due to #598. assert clip.duration == 5 + clip.close() clip2 = clip.fx(blink, d_on=1, d_off=1) - clip2.set_duration(5) + clip2 = clip2.set_duration(5) assert clip2.duration == 5 + clip2.close() # Moved from tests.py. Maybe we can remove these? def test_if_textclip_crashes_in_caption_mode(): @@ -28,13 +30,13 @@ def test_if_textclip_crashes_in_caption_mode(): return TextClip(txt='foo', color='white', size=(640, 480), method='caption', - align='center', fontsize=25) + align='center', fontsize=25).close() def test_if_textclip_crashes_in_label_mode(): if TRAVIS: return - TextClip(txt='foo', method='label') + TextClip(txt='foo', method='label').close() if __name__ == '__main__': pytest.main() diff --git a/tests/test_VideoFileClip.py b/tests/test_VideoFileClip.py index a8ec83e2a..61bfd0805 100644 --- a/tests/test_VideoFileClip.py +++ b/tests/test_VideoFileClip.py @@ -15,39 +15,43 @@ def test_setup(): blue = ColorClip((1024,800), color=(0,0,255)) red.fps = green.fps = blue.fps = 30 - video = clips_array([[red, green, blue]]).set_duration(5) - video.write_videofile("/tmp/test.mp4") + with clips_array([[red, green, blue]]).set_duration(5) as video: + video.write_videofile("/tmp/test.mp4") assert os.path.exists("/tmp/test.mp4") - clip = VideoFileClip("/tmp/test.mp4") - assert clip.duration == 5 - assert clip.fps == 30 - assert clip.size == [1024*3, 800] + with VideoFileClip("/tmp/test.mp4") as clip: + assert clip.duration == 5 + assert clip.fps == 30 + assert clip.size == [1024*3, 800] + + red.close() + green.close() + blue.close() def test_ffmpeg_resizing(): """Test FFmpeg resizing, to include downscaling.""" video_file = 'media/big_buck_bunny_432_433.webm' target_resolution = (128, 128) - video = VideoFileClip(video_file, target_resolution=target_resolution) - frame = video.get_frame(0) - assert frame.shape[0:2] == target_resolution + with VideoFileClip(video_file, target_resolution=target_resolution) as video: + frame = video.get_frame(0) + assert frame.shape[0:2] == target_resolution target_resolution = (128, None) - video = VideoFileClip(video_file, target_resolution=target_resolution) - frame = video.get_frame(0) - assert frame.shape[0] == target_resolution[0] + with VideoFileClip(video_file, target_resolution=target_resolution) as video: + frame = video.get_frame(0) + assert frame.shape[0] == target_resolution[0] target_resolution = (None, 128) - video = VideoFileClip(video_file, target_resolution=target_resolution) - frame = video.get_frame(0) - assert frame.shape[1] == target_resolution[1] + with VideoFileClip(video_file, target_resolution=target_resolution) as video: + frame = video.get_frame(0) + assert frame.shape[1] == target_resolution[1] # Test upscaling target_resolution = (None, 2048) - video = VideoFileClip(video_file, target_resolution=target_resolution) - frame = video.get_frame(0) - assert frame.shape[1] == target_resolution[1] + with VideoFileClip(video_file, target_resolution=target_resolution) as video: + frame = video.get_frame(0) + assert frame.shape[1] == target_resolution[1] if __name__ == '__main__': diff --git a/tests/test_Videos.py b/tests/test_Videos.py index bd001a602..f3d17816c 100644 --- a/tests/test_Videos.py +++ b/tests/test_Videos.py @@ -17,15 +17,15 @@ def test_download_media(capsys): download_media.download() def test_afterimage(): - ai = ImageClip("media/afterimage.png") - masked_clip = mask_color(ai, color=[0,255,1]) # for green + with ImageClip("media/afterimage.png") as ai: + masked_clip = mask_color(ai, color=[0,255,1]) # for green - some_background_clip = ColorClip((800,600), color=(255,255,255)) + with ColorClip((800,600), color=(255,255,255)) as some_background_clip: - final_clip = CompositeVideoClip([some_background_clip, masked_clip], - use_bgclip=True) - final_clip.duration = 5 - final_clip.write_videofile("/tmp/afterimage.mp4", fps=30) + with CompositeVideoClip([some_background_clip, masked_clip], + use_bgclip=True) as final_clip: + final_clip.duration = 5 + final_clip.write_videofile("/tmp/afterimage.mp4", fps=30) if __name__ == '__main__': pytest.main() diff --git a/tests/test_compositing.py b/tests/test_compositing.py index 4b2db7a41..fbb47970b 100644 --- a/tests/test_compositing.py +++ b/tests/test_compositing.py @@ -1,7 +1,11 @@ # -*- coding: utf-8 -*- """Compositing tests for use with pytest.""" +from os.path import join +import sys import pytest from moviepy.editor import * +sys.path.append("tests") +from test_helper import TMP_DIR def test_clips_array(): red = ColorClip((1024,800), color=(255,0,0)) @@ -12,25 +16,30 @@ def test_clips_array(): with pytest.raises(ValueError, message="Expecting ValueError (duration not set)"): - video.resize(width=480).write_videofile("/tmp/test_clips_array.mp4") + video.resize(width=480).write_videofile(join(TMP_DIR, "test_clips_array.mp4")) + video.close() + red.close() + green.close() + blue.close() def test_clips_array_duration(): - red = ColorClip((1024,800), color=(255,0,0)) - green = ColorClip((1024,800), color=(0,255,0)) - blue = ColorClip((1024,800), color=(0,0,255)) - - video = clips_array([[red, green, blue]]).set_duration(5) + for i in range(20): + red = ColorClip((1024,800), color=(255,0,0)) + green = ColorClip((1024,800), color=(0,255,0)) + blue = ColorClip((1024,800), color=(0,0,255)) - with pytest.raises(AttributeError, - message="Expecting ValueError (fps not set)"): - video.write_videofile("/tmp/test_clips_array.mp4") + with clips_array([[red, green, blue]]).set_duration(5) as video: + with pytest.raises(AttributeError, + message="Expecting ValueError (fps not set)"): + video.write_videofile(join(TMP_DIR, "test_clips_array.mp4")) - #this one should work correctly - red.fps=green.fps=blue.fps=30 - video = clips_array([[red, green, blue]]).set_duration(5) - video.write_videofile("/tmp/test_clips_array.mp4") + #this one should work correctly + red.fps = green.fps = blue.fps = 30 + with clips_array([[red, green, blue]]).set_duration(5) as video: + video.write_videofile(join(TMP_DIR, "test_clips_array.mp4")) -if __name__ == '__main__': - pytest.main() + red.close() + green.close() + blue.close() diff --git a/tests/test_fx.py b/tests/test_fx.py index 7e400148a..88f5f4280 100644 --- a/tests/test_fx.py +++ b/tests/test_fx.py @@ -10,10 +10,11 @@ from moviepy.video.fx.fadeout import fadeout from moviepy.video.io.VideoFileClip import VideoFileClip +sys.path.append("tests") + import download_media from test_helper import TMP_DIR -sys.path.append("tests") def test_download_media(capsys): @@ -21,51 +22,51 @@ def test_download_media(capsys): download_media.download() def test_blackwhite(): - clip = VideoFileClip("media/big_buck_bunny_432_433.webm") - clip1 = blackwhite(clip) - clip1.write_videofile(os.path.join(TMP_DIR,"blackwhite1.webm")) + with VideoFileClip("media/big_buck_bunny_432_433.webm") as clip: + clip1 = blackwhite(clip) + clip1.write_videofile(os.path.join(TMP_DIR,"blackwhite1.webm")) # This currently fails with a with_mask error! # def test_blink(): -# clip = VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,10) -# clip1 = blink(clip, 1, 1) -# clip1.write_videofile(os.path.join(TMP_DIR,"blink1.webm")) +# with VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,10) as clip: +# clip1 = blink(clip, 1, 1) +# clip1.write_videofile(os.path.join(TMP_DIR,"blink1.webm")) def test_colorx(): - clip = VideoFileClip("media/big_buck_bunny_432_433.webm") - clip1 = colorx(clip, 2) - clip1.write_videofile(os.path.join(TMP_DIR,"colorx1.webm")) + with VideoFileClip("media/big_buck_bunny_432_433.webm") as clip: + clip1 = colorx(clip, 2) + clip1.write_videofile(os.path.join(TMP_DIR,"colorx1.webm")) def test_crop(): - clip = VideoFileClip("media/big_buck_bunny_432_433.webm") + with VideoFileClip("media/big_buck_bunny_432_433.webm") as clip: - clip1=crop(clip) #ie, no cropping (just tests all default values) - clip1.write_videofile(os.path.join(TMP_DIR, "crop1.webm")) + clip1=crop(clip) #ie, no cropping (just tests all default values) + clip1.write_videofile(os.path.join(TMP_DIR, "crop1.webm")) - clip2=crop(clip, x1=50, y1=60, x2=460, y2=275) - clip2.write_videofile(os.path.join(TMP_DIR, "crop2.webm")) + clip2=crop(clip, x1=50, y1=60, x2=460, y2=275) + clip2.write_videofile(os.path.join(TMP_DIR, "crop2.webm")) - clip3=crop(clip, y1=30) #remove part above y=30 - clip3.write_videofile(os.path.join(TMP_DIR, "crop3.webm")) + clip3=crop(clip, y1=30) #remove part above y=30 + clip3.write_videofile(os.path.join(TMP_DIR, "crop3.webm")) - clip4=crop(clip, x1=10, width=200) # crop a rect that has width=200 - clip4.write_videofile(os.path.join(TMP_DIR, "crop4.webm")) + clip4=crop(clip, x1=10, width=200) # crop a rect that has width=200 + clip4.write_videofile(os.path.join(TMP_DIR, "crop4.webm")) - clip5=crop(clip, x_center=300, y_center=400, width=50, height=150) - clip5.write_videofile(os.path.join(TMP_DIR, "crop5.webm")) + clip5=crop(clip, x_center=300, y_center=400, width=50, height=150) + clip5.write_videofile(os.path.join(TMP_DIR, "crop5.webm")) - clip6=crop(clip, x_center=300, width=400, y1=100, y2=600) - clip6.write_videofile(os.path.join(TMP_DIR, "crop6.webm")) + clip6=crop(clip, x_center=300, width=400, y1=100, y2=600) + clip6.write_videofile(os.path.join(TMP_DIR, "crop6.webm")) def test_fadein(): - clip = VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,5) - clip1 = fadein(clip, 1) - clip1.write_videofile(os.path.join(TMP_DIR,"fadein1.webm")) + with VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,5) as clip: + clip1 = fadein(clip, 1) + clip1.write_videofile(os.path.join(TMP_DIR,"fadein1.webm")) def test_fadeout(): - clip = VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,5) - clip1 = fadeout(clip, 1) - clip1.write_videofile(os.path.join(TMP_DIR,"fadeout1.webm")) + with VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,5) as clip: + clip1 = fadeout(clip, 1) + clip1.write_videofile(os.path.join(TMP_DIR,"fadeout1.webm")) if __name__ == '__main__': diff --git a/tests/test_helper.py b/tests/test_helper.py index 7913b44e9..1dd6a71c1 100644 --- a/tests/test_helper.py +++ b/tests/test_helper.py @@ -2,7 +2,17 @@ """Define general test helper attributes and utilities.""" import os import sys +import tempfile -TRAVIS=os.getenv("TRAVIS_PYTHON_VERSION") is not None +TRAVIS = os.getenv("TRAVIS_PYTHON_VERSION") is not None PYTHON_VERSION = "%s.%s" % (sys.version_info.major, sys.version_info.minor) -TMP_DIR="/tmp" +TMP_DIR = tempfile.tempdir + +# Arbitrary font used in caption testing. +if sys.platform in ("win32", "cygwin"): + FONT = "Arial" + # Even if Windows users install the Liberation fonts, it is called LiberationMono on Windows, so + # it doesn't help. +else: + FONT = "Liberation-Mono" # This is available in the fonts-liberation package on Linux. + diff --git a/tests/test_issues.py b/tests/test_issues.py index c125b8faf..1859eb3bb 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -18,9 +18,9 @@ def test_download_media(capsys): download_media.download() def test_issue_145(): - video = ColorClip((800, 600), color=(255,0,0)).set_duration(5) - with pytest.raises(Exception, message='Expecting Exception'): - concatenate_videoclips([video], method='composite') + with ColorClip((800, 600), color=(255,0,0)).set_duration(5) as video: + with pytest.raises(Exception, message='Expecting Exception'): + concatenate_videoclips([video], method='composite') def test_issue_190(): #from PIL import Image @@ -40,6 +40,9 @@ def test_issue_285(): ImageClip('media/python_logo.png', duration=10) merged_clip = concatenate_videoclips([clip_1, clip_2, clip_3]) assert merged_clip.duration == 30 + clip_1.close() + clip_2.close() + clip_3.close() def test_issue_334(): last_move = None @@ -126,41 +129,41 @@ def size(t): return (nsw, nsh) return (last_move1[3], last_move1[3] * 1.33) - avatar = VideoFileClip("media/big_buck_bunny_432_433.webm", has_mask=True) - avatar.audio=None - maskclip = ImageClip("media/afterimage.png", ismask=True, transparent=True) - avatar.set_mask(maskclip) #must set maskclip here.. + with VideoFileClip("media/big_buck_bunny_432_433.webm", has_mask=True) as avatar: + avatar.audio=None + maskclip = ImageClip("media/afterimage.png", ismask=True, transparent=True) + avatar.set_mask(maskclip) #must set maskclip here.. - avatar = concatenate_videoclips([avatar]*11) + concatenated = concatenate_videoclips([avatar]*11) - tt = VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,11) - # TODO: Setting mask here does not work: .set_mask(maskclip).resize(size)]) - final = CompositeVideoClip([tt, avatar.set_position(posi).resize(size)]) - final.duration = tt.duration - final.write_videofile(os.path.join(TMP_DIR, 'issue_334.mp4'), fps=24) + with VideoFileClip("media/big_buck_bunny_0_30.webm").subclip(0,11) as tt: + # TODO: Setting mask here does not work: .set_mask(maskclip).resize(size)]) + final = CompositeVideoClip([tt, concatenated.set_position(posi).resize(size)]) + final.duration = tt.duration + final.write_videofile(os.path.join(TMP_DIR, 'issue_334.mp4'), fps=24) def test_issue_354(): - clip = ImageClip("media/python_logo.png") + with ImageClip("media/python_logo.png") as clip: - clip.duration = 10 - crosstime = 1 + clip.duration = 10 + crosstime = 1 - # TODO: Should this be removed? - # caption = editor.TextClip("test text", font="Liberation-Sans-Bold", - # color='white', stroke_color='gray', - # stroke_width=2, method='caption', - # size=(1280, 720), fontsize=60, - # align='South-East') - #caption.duration = clip.duration + # TODO: Should this be removed? + # caption = editor.TextClip("test text", font="Liberation-Sans-Bold", + # color='white', stroke_color='gray', + # stroke_width=2, method='caption', + # size=(1280, 720), fontsize=60, + # align='South-East') + #caption.duration = clip.duration - fadecaption = clip.crossfadein(crosstime).crossfadeout(crosstime) - CompositeVideoClip([clip, fadecaption]) + fadecaption = clip.crossfadein(crosstime).crossfadeout(crosstime) + CompositeVideoClip([clip, fadecaption]).close() def test_issue_359(): - video = ColorClip((800, 600), color=(255,0,0)).set_duration(5) - video.fps=30 - video.write_gif(filename=os.path.join(TMP_DIR, "issue_359.gif"), - tempfiles=True) + with ColorClip((800, 600), color=(255,0,0)).set_duration(5) as video: + video.fps=30 + video.write_gif(filename=os.path.join(TMP_DIR, "issue_359.gif"), + tempfiles=True) # TODO: Debug matplotlib failures following successful travis builds. # def test_issue_368(): diff --git a/tests/test_misc.py b/tests/test_misc.py index a375900ad..7d79e4042 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -10,7 +10,7 @@ from moviepy.video.io.VideoFileClip import VideoFileClip import download_media -from test_helper import TMP_DIR, TRAVIS +from test_helper import TMP_DIR, TRAVIS, FONT sys.path.append("tests") @@ -20,8 +20,8 @@ def test_download_media(capsys): download_media.download() def test_cuts1(): - clip = VideoFileClip("media/big_buck_bunny_432_433.webm").resize(0.2) - cuts.find_video_period(clip) == pytest.approx(0.966666666667, 0.0001) + with VideoFileClip("media/big_buck_bunny_432_433.webm").resize(0.2) as clip: + cuts.find_video_period(clip) == pytest.approx(0.966666666667, 0.0001) def test_subtitles(): red = ColorClip((800, 600), color=(255,0,0)).set_duration(10) @@ -35,14 +35,14 @@ def test_subtitles(): if TRAVIS: return - generator = lambda txt: TextClip(txt, font='Liberation-Mono', + generator = lambda txt: TextClip(txt, font=FONT, size=(800,600), fontsize=24, method='caption', align='South', color='white') subtitles = SubtitlesClip("media/subtitles1.srt", generator) - final = CompositeVideoClip([myvideo, subtitles]) - final.to_videofile(os.path.join(TMP_DIR, "subtitles1.mp4"), fps=30) + with CompositeVideoClip([myvideo, subtitles]) as final: + final.to_videofile(os.path.join(TMP_DIR, "subtitles1.mp4"), fps=30) data = [([0.0, 4.0], 'Red!'), ([5.0, 9.0], 'More Red!'), ([10.0, 14.0], 'Green!'), ([15.0, 19.0], 'More Green!'), diff --git a/tests/test_resourcerelease.py b/tests/test_resourcerelease.py new file mode 100644 index 000000000..98c9eb0cd --- /dev/null +++ b/tests/test_resourcerelease.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- + +""" + Tool tests meant to be run with pytest. + + Testing whether issue #596 has been repaired. + + Note: Platform dependent test. Will only fail on Windows > NT. """ + +from os import remove +from os.path import join +import subprocess as sp +import time +# from tempfile import NamedTemporaryFile +from test_helper import TMP_DIR + +from moviepy.video.compositing.CompositeVideoClip import clips_array +from moviepy.video.VideoClip import ColorClip +from moviepy.video.io.VideoFileClip import VideoFileClip + + +def test_release_of_file_via_close(): + # Create a random video file. + red = ColorClip((1024, 800), color=(255, 0, 0)) + green = ColorClip((1024, 800), color=(0, 255, 0)) + blue = ColorClip((1024, 800), color=(0, 0, 255)) + + red.fps = green.fps = blue.fps = 30 + + # Repeat this so we can see no conflicts. + for i in range(5): + # Get the name of a temporary file we can use. + local_video_filename = join(TMP_DIR, "test_release_of_file_via_close_%s.mp4" % int(time.time())) + + with clips_array([[red, green, blue]]) as ca: + video = ca.set_duration(1) + + video.write_videofile(local_video_filename) + + # Open it up with VideoFileClip. + with VideoFileClip(local_video_filename) as clip: + # Normally a client would do processing here. + pass + + # Now remove the temporary file. + # This would fail on Windows if the file is still locked. + + # This should succeed without exceptions. + remove(local_video_filename) + + red.close() + green.close() + blue.close() diff --git a/tests/test_resourcereleasedemo.py b/tests/test_resourcereleasedemo.py new file mode 100644 index 000000000..51ba6dc6b --- /dev/null +++ b/tests/test_resourcereleasedemo.py @@ -0,0 +1,75 @@ +# -*- coding: utf-8 -*- + +""" + Tool tests meant to be run with pytest. + + Demonstrates issue #596 exists. + + Note: Platform dependent test. Will only fail on Windows > NT. """ + +from os import remove +from os.path import join +import subprocess as sp +import time +# from tempfile import NamedTemporaryFile +from test_helper import TMP_DIR + +from moviepy.video.compositing.CompositeVideoClip import clips_array +from moviepy.video.VideoClip import ColorClip +from moviepy.video.io.VideoFileClip import VideoFileClip +# import pytest + +def test_failure_to_release_file(): + + """ This isn't really a test, because it is expected to fail. + It demonstrates that there *is* a problem with not releasing resources when running on + Windows. + + The real issue was that, as of movepy 0.2.3.2, there was no way around it. + + See test_resourcerelease.py to see how the close() methods provide a solution. + """ + + # Get the name of a temporary file we can use. + local_video_filename = join(TMP_DIR, "test_release_of_file_%s.mp4" % int(time.time())) + + # Repeat this so we can see that the problems escalate: + for i in range(5): + + # Create a random video file. + red = ColorClip((1024,800), color=(255,0,0)) + green = ColorClip((1024,800), color=(0,255,0)) + blue = ColorClip((1024,800), color=(0,0,255)) + + red.fps = green.fps = blue.fps = 30 + video = clips_array([[red, green, blue]]).set_duration(1) + + try: + video.write_videofile(local_video_filename) + + # Open it up with VideoFileClip. + clip = VideoFileClip(local_video_filename) + + # Normally a client would do processing here. + + # All finished, so delete the clipS. + del clip + del video + + except IOError: + print("On Windows, this succeeds the first few times around the loop, but eventually fails.") + print("Need to shut down the process now. No more tests in this file.") + return + + + try: + # Now remove the temporary file. + # This will fail on Windows if the file is still locked. + + # In particular, this raises an exception with PermissionError. + # In there was no way to avoid it. + + remove(local_video_filename) + print("You are not running Windows, because that worked.") + except PermissionError: + print("Yes, on Windows this fails.") diff --git a/tests/test_tools.py b/tests/test_tools.py index b1510bb6c..2c276b4a4 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -69,17 +69,5 @@ def test_5(): file = sys.stdout.read() assert file == b"" -def test_6(): - """Test subprocess_call for operation. - - The process sleep should run for a given time in seconds. - This checks that the process has deallocated from the stack on - completion of the called process. - - """ - process = tools.subprocess_call(["sleep" , '1']) - time.sleep(1) - assert process is None - if __name__ == '__main__': pytest.main()