From dc1326272f4e08171c5bef52ebbbb2763bc6be7a Mon Sep 17 00:00:00 2001 From: Keith Feldman Date: Sat, 30 Jan 2021 17:19:26 -0500 Subject: [PATCH 1/2] Additional video options Adding in the ability to do 2-pass encoding Allowing user to disable video resizing Allowing user to force a video encode even within the same codec --- .gitignore | 2 + AUTHORS | 1 + sigal/settings.py | 5 ++ sigal/templates/sigal.conf.py | 21 +++++++ sigal/video.py | 113 +++++++++++++++++++++++++--------- tests/test_video.py | 39 +++++++++--- 6 files changed, 145 insertions(+), 36 deletions(-) diff --git a/.gitignore b/.gitignore index dd49f76..fc983c5 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,5 @@ htmlcov/ output/ sigal/version.py tags +venv +ffmpeg2pass-0.log diff --git a/AUTHORS b/AUTHORS index 8b8b7b5..bf3cc0b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -29,6 +29,7 @@ alphabetical order): - Juan A. Suarez Romero - Julien Voisin - Kai Fricke +- Keith Feldman - Keith Johnson - Kevin Murray - Lukas Vacek diff --git a/sigal/settings.py b/sigal/settings.py index 2d655df..20f0a5c 100644 --- a/sigal/settings.py +++ b/sigal/settings.py @@ -1,6 +1,7 @@ # Copyright (c) 2009-2020 - Simon Conseil # Copyright (c) 2013 - Christophe-Marie Duquesne # Copyright (c) 2017 - Mate Lakat +# Copyright (c) 2021 - Keith Feldman # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to @@ -54,6 +55,7 @@ _DEFAULT_CONFIG = { 'medias_sort_attr': 'filename', 'medias_sort_reverse': False, 'mp4_options': ['-crf', '23', '-strict', '-2'], + 'mp4_options_second_pass': None, 'orig_dir': 'original', 'orig_link': False, 'rel_link': False, @@ -78,10 +80,13 @@ _DEFAULT_CONFIG = { 'video_converter': 'ffmpeg', 'video_extensions': ['.mov', '.avi', '.mp4', '.webm', '.ogv', '.3gp'], 'video_format': 'webm', + 'video_resize': True, + 'video_always_convert': False, 'video_size': (480, 360), 'watermark': '', 'webm_options': ['-crf', '10', '-b:v', '1.6M', '-qmin', '4', '-qmax', '63'], + 'webm_options_second_pass': None, 'write_html': True, 'zip_gallery': False, 'zip_media_format': 'resized', diff --git a/sigal/templates/sigal.conf.py b/sigal/templates/sigal.conf.py index 3481768..175d1c7 100644 --- a/sigal/templates/sigal.conf.py +++ b/sigal/templates/sigal.conf.py @@ -171,14 +171,35 @@ ignore_files = [] # webm_options = ['-crf', '10', '-b:v', '1.6M', # '-qmin', '4', '-qmax', '63'] +# Webm options for 2-pass encoding +# Options used to encode the webm video on the second pass. +# Set to None by default, set to an array if a second pass is desired. +# webm_options_second_pass = None + + # MP4 options # Options used to encode the mp4 video. You may want to read # https://trac.ffmpeg.org/wiki/Encode/H.264 # mp4_options = ['-crf', '23' ] +# MP4 options for 2-pass encoding +# Options used to encode the mp4 video on the second pass. +# Set to None by default, set to an array if a second pass is desired. +# mp4_options_second_pass = None + + # Size of resized video (default: (480, 360)) # video_size = (480, 360) +# If the desired video extension and filename are the same, the video will +# not be converted. If a transcode to different quality is required, +# set this to True to force convert it. False by default. +# video_always_convert = False + +# Set this to false if no resizing is desired on the video. This overrides +# the video_size option. +# video_resize = True + # ------------- # Miscellaneous # ------------- diff --git a/sigal/video.py b/sigal/video.py index a595e8a..6b70c44 100644 --- a/sigal/video.py +++ b/sigal/video.py @@ -1,5 +1,6 @@ # Copyright (c) 2013 - Christophe-Marie Duquesne # Copyright (c) 2013-2020 - Simon Conseil +# Copyright (c) 2021 - Keith Feldman # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to @@ -35,7 +36,7 @@ class SubprocessException(Exception): pass -def check_subprocess(cmd, source, outname): +def check_subprocess(cmd, source, outname=None): """Run the command to resize the video and remove the output file if the processing fails. @@ -46,22 +47,21 @@ def check_subprocess(cmd, source, outname): stderr=subprocess.PIPE) except KeyboardInterrupt: logger.debug('Process terminated, removing file %s', outname) - if os.path.isfile(outname): + if outname and os.path.isfile(outname): os.remove(outname) raise if res.returncode: logger.debug('STDOUT:\n %s', res.stdout.decode('utf8')) logger.debug('STDERR:\n %s', res.stderr.decode('utf8')) - if os.path.isfile(outname): + if outname and os.path.isfile(outname): logger.debug('Removing file %s', outname) os.remove(outname) raise SubprocessException('Failed to process ' + source) def video_size(source, converter='ffmpeg'): - """Returns the dimensions of the video.""" - + """Return the dimensions of the video.""" res = subprocess.run([converter, '-i', source], stderr=subprocess.PIPE) stderr = res.stderr.decode('utf8') pattern = re.compile(r'Stream.*Video.* ([0-9]+)x([0-9]+)') @@ -78,30 +78,36 @@ def video_size(source, converter='ffmpeg'): return x, y -def generate_video(source, outname, settings, options=None): - """Video processor. +def get_dimensions(source, converter, output_size): + """Figure out src and dest width and height for video. :param source: path to a video :param outname: path to the generated video :param settings: settings dict - :param options: array of options passed to ffmpeg """ logger = logging.getLogger(__name__) # Don't transcode if source is in the required format and # has fitting datedimensions, copy instead. - converter = settings['video_converter'] w_src, h_src = video_size(source, converter=converter) - w_dst, h_dst = settings['video_size'] + w_dst, h_dst = output_size logger.debug('Video size: %i, %i -> %i, %i', w_src, h_src, w_dst, h_dst) + return {'src': (w_src, h_src), 'dst': (w_dst, h_dst)} - base, src_ext = splitext(source) - base, dst_ext = splitext(outname) - if dst_ext == src_ext and w_src <= w_dst and h_src <= h_dst: - logger.debug('Video is smaller than the max size, copying it instead') - shutil.copy(source, outname) - return + +def get_resize_options(src_dst_dimension_dict): + """Figure out resize options for video from src and dst sizes. + + :param src_dst_dimension_dict: a dictionary of src and dst, + each with a width, height tuple + """ + w_src, h_src = src_dst_dimension_dict['src'] + w_dst, h_dst = src_dst_dimension_dict['dst'] + + # do not resize if input dimensions are smaller than output dimensions + if w_src <= w_dst and h_src <= h_dst: + return [] # http://stackoverflow.com/questions/8218363/maintaining-ffmpeg-aspect-ratio # + I made a drawing on paper to figure this out @@ -112,25 +118,78 @@ def generate_video(source, outname, settings, options=None): # biggest fitting dimension is width resize_opt = ['-vf', "scale=%i:trunc(ow/a/2)*2" % w_dst] - # do not resize if input dimensions are smaller than output dimensions - if w_src <= w_dst and h_src <= h_dst: - resize_opt = [] + return resize_opt + + +def _get_empty_if_none_else_variable(variable): + return [] if not variable else variable + +def generate_video_pass(converter, source, options, outname=None): + """Run a single pass of encoding. + + :param source: source video + :param options: options to pass to encoder + :param outname: if multi-pass, this is None on the first pass + """ + logger = logging.getLogger(__name__) + outname_opt = [] if not outname else [outname] # Encoding options improved, thanks to # http://ffmpeg.org/trac/ffmpeg/wiki/vpxEncodingGuide cmd = [converter, '-i', source, '-y'] # -y to overwrite output files - if options is not None: - cmd += options - cmd += resize_opt + [outname] - + cmd += options + outname_opt logger.debug('Processing video: %s', ' '.join(cmd)) - check_subprocess(cmd, source, outname) + check_subprocess(cmd, source, outname=outname) + + +def generate_video(source, outname, settings): + """Video processor. + + :param source: path to a video + :param outname: path to the generated video + :param settings: settings dict + :param options: array of options passed to ffmpeg + + """ + logger = logging.getLogger(__name__) + + video_format = settings.get('video_format') + options = settings.get(video_format + '_options') + second_pass_options = settings.get(video_format + '_options_second_pass') + video_always_convert = settings.get('video_always_convert') + converter = settings['video_converter'] + + resize_opt = [] + if settings.get("video_resize"): + src_dst_dimension_dict = get_dimensions(source, converter, + settings['video_size']) + resize_opt = get_resize_options(src_dst_dimension_dict) + + base, src_ext = splitext(source) + base, dst_ext = splitext(outname) + + if dst_ext == src_ext and not resize_opt and not video_always_convert: + logger.debug('For %s, the source and destination extension are the " \ + "same, there is no resizing to be done, and " \ + "video_always_convert is False, so the output is " \ + " being copied', outname) + shutil.copy(source, outname) + return + + final_pass_options = _get_empty_if_none_else_variable(options) + resize_opt + if second_pass_options: + generate_video_pass(converter, source, final_pass_options) + final_second_pass_options = _get_empty_if_none_else_variable( + second_pass_options) + resize_opt + generate_video_pass(converter, source, + final_second_pass_options, outname) + else: + generate_video_pass(converter, source, final_pass_options, outname) def generate_thumbnail(source, outname, box, delay, fit=True, options=None, converter='ffmpeg'): """Create a thumbnail image for the video source, based on ffmpeg.""" - logger = logging.getLogger(__name__) tmpfile = outname + ".tmp.jpg" @@ -159,7 +218,6 @@ def generate_thumbnail(source, outname, box, delay, fit=True, options=None, def process_video(filepath, outpath, settings): """Process a video: resize, create thumbnail.""" - logger = logging.getLogger(__name__) filename = os.path.split(filepath)[1] basename, ext = splitext(filename) @@ -178,8 +236,7 @@ def process_video(filepath, outpath, settings): raise ValueError outname = os.path.join(outpath, basename + '.' + video_format) - generate_video(filepath, outname, settings, - options=settings.get(video_format + '_options')) + generate_video(filepath, outname, settings) except Exception: if logger.getEffectiveLevel() == logging.DEBUG: raise diff --git a/tests/test_video.py b/tests/test_video.py index 4b17b6c..ab2a1fa 100644 --- a/tests/test_video.py +++ b/tests/test_video.py @@ -4,7 +4,8 @@ import pytest from sigal.settings import Status, create_settings from sigal.video import (generate_thumbnail, generate_video, process_video, - video_size) + video_size, get_resize_options, generate_video_pass) +from unittest.mock import patch CURRENT_DIR = os.path.dirname(__file__) TEST_VIDEO = 'example video.ogv' @@ -47,8 +48,7 @@ def test_generate_video_fit_height(tmpdir, fmt): base, ext = os.path.splitext(TEST_VIDEO) dstfile = str(tmpdir.join(base + '.' + fmt)) settings = create_settings(video_size=(80, 100), video_format=fmt) - generate_video(SRCFILE, dstfile, settings, - options=settings[fmt + '_options']) + generate_video(SRCFILE, dstfile, settings) size_src = video_size(SRCFILE) size_dst = video_size(dstfile) @@ -65,8 +65,7 @@ def test_generate_video_fit_width(tmpdir, fmt): base, ext = os.path.splitext(TEST_VIDEO) dstfile = str(tmpdir.join(base + '.' + fmt)) settings = create_settings(video_size=(100, 50), video_format=fmt) - generate_video(SRCFILE, dstfile, settings, - options=settings[fmt + '_options']) + generate_video(SRCFILE, dstfile, settings) size_src = video_size(SRCFILE) size_dst = video_size(dstfile) @@ -78,14 +77,38 @@ def test_generate_video_fit_width(tmpdir, fmt): @pytest.mark.parametrize("fmt", ['webm', 'mp4', 'ogv']) def test_generate_video_dont_enlarge(tmpdir, fmt): - """video dimensions should not be enlarged""" + """Video dimensions should not be enlarged.""" base, ext = os.path.splitext(TEST_VIDEO) dstfile = str(tmpdir.join(base + '.' + fmt)) settings = create_settings(video_size=(1000, 1000), video_format=fmt) - generate_video(SRCFILE, dstfile, settings, - options=settings.get(fmt + '_options')) + generate_video(SRCFILE, dstfile, settings) size_src = video_size(SRCFILE) size_dst = video_size(dstfile) assert size_src == size_dst + + +@patch('sigal.video.generate_video_pass') +@pytest.mark.parametrize("fmt", ['webm', 'mp4']) +def test_second_pass_video(mock_generate_video_pass, fmt, tmpdir): + """Video should be run through ffmpeg.""" + base, ext = os.path.splitext(TEST_VIDEO) + dstfile = str(tmpdir.join(base + '.' + fmt)) + settings_1 = '-c:v libvpx-vp9 -b:v 0 -crf 30 -pass 1 -an -f null dev/null' + settings_2 = '-c:v libvpx-vp9 -b:v 0 -crf 30 -pass 2 -f {}'.format(fmt) + settings_opts = {'video_size': (100, 50), 'video_format': fmt, + fmt + '_options': settings_1.split(" "), + fmt + '_options_second_pass': settings_2.split(" ")} + + settings = create_settings(**settings_opts) + generate_video(SRCFILE, dstfile, settings) + call_args_list = mock_generate_video_pass.call_args_list + # The method is called twice + assert len(call_args_list) == 2 + # The first call to the method should have 3 args, without the outname + args, kwargs = call_args_list[0] + assert len(args) == 3 + # The second call to the method should have 4 args, with the outname + args, kwargs = call_args_list[1] + assert len(args) == 4 From 76bc8afe89d92c29a724151d0798b27df5be7f05 Mon Sep 17 00:00:00 2001 From: Keith Feldman Date: Sat, 6 Feb 2021 17:19:09 -0500 Subject: [PATCH 2/2] Recommended fixes Using video_size = None to prevent resizing instead of another option Eliminating pointless method for just getting resize_dimensions --- sigal/settings.py | 1 - sigal/templates/sigal.conf.py | 5 +---- sigal/video.py | 27 +++++---------------------- 3 files changed, 6 insertions(+), 27 deletions(-) diff --git a/sigal/settings.py b/sigal/settings.py index 20f0a5c..7df6324 100644 --- a/sigal/settings.py +++ b/sigal/settings.py @@ -80,7 +80,6 @@ _DEFAULT_CONFIG = { 'video_converter': 'ffmpeg', 'video_extensions': ['.mov', '.avi', '.mp4', '.webm', '.ogv', '.3gp'], 'video_format': 'webm', - 'video_resize': True, 'video_always_convert': False, 'video_size': (480, 360), 'watermark': '', diff --git a/sigal/templates/sigal.conf.py b/sigal/templates/sigal.conf.py index 175d1c7..7518d7a 100644 --- a/sigal/templates/sigal.conf.py +++ b/sigal/templates/sigal.conf.py @@ -189,6 +189,7 @@ ignore_files = [] # Size of resized video (default: (480, 360)) +# Set this to None if no resizing is desired on the video. # video_size = (480, 360) # If the desired video extension and filename are the same, the video will @@ -196,10 +197,6 @@ ignore_files = [] # set this to True to force convert it. False by default. # video_always_convert = False -# Set this to false if no resizing is desired on the video. This overrides -# the video_size option. -# video_resize = True - # ------------- # Miscellaneous # ------------- diff --git a/sigal/video.py b/sigal/video.py index 6b70c44..4b66f61 100644 --- a/sigal/video.py +++ b/sigal/video.py @@ -77,33 +77,17 @@ def video_size(source, converter='ffmpeg'): x, y = y, x return x, y - -def get_dimensions(source, converter, output_size): - """Figure out src and dest width and height for video. +def get_resize_options(source, converter, output_size): + """Figure out resize options for video from src and dst sizes. :param source: path to a video :param outname: path to the generated video :param settings: settings dict - """ logger = logging.getLogger(__name__) - - # Don't transcode if source is in the required format and - # has fitting datedimensions, copy instead. w_src, h_src = video_size(source, converter=converter) w_dst, h_dst = output_size logger.debug('Video size: %i, %i -> %i, %i', w_src, h_src, w_dst, h_dst) - return {'src': (w_src, h_src), 'dst': (w_dst, h_dst)} - - -def get_resize_options(src_dst_dimension_dict): - """Figure out resize options for video from src and dst sizes. - - :param src_dst_dimension_dict: a dictionary of src and dst, - each with a width, height tuple - """ - w_src, h_src = src_dst_dimension_dict['src'] - w_dst, h_dst = src_dst_dimension_dict['dst'] # do not resize if input dimensions are smaller than output dimensions if w_src <= w_dst and h_src <= h_dst: @@ -160,10 +144,9 @@ def generate_video(source, outname, settings): converter = settings['video_converter'] resize_opt = [] - if settings.get("video_resize"): - src_dst_dimension_dict = get_dimensions(source, converter, - settings['video_size']) - resize_opt = get_resize_options(src_dst_dimension_dict) + if settings.get("video_size"): + resize_opt = get_resize_options(source, converter, + settings['video_size']) base, src_ext = splitext(source) base, dst_ext = splitext(outname)