From 48e47ad31b499cec0be80c12f5346d5e144410db Mon Sep 17 00:00:00 2001 From: Chad von Nau Date: Sat, 2 Dec 2017 20:11:10 -0800 Subject: [PATCH] Fix stretched vertical video, add ffmpeg_params to VideoFileClip - add test to determine whether ffmpeg supports autorotate - FFMPEG_VideoReader: set size based on video rotation metadata - use `-noautorotate` to disable autorotation (ffmpeg >=2.7 autorotates by default) - closes #212, closes #577, closes #682 --- moviepy/config.py | 18 ++++++++++++++++-- moviepy/video/io/VideoFileClip.py | 12 ++++++++++-- moviepy/video/io/ffmpeg_reader.py | 14 ++++++++++++-- tests/download_media.py | 3 ++- tests/test_VideoFileClip.py | 13 +++++++++++++ tests/test_ffmpeg_reader.py | 20 +++++++++++++++++--- 6 files changed, 70 insertions(+), 10 deletions(-) diff --git a/moviepy/config.py b/moviepy/config.py index 5a8451607..870677cf9 100644 --- a/moviepy/config.py +++ b/moviepy/config.py @@ -24,11 +24,11 @@ def try_cmd(cmd): popen_params["creationflags"] = 0x08000000 proc = sp.Popen(cmd, **popen_params) - proc.communicate() + stdout, stderr = proc.communicate() except Exception as err: return False, err else: - return True, None + return True, stderr if FFMPEG_BINARY=='ffmpeg-imageio': from imageio.plugins.ffmpeg import get_exe @@ -49,6 +49,15 @@ def try_cmd(cmd): str(err) + " - The path specified for the ffmpeg binary might be wrong") +# ffmpeg >=2.7 autorotates videos with rotation metadata. Detect this by testing +# if '-noautorotate' is a recognized option. +success, err = try_cmd([FFMPEG_BINARY, '-noautorotate']) +if success: + FFMPEG_SUPPORTS_AUTOROTATE = not 'Unrecognized option' in str(err) +else: + FFMPEG_SUPPORTS_AUTOROTATE = False + + if IMAGEMAGICK_BINARY=='auto-detect': if os.name == 'nt': try: @@ -99,6 +108,11 @@ def change_settings(new_settings=None, file=None): else: print( "MoviePy : can't find or access ffmpeg." ) + if FFMPEG_SUPPORTS_AUTOROTATE: + print( "MoviePy : ffmpeg supports autorotate." ) + else: + print( "MoviePy : ffmpeg does not support autorotate." ) + if try_cmd([IMAGEMAGICK_BINARY])[0]: print( "MoviePy : ImageMagick successfully found." ) else: diff --git a/moviepy/video/io/VideoFileClip.py b/moviepy/video/io/VideoFileClip.py index a5e300c40..3ff0adbdf 100644 --- a/moviepy/video/io/VideoFileClip.py +++ b/moviepy/video/io/VideoFileClip.py @@ -51,6 +51,10 @@ class VideoFileClip(VideoClip): can be set to 'fps', which may be helpful if importing slow-motion videos that get messed up otherwise. + ffmpeg_params: + Any additional ffmpeg parameters to pass when reading files, as a list of + terms, like ['-noautorotate']. + Attributes ----------- @@ -60,6 +64,9 @@ class VideoFileClip(VideoClip): fps: Frames per second in the original file. + + rotation: + Rotation metadata, in degrees. Read docs for Clip() and VideoClip() for other, more generic, attributes. @@ -78,7 +85,7 @@ def __init__(self, filename, has_mask=False, audio=True, audio_buffersize = 200000, target_resolution=None, resize_algorithm='bicubic', audio_fps=44100, audio_nbytes=2, verbose=False, - fps_source='tbr'): + fps_source='tbr', ffmpeg_params=None): VideoClip.__init__(self) @@ -87,7 +94,8 @@ def __init__(self, filename, has_mask=False, self.reader = FFMPEG_VideoReader(filename, pix_fmt=pix_fmt, target_resolution=target_resolution, resize_algo=resize_algorithm, - fps_source=fps_source) + fps_source=fps_source, + ffmpeg_params=ffmpeg_params) # Make some of the reader's attributes accessible from the clip self.duration = self.reader.duration diff --git a/moviepy/video/io/ffmpeg_reader.py b/moviepy/video/io/ffmpeg_reader.py index 0a26a88aa..8941da64b 100644 --- a/moviepy/video/io/ffmpeg_reader.py +++ b/moviepy/video/io/ffmpeg_reader.py @@ -25,7 +25,7 @@ class FFMPEG_VideoReader: def __init__(self, filename, print_infos=False, bufsize = None, pix_fmt="rgb24", check_duration=True, target_resolution=None, resize_algo='bicubic', - fps_source='tbr'): + fps_source='tbr', ffmpeg_params=None): self.filename = filename self.proc = None @@ -34,6 +34,13 @@ def __init__(self, filename, print_infos=False, bufsize = None, self.fps = infos['video_fps'] self.size = infos['video_size'] self.rotation = infos['video_rotation'] + self.ffmpeg_params = ffmpeg_params or [] + + autorotate = get_setting('FFMPEG_SUPPORTS_AUTOROTATE') \ + and not '-noautorotate' in self.ffmpeg_params + if autorotate and abs(self.rotation) in [90, 270]: + # swap dimensions to preserve aspect ratio after autorotate + self.size = self.size[::-1] if target_resolution: # revert the order, as ffmpeg used (width, height) @@ -86,6 +93,9 @@ def initialize(self, starttime=0): else: i_arg = [ '-i', self.filename] + if self.ffmpeg_params: + i_arg = self.ffmpeg_params + i_arg + cmd = ([get_setting("FFMPEG_BINARY")] + i_arg + ['-loglevel', 'error', '-f', 'image2pipe', @@ -231,7 +241,7 @@ def ffmpeg_parse_infos(filename, print_infos=False, check_duration=True, Returns a dictionnary with the fields: "video_found", "video_fps", "duration", "video_nframes", - "video_duration", "audio_found", "audio_fps" + "video_duration", "video_rotation", "audio_found", "audio_fps" "video_duration" is slightly smaller than "duration" to avoid fetching the uncomplete frames at the end, which raises an error. diff --git a/tests/download_media.py b/tests/download_media.py index 89ef7cc19..bee94cd46 100644 --- a/tests/download_media.py +++ b/tests/download_media.py @@ -30,7 +30,8 @@ def download(): '/sounds/crunching.mp3', '/images/pigs_in_a_polka.gif', '/videos/fire2.mp4', '/videos/big_buck_bunny_0_30.webm', '/subtitles/subtitles1.srt', '/misc/traj.txt', - '/images/vacation_2017.jpg', '/images/python_logo_upside_down.png'] + '/images/vacation_2017.jpg', '/images/python_logo_upside_down.png', + '/videos/ficus_vertical.mp4'] # Loop through download url strings, build out path, and download the asset. for url in urls: diff --git a/tests/test_VideoFileClip.py b/tests/test_VideoFileClip.py index 584e5235e..d93178aea 100644 --- a/tests/test_VideoFileClip.py +++ b/tests/test_VideoFileClip.py @@ -56,6 +56,19 @@ def test_ffmpeg_resizing(): frame = video.get_frame(0) assert frame.shape[1] == target_resolution[1] +def test_ffmpeg_resizing_with_autorotate(): + # This test requires ffmpeg >=2.7 + video_file = 'media/ficus_vertical.mp4' + target_resolution=(720, None) + with VideoFileClip(video_file, target_resolution=target_resolution) as video: + frame = video.get_frame(0) + assert frame.shape[0:2] == (720, 405) + + with VideoFileClip(video_file, target_resolution=target_resolution, + ffmpeg_params=['-noautorotate']) as video: + frame = video.get_frame(0) + assert frame.shape[0:2] == (720, 1280) + if __name__ == '__main__': pytest.main() diff --git a/tests/test_ffmpeg_reader.py b/tests/test_ffmpeg_reader.py index 5f0d381e0..8613adbcd 100644 --- a/tests/test_ffmpeg_reader.py +++ b/tests/test_ffmpeg_reader.py @@ -3,12 +3,12 @@ import sys import pytest -from moviepy.video.io.ffmpeg_reader import ffmpeg_parse_infos - -import download_media +from moviepy.video.io.ffmpeg_reader import ffmpeg_parse_infos, FFMPEG_VideoReader sys.path.append("tests") +import download_media + def test_download_media(capsys): with capsys.disabled(): @@ -22,6 +22,20 @@ def test_ffmpeg_parse_infos(): assert d['video_size'] == [314, 273] assert d['duration'] == 3.0 +def test_autorotate(): + # This test requires ffmpeg >=2.7 + video_file = 'media/ficus_vertical.mp4' + reader = FFMPEG_VideoReader(video_file) + assert reader.infos['video_size'] == [1920, 1080] + assert reader.infos['video_rotation'] == 90 + assert reader.size == [1080, 1920] + reader.close() + + reader = FFMPEG_VideoReader(video_file, ffmpeg_params=['-noautorotate']) + assert reader.size == [1920, 1080] + assert reader.rotation == 90 + reader.close() + if __name__ == '__main__': pytest.main()