Misc fixes - See desc

* Remove unnecessary uses of _list_from_options_callback
* Fix download tests - Bug from 6e84b21559
* Rename ExecAfterDownloadPP to ExecPP and refactor its tests
* Ensure _write_ytdl_file closes file handle on error - Potential fix for #517
This commit is contained in:
pukkandan 2021-08-09 17:40:24 +05:30
parent 2831b4686c
commit ad3dc496bb
8 changed files with 43 additions and 41 deletions

View file

@ -147,7 +147,7 @@ def generator(test_case, tname):
expect_warnings(ydl, test_case.get('expected_warnings', [])) expect_warnings(ydl, test_case.get('expected_warnings', []))
def get_tc_filename(tc): 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 res_dict = None

View file

@ -11,7 +11,7 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
from yt_dlp import YoutubeDL from yt_dlp import YoutubeDL
from yt_dlp.compat import compat_shlex_quote from yt_dlp.compat import compat_shlex_quote
from yt_dlp.postprocessor import ( from yt_dlp.postprocessor import (
ExecAfterDownloadPP, ExecPP,
FFmpegThumbnailsConvertorPP, FFmpegThumbnailsConvertorPP,
MetadataFromFieldPP, MetadataFromFieldPP,
MetadataParserPP, MetadataParserPP,
@ -59,12 +59,12 @@ class TestConvertThumbnail(unittest.TestCase):
os.remove(file.format(out)) os.remove(file.format(out))
class TestExecAfterDownload(unittest.TestCase): class TestExec(unittest.TestCase):
def test_parse_cmd(self): def test_parse_cmd(self):
pp = ExecAfterDownloadPP(YoutubeDL(), '') pp = ExecPP(YoutubeDL(), '')
info = {'filepath': 'file name'} 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), cmd)
self.assertEqual(pp.parse_cmd('echo.{}', info), 'echo.%s' % quoted_filepath) self.assertEqual(pp.parse_cmd('echo {}', info), cmd)
self.assertEqual(pp.parse_cmd('echo "%(filepath)s"', info), 'echo "%s"' % info['filepath']) self.assertEqual(pp.parse_cmd('echo %(filepath)q', info), cmd)

View file

@ -2339,7 +2339,8 @@ class YoutubeDL(object):
requested_langs = ['en'] requested_langs = ['en']
else: else:
requested_langs = [list(all_sub_langs)[0]] 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_query = self.params.get('subtitlesformat', 'best')
formats_preference = formats_query.split('/') if formats_query else [] formats_preference = formats_query.split('/') if formats_query else []
@ -3256,13 +3257,13 @@ class YoutubeDL(object):
from .postprocessor.embedthumbnail import has_mutagen from .postprocessor.embedthumbnail import has_mutagen
from .cookies import SQLITE_AVAILABLE, KEYRING_AVAILABLE from .cookies import SQLITE_AVAILABLE, KEYRING_AVAILABLE
lib_str = ', '.join(filter(None, ( lib_str = ', '.join(sorted(filter(None, (
can_decrypt_frag and 'pycryptodome', can_decrypt_frag and 'pycryptodome',
has_websockets and 'websockets', has_websockets and 'websockets',
has_mutagen and 'mutagen', has_mutagen and 'mutagen',
SQLITE_AVAILABLE and 'sqlite', SQLITE_AVAILABLE and 'sqlite',
KEYRING_AVAILABLE and 'keyring', KEYRING_AVAILABLE and 'keyring',
))) or 'none' )))) or 'none'
self._write_string('[debug] Optional libraries: %s\n' % lib_str) self._write_string('[debug] Optional libraries: %s\n' % lib_str)
proxy_map = {} proxy_map = {}

View file

@ -332,7 +332,8 @@ def _real_main(argv=None):
for k, tmpl in opts.outtmpl.items(): for k, tmpl in opts.outtmpl.items():
validate_outtmpl(tmpl, '%s output template' % k) 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') validate_outtmpl(tmpl, 'print template')
if opts.extractaudio and not opts.keepvideo and opts.format is None: if opts.extractaudio and not opts.keepvideo and opts.format is None:
@ -445,7 +446,7 @@ def _real_main(argv=None):
# Must be after all other before_dl # Must be after all other before_dl
if opts.exec_before_dl_cmd: if opts.exec_before_dl_cmd:
postprocessors.append({ postprocessors.append({
'key': 'ExecAfterDownload', 'key': 'Exec',
'exec_cmd': opts.exec_before_dl_cmd, 'exec_cmd': opts.exec_before_dl_cmd,
'when': 'before_dl' 'when': 'before_dl'
}) })
@ -516,10 +517,10 @@ def _real_main(argv=None):
# XAttrMetadataPP should be run after post-processors that may change file contents # XAttrMetadataPP should be run after post-processors that may change file contents
if opts.xattrs: if opts.xattrs:
postprocessors.append({'key': 'XAttrMetadata'}) postprocessors.append({'key': 'XAttrMetadata'})
# ExecAfterDownload must be the last PP # Exec must be the last PP
if opts.exec_cmd: if opts.exec_cmd:
postprocessors.append({ postprocessors.append({
'key': 'ExecAfterDownload', 'key': 'Exec',
'exec_cmd': opts.exec_cmd, 'exec_cmd': opts.exec_cmd,
# Run this only after the files have been moved to their final locations # Run this only after the files have been moved to their final locations
'when': 'after_move' 'when': 'after_move'

View file

@ -105,17 +105,19 @@ class FragmentFD(FileDownloader):
def _write_ytdl_file(self, ctx): def _write_ytdl_file(self, ctx):
frag_index_stream, _ = sanitize_open(self.ytdl_filename(ctx['filename']), 'w') frag_index_stream, _ = sanitize_open(self.ytdl_filename(ctx['filename']), 'w')
downloader = { try:
'current_fragment': { downloader = {
'index': ctx['fragment_index'], 'current_fragment': {
}, 'index': ctx['fragment_index'],
} },
if 'extra_state' in ctx: }
downloader['extra_state'] = ctx['extra_state'] if 'extra_state' in ctx:
if ctx.get('fragment_count') is not None: downloader['extra_state'] = ctx['extra_state']
downloader['fragment_count'] = ctx['fragment_count'] if ctx.get('fragment_count') is not None:
frag_index_stream.write(json.dumps({'downloader': downloader})) downloader['fragment_count'] = ctx['fragment_count']
frag_index_stream.close() 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): def _download_fragment(self, ctx, frag_url, info_dict, headers=None, request_data=None):
fragment_filename = '%s-Frag%d' % (ctx['tmpfilename'], ctx['fragment_index']) fragment_filename = '%s-Frag%d' % (ctx['tmpfilename'], ctx['fragment_index'])

View file

@ -23,7 +23,7 @@ from .cookies import SUPPORTED_BROWSERS
from .version import __version__ from .version import __version__
from .downloader.external import list_external_downloaders from .downloader.external import list_external_downloaders
from .postprocessor.ffmpeg import ( from .postprocessor import (
FFmpegExtractAudioPP, FFmpegExtractAudioPP,
FFmpegSubtitlesConvertorPP, FFmpegSubtitlesConvertorPP,
FFmpegThumbnailsConvertorPP, FFmpegThumbnailsConvertorPP,
@ -803,9 +803,8 @@ def parseOpts(overrideArguments=None):
action='store_true', dest='skip_download', default=False, action='store_true', dest='skip_download', default=False,
help='Do not download the video but write all related files (Alias: --no-download)') help='Do not download the video but write all related files (Alias: --no-download)')
verbosity.add_option( verbosity.add_option(
'-O', '--print', metavar='TEMPLATE', '-O', '--print',
action='callback', dest='forceprint', type='str', default=[], metavar='TEMPLATE', action='append', dest='forceprint',
callback=_list_from_options_callback, callback_kwargs={'delim': None},
help=( help=(
'Quiet, but print the given fields for each video. Simulate unless --no-simulate is used. ' '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')) 'Either a field name or same syntax as the output template can be used'))
@ -1276,8 +1275,7 @@ def parseOpts(overrideArguments=None):
help='Location of the ffmpeg binary; either the path to the binary or its containing directory') help='Location of the ffmpeg binary; either the path to the binary or its containing directory')
postproc.add_option( postproc.add_option(
'--exec', metavar='CMD', '--exec', metavar='CMD',
action='callback', dest='exec_cmd', default=[], type='str', action='append', dest='exec_cmd',
callback=_list_from_options_callback, callback_kwargs={'delim': None},
help=( help=(
'Execute a command on the file after downloading and post-processing. ' '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. ' 'Same syntax as the output template can be used to pass any field as arguments to the command. '
@ -1290,8 +1288,7 @@ def parseOpts(overrideArguments=None):
help='Remove any previously defined --exec') help='Remove any previously defined --exec')
postproc.add_option( postproc.add_option(
'--exec-before-download', metavar='CMD', '--exec-before-download', metavar='CMD',
action='callback', dest='exec_before_dl_cmd', default=[], type='str', action='append', dest='exec_before_dl_cmd',
callback=_list_from_options_callback, callback_kwargs={'delim': None},
help=( help=(
'Execute a command before the actual download. ' 'Execute a command before the actual download. '
'The syntax is the same as --exec but "filepath" is not available. ' 'The syntax is the same as --exec but "filepath" is not available. '

View file

@ -19,7 +19,7 @@ from .ffmpeg import (
FFmpegVideoRemuxerPP, FFmpegVideoRemuxerPP,
) )
from .xattrpp import XAttrMetadataPP from .xattrpp import XAttrMetadataPP
from .execafterdownload import ExecAfterDownloadPP from .exec import ExecPP, ExecAfterDownloadPP
from .metadataparser import ( from .metadataparser import (
MetadataFromFieldPP, MetadataFromFieldPP,
MetadataFromTitlePP, MetadataFromTitlePP,
@ -36,6 +36,7 @@ def get_postprocessor(key):
__all__ = [ __all__ = [
'FFmpegPostProcessor', 'FFmpegPostProcessor',
'EmbedThumbnailPP', 'EmbedThumbnailPP',
'ExecPP',
'ExecAfterDownloadPP', 'ExecAfterDownloadPP',
'FFmpegEmbedSubtitlePP', 'FFmpegEmbedSubtitlePP',
'FFmpegExtractAudioPP', 'FFmpegExtractAudioPP',

View file

@ -11,16 +11,12 @@ from ..utils import (
) )
class ExecAfterDownloadPP(PostProcessor): class ExecPP(PostProcessor):
def __init__(self, downloader, exec_cmd): def __init__(self, downloader, exec_cmd):
super(ExecAfterDownloadPP, self).__init__(downloader) PostProcessor.__init__(self, downloader)
self.exec_cmd = variadic(exec_cmd) self.exec_cmd = variadic(exec_cmd)
@classmethod
def pp_key(cls):
return 'Exec'
def parse_cmd(self, cmd, info): def parse_cmd(self, cmd, info):
tmpl, tmpl_dict = self._downloader.prepare_outtmpl(cmd, info) tmpl, tmpl_dict = self._downloader.prepare_outtmpl(cmd, info)
if tmpl_dict: # if there are no replacements, tmpl_dict = {} if tmpl_dict: # if there are no replacements, tmpl_dict = {}
@ -40,3 +36,7 @@ class ExecAfterDownloadPP(PostProcessor):
if retCode != 0: if retCode != 0:
raise PostProcessingError('Command returned error code %d' % retCode) raise PostProcessingError('Command returned error code %d' % retCode)
return [], info return [], info
class ExecAfterDownloadPP(ExecPP): # for backward compatibility
pass