[embedthumbnail] Correctly escape filename

Closes #352
The approach in [1] is faulty as can be seen in the test cases
1. https://github.com/ytdl-org/youtube-dl/commit/bff857a8af696e701482208617bf0b7564951326

diff --git a/.gitignore b/.gitignore
index a2484b752612783bba2257071720e306640df553..b6431b76662ec493229a5fbc1aa32d14a47394a7 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,46 @@
+# Config
+*.conf
+*.spec
+cookies
+cookies.txt
+
+# Downloaded
+*.srt
+*.ttml
+*.sbv
+*.vtt
+*.flv
+*.mp4
+*.m4a
+*.m4v
+*.mp3
+*.3gp
+*.webm
+*.wav
+*.ape
+*.mkv
+*.swf
+*.part
+*.part-*
+*.ytdl
+*.dump
+*.frag
+*.frag.urls
+*.aria2
+*.swp
+*.ogg
+*.opus
+*.info.json
+*.live_chat.json
+*.jpg
+*.png
+*.webp
+*.annotations.xml
+*.description
+
+# Allow config/media files in testdata
+!test/testdata/**
+
 # Python
 *.pyc
 *.pyo
@@ -43,48 +86,6 @@ README.txt
 yt-dlp.zip
 *.exe
 
-# Downloaded
-*.srt
-*.ttml
-*.sbv
-*.vtt
-*.flv
-*.mp4
-*.m4a
-*.m4v
-*.mp3
-*.3gp
-*.webm
-*.wav
-*.ape
-*.mkv
-*.swf
-*.part
-*.part-*
-*.ytdl
-*.dump
-*.frag
-*.frag.urls
-*.aria2
-*.swp
-*.ogg
-*.opus
-*.info.json
-*.live_chat.json
-*.jpg
-*.png
-*.webp
-*.annotations.xml
-*.description
-
-# Config
-*.conf
-*.spec
-cookies
-cookies.txt
-
-
-
 # Text Editor / IDE
 .idea
 *.iml

diff --git a/test/test_postprocessors.py b/test/test_postprocessors.py
index 7574a0b950a2260596e1c851eb56a04a21fe5869..868bb25f9bf7d1a53a9daca4bf41e7cefb4ab80f 100644 (file)
--- a/test/test_postprocessors.py
+++ b/test/test_postprocessors.py
@@ -8,7 +8,11 @@
 import unittest
 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-from yt_dlp.postprocessor import MetadataFromFieldPP, MetadataFromTitlePP
+from yt_dlp.postprocessor import (
+    FFmpegThumbnailsConvertorPP,
+    MetadataFromFieldPP,
+    MetadataFromTitlePP,
+)
 
 
 class TestMetadataFromField(unittest.TestCase):
@@ -30,3 +34,24 @@ class TestMetadataFromTitle(unittest.TestCase):
     def test_format_to_regex(self):
         pp = MetadataFromTitlePP(None, '%(title)s - %(artist)s')
         self.assertEqual(pp._titleregex, r'(?P<title>.+)\ \-\ (?P<artist>.+)')
+
+
+class TestConvertThumbnail(unittest.TestCase):
+    def test_escaping(self):
+        pp = FFmpegThumbnailsConvertorPP()
+        if not pp.available:
+            print('Skipping: ffmpeg not found')
+            return
+
+        file = 'test/testdata/thumbnails/foo %d bar/foo_%d.{}'
+        tests = (('webp', 'png'), ('png', 'jpg'))
+
+        for inp, out in tests:
+            out_file = file.format(out)
+            if os.path.exists(out_file):
+                os.remove(out_file)
+            pp.convert_thumbnail(file.format(inp), out)
+            assert os.path.exists(out_file)
+
+        for _, out in tests:
+            os.remove(file.format(out))

diff --git a/test/testdata/thumbnails/foo %d bar/foo_%d.webp b/test/testdata/thumbnails/foo %d bar/foo_%d.webp
new file mode 100644 (file)
index 0000000..d64d083
Binary files /dev/null and b/test/testdata/thumbnails/foo %d bar/foo_%d.webp differ

diff --git a/yt_dlp/postprocessor/embedthumbnail.py b/yt_dlp/postprocessor/embedthumbnail.py
index f3eb7d96d50610ee4905497d7af370a30ddc4a94..278a45eb64b42294681a58ed8c688b1d6c0b0ae3 100644 (file)
--- a/yt_dlp/postprocessor/embedthumbnail.py
+++ b/yt_dlp/postprocessor/embedthumbnail.py
@@ -70,7 +70,7 @@ def run(self, info):
             self.to_screen('There aren\'t any thumbnails to embed')
             return [], info
 
-        idx = next((-(i+1) for i, t in enumerate(info['thumbnails'][::-1]) if t.get('filepath')), None)
+        idx = next((-i for i, t in enumerate(info['thumbnails'][::-1], 1) if t.get('filepath')), None)
         if idx is None:
             self.to_screen('There are no thumbnails on disk')
             return [], info

diff --git a/yt_dlp/postprocessor/ffmpeg.py b/yt_dlp/postprocessor/ffmpeg.py
index ea728be37b7d4be0cc745eb35a354f0e0bf3227b..d9f816b0431667e0a30deefa50473cf0d6ab75f7 100644 (file)
--- a/yt_dlp/postprocessor/ffmpeg.py
+++ b/yt_dlp/postprocessor/ffmpeg.py
@@ -853,19 +853,12 @@ def _options(target_ext):
         return []
 
     def convert_thumbnail(self, thumbnail_filename, target_ext):
-        # NB: % is supposed to be escaped with %% but this does not work
-        # for input files so working around with standard substitution
-        escaped_thumbnail_filename = thumbnail_filename.replace('%', '#')
-        os.rename(encodeFilename(thumbnail_filename), encodeFilename(escaped_thumbnail_filename))
-        escaped_thumbnail_conv_filename = replace_extension(escaped_thumbnail_filename, target_ext)
-
-        self.to_screen('Converting thumbnail "%s" to %s' % (escaped_thumbnail_filename, target_ext))
-        self.run_ffmpeg(escaped_thumbnail_filename, escaped_thumbnail_conv_filename, self._options(target_ext))
-
-        # Rename back to unescaped
         thumbnail_conv_filename = replace_extension(thumbnail_filename, target_ext)
-        os.rename(encodeFilename(escaped_thumbnail_filename), encodeFilename(thumbnail_filename))
-        os.rename(encodeFilename(escaped_thumbnail_conv_filename), encodeFilename(thumbnail_conv_filename))
+
+        self.to_screen('Converting thumbnail "%s" to %s' % (thumbnail_filename, target_ext))
+        self.real_run_ffmpeg(
+            [(thumbnail_filename, ['-f', 'image2', '-pattern_type', 'none'])],
+            [(thumbnail_conv_filename.replace('%', '%%'), self._options(target_ext))])
         return thumbnail_conv_filename
 
     def run(self, info):