From ad3dc496bbf2e2a574a16244ddde0740778e5daf Mon Sep 17 00:00:00 2001 From: pukkandan Date: Mon, 9 Aug 2021 17:40:24 +0530 Subject: [PATCH] Misc fixes - See desc * Remove unnecessary uses of _list_from_options_callback * Fix download tests - Bug from 6e84b21559f586ee4d6affb61688d5c6a0c21221 * Rename ExecAfterDownloadPP to ExecPP and refactor its tests * Ensure _write_ytdl_file closes file handle on error - Potential fix for #517 --- test/test_download.py | 2 +- test/test_postprocessors.py | 14 +++++------ yt_dlp/YoutubeDL.py | 7 +++--- yt_dlp/__init__.py | 9 +++---- yt_dlp/downloader/fragment.py | 24 ++++++++++--------- yt_dlp/options.py | 13 ++++------ yt_dlp/postprocessor/__init__.py | 3 ++- .../{execafterdownload.py => exec.py} | 12 +++++----- 8 files changed, 43 insertions(+), 41 deletions(-) rename yt_dlp/postprocessor/{execafterdownload.py => exec.py} (87%) diff --git a/test/test_download.py b/test/test_download.py index f26fb23c0..d7c469f3d 100644 --- a/test/test_download.py +++ b/test/test_download.py @@ -147,7 +147,7 @@ def _hook(status): expect_warnings(ydl, test_case.get('expected_warnings', [])) def get_tc_filename(tc): - return ydl.prepare_filename(tc.get('info_dict', {})) + return ydl.prepare_filename(dict(tc.get('info_dict', {}))) res_dict = None diff --git a/test/test_postprocessors.py b/test/test_postprocessors.py index 320e69e88..b15cbd28c 100644 --- a/test/test_postprocessors.py +++ b/test/test_postprocessors.py @@ -11,7 +11,7 @@ from yt_dlp import YoutubeDL from yt_dlp.compat import compat_shlex_quote from yt_dlp.postprocessor import ( - ExecAfterDownloadPP, + ExecPP, FFmpegThumbnailsConvertorPP, MetadataFromFieldPP, MetadataParserPP, @@ -59,12 +59,12 @@ def test_escaping(self): os.remove(file.format(out)) -class TestExecAfterDownload(unittest.TestCase): +class TestExec(unittest.TestCase): def test_parse_cmd(self): - pp = ExecAfterDownloadPP(YoutubeDL(), '') + pp = ExecPP(YoutubeDL(), '') info = {'filepath': 'file name'} - quoted_filepath = compat_shlex_quote(info['filepath']) + cmd = 'echo %s' % compat_shlex_quote(info['filepath']) - self.assertEqual(pp.parse_cmd('echo', info), 'echo %s' % quoted_filepath) - self.assertEqual(pp.parse_cmd('echo.{}', info), 'echo.%s' % quoted_filepath) - self.assertEqual(pp.parse_cmd('echo "%(filepath)s"', info), 'echo "%s"' % info['filepath']) + self.assertEqual(pp.parse_cmd('echo', info), cmd) + self.assertEqual(pp.parse_cmd('echo {}', info), cmd) + self.assertEqual(pp.parse_cmd('echo %(filepath)q', info), cmd) diff --git a/yt_dlp/YoutubeDL.py b/yt_dlp/YoutubeDL.py index 7edae6fa2..5937e85bd 100644 --- a/yt_dlp/YoutubeDL.py +++ b/yt_dlp/YoutubeDL.py @@ -2339,7 +2339,8 @@ def process_subtitles(self, video_id, normal_subtitles, automatic_captions): requested_langs = ['en'] else: requested_langs = [list(all_sub_langs)[0]] - self.write_debug('Downloading subtitles: %s' % ', '.join(requested_langs)) + if requested_langs: + self.write_debug('Downloading subtitles: %s' % ', '.join(requested_langs)) formats_query = self.params.get('subtitlesformat', 'best') formats_preference = formats_query.split('/') if formats_query else [] @@ -3256,13 +3257,13 @@ def python_implementation(): from .postprocessor.embedthumbnail import has_mutagen from .cookies import SQLITE_AVAILABLE, KEYRING_AVAILABLE - lib_str = ', '.join(filter(None, ( + lib_str = ', '.join(sorted(filter(None, ( can_decrypt_frag and 'pycryptodome', has_websockets and 'websockets', has_mutagen and 'mutagen', SQLITE_AVAILABLE and 'sqlite', KEYRING_AVAILABLE and 'keyring', - ))) or 'none' + )))) or 'none' self._write_string('[debug] Optional libraries: %s\n' % lib_str) proxy_map = {} diff --git a/yt_dlp/__init__.py b/yt_dlp/__init__.py index 73e3f9f78..d6479b3ff 100644 --- a/yt_dlp/__init__.py +++ b/yt_dlp/__init__.py @@ -332,7 +332,8 @@ def validate_outtmpl(tmpl, msg): for k, tmpl in opts.outtmpl.items(): validate_outtmpl(tmpl, '%s output template' % k) - for tmpl in opts.forceprint: + opts.forceprint = opts.forceprint or [] + for tmpl in opts.forceprint or []: validate_outtmpl(tmpl, 'print template') if opts.extractaudio and not opts.keepvideo and opts.format is None: @@ -445,7 +446,7 @@ def report_conflict(arg1, arg2): # Must be after all other before_dl if opts.exec_before_dl_cmd: postprocessors.append({ - 'key': 'ExecAfterDownload', + 'key': 'Exec', 'exec_cmd': opts.exec_before_dl_cmd, 'when': 'before_dl' }) @@ -516,10 +517,10 @@ def report_conflict(arg1, arg2): # XAttrMetadataPP should be run after post-processors that may change file contents if opts.xattrs: postprocessors.append({'key': 'XAttrMetadata'}) - # ExecAfterDownload must be the last PP + # Exec must be the last PP if opts.exec_cmd: postprocessors.append({ - 'key': 'ExecAfterDownload', + 'key': 'Exec', 'exec_cmd': opts.exec_cmd, # Run this only after the files have been moved to their final locations 'when': 'after_move' diff --git a/yt_dlp/downloader/fragment.py b/yt_dlp/downloader/fragment.py index 88238b64d..1cc99a4e9 100644 --- a/yt_dlp/downloader/fragment.py +++ b/yt_dlp/downloader/fragment.py @@ -105,17 +105,19 @@ def _read_ytdl_file(self, ctx): def _write_ytdl_file(self, ctx): frag_index_stream, _ = sanitize_open(self.ytdl_filename(ctx['filename']), 'w') - downloader = { - 'current_fragment': { - 'index': ctx['fragment_index'], - }, - } - if 'extra_state' in ctx: - downloader['extra_state'] = ctx['extra_state'] - if ctx.get('fragment_count') is not None: - downloader['fragment_count'] = ctx['fragment_count'] - frag_index_stream.write(json.dumps({'downloader': downloader})) - frag_index_stream.close() + try: + downloader = { + 'current_fragment': { + 'index': ctx['fragment_index'], + }, + } + if 'extra_state' in ctx: + downloader['extra_state'] = ctx['extra_state'] + if ctx.get('fragment_count') is not None: + downloader['fragment_count'] = ctx['fragment_count'] + frag_index_stream.write(json.dumps({'downloader': downloader})) + finally: + frag_index_stream.close() def _download_fragment(self, ctx, frag_url, info_dict, headers=None, request_data=None): fragment_filename = '%s-Frag%d' % (ctx['tmpfilename'], ctx['fragment_index']) diff --git a/yt_dlp/options.py b/yt_dlp/options.py index f8cfdeb12..f2d5deb68 100644 --- a/yt_dlp/options.py +++ b/yt_dlp/options.py @@ -23,7 +23,7 @@ from .version import __version__ from .downloader.external import list_external_downloaders -from .postprocessor.ffmpeg import ( +from .postprocessor import ( FFmpegExtractAudioPP, FFmpegSubtitlesConvertorPP, FFmpegThumbnailsConvertorPP, @@ -803,9 +803,8 @@ def _dict_from_options_callback( action='store_true', dest='skip_download', default=False, help='Do not download the video but write all related files (Alias: --no-download)') verbosity.add_option( - '-O', '--print', metavar='TEMPLATE', - action='callback', dest='forceprint', type='str', default=[], - callback=_list_from_options_callback, callback_kwargs={'delim': None}, + '-O', '--print', + metavar='TEMPLATE', action='append', dest='forceprint', help=( 'Quiet, but print the given fields for each video. Simulate unless --no-simulate is used. ' 'Either a field name or same syntax as the output template can be used')) @@ -1276,8 +1275,7 @@ def _dict_from_options_callback( help='Location of the ffmpeg binary; either the path to the binary or its containing directory') postproc.add_option( '--exec', metavar='CMD', - action='callback', dest='exec_cmd', default=[], type='str', - callback=_list_from_options_callback, callback_kwargs={'delim': None}, + action='append', dest='exec_cmd', help=( 'Execute a command on the file after downloading and post-processing. ' 'Same syntax as the output template can be used to pass any field as arguments to the command. ' @@ -1290,8 +1288,7 @@ def _dict_from_options_callback( help='Remove any previously defined --exec') postproc.add_option( '--exec-before-download', metavar='CMD', - action='callback', dest='exec_before_dl_cmd', default=[], type='str', - callback=_list_from_options_callback, callback_kwargs={'delim': None}, + action='append', dest='exec_before_dl_cmd', help=( 'Execute a command before the actual download. ' 'The syntax is the same as --exec but "filepath" is not available. ' diff --git a/yt_dlp/postprocessor/__init__.py b/yt_dlp/postprocessor/__init__.py index b1a6917d7..31c2d7c68 100644 --- a/yt_dlp/postprocessor/__init__.py +++ b/yt_dlp/postprocessor/__init__.py @@ -19,7 +19,7 @@ FFmpegVideoRemuxerPP, ) from .xattrpp import XAttrMetadataPP -from .execafterdownload import ExecAfterDownloadPP +from .exec import ExecPP, ExecAfterDownloadPP from .metadataparser import ( MetadataFromFieldPP, MetadataFromTitlePP, @@ -36,6 +36,7 @@ def get_postprocessor(key): __all__ = [ 'FFmpegPostProcessor', 'EmbedThumbnailPP', + 'ExecPP', 'ExecAfterDownloadPP', 'FFmpegEmbedSubtitlePP', 'FFmpegExtractAudioPP', diff --git a/yt_dlp/postprocessor/execafterdownload.py b/yt_dlp/postprocessor/exec.py similarity index 87% rename from yt_dlp/postprocessor/execafterdownload.py rename to yt_dlp/postprocessor/exec.py index 182b900d9..7a3cb4999 100644 --- a/yt_dlp/postprocessor/execafterdownload.py +++ b/yt_dlp/postprocessor/exec.py @@ -11,16 +11,12 @@ ) -class ExecAfterDownloadPP(PostProcessor): +class ExecPP(PostProcessor): def __init__(self, downloader, exec_cmd): - super(ExecAfterDownloadPP, self).__init__(downloader) + PostProcessor.__init__(self, downloader) self.exec_cmd = variadic(exec_cmd) - @classmethod - def pp_key(cls): - return 'Exec' - def parse_cmd(self, cmd, info): tmpl, tmpl_dict = self._downloader.prepare_outtmpl(cmd, info) if tmpl_dict: # if there are no replacements, tmpl_dict = {} @@ -40,3 +36,7 @@ def run(self, info): if retCode != 0: raise PostProcessingError('Command returned error code %d' % retCode) return [], info + + +class ExecAfterDownloadPP(ExecPP): # for backward compatibility + pass