Fix bug in 8a7f68d0b12d0f4910a15b59a3ec090bbf83b6f2

[yt-dlp.git] / yt_dlp / postprocessor / embedthumbnail.py

diff --git a/yt_dlp/postprocessor/embedthumbnail.py b/yt_dlp/postprocessor/embedthumbnail.py
index 7008f4d4dbfb5d155b795df8f956979d9d474b6c..84ab54f44be973c85e1d6e255d097cb9861674e0 100644 (file)
--- a/yt_dlp/postprocessor/embedthumbnail.py
+++ b/yt_dlp/postprocessor/embedthumbnail.py
@@ -26,9 +26,9 @@
     encodeArgument,
     encodeFilename,
     error_to_compat_str,
+    Popen,
     PostProcessingError,
     prepend_extension,
-    process_communicate_or_kill,
     shell_quote,
 )
 
@@ -108,7 +108,7 @@ def run(self, info):
             self.run_ffmpeg_multiple_files([filename, thumbnail_filename], temp_filename, options)
 
         elif info['ext'] in ['mkv', 'mka']:
-            options = ['-c', 'copy', '-map', '0', '-dn']
+            options = list(self.stream_copy_opts())
 
             mimetype = 'image/%s' % ('png' if thumbnail_ext == 'png' else 'jpeg')
             old_stream, new_stream = self.get_stream_number(
@@ -145,11 +145,46 @@ def run(self, info):
                     self.report_warning('unable to embed using mutagen; %s' % error_to_compat_str(err))
                     success = False
 
-            # Method 2: Use ffmpeg+ffprobe
-            if not success and not prefer_atomicparsley:
+            # Method 2: Use AtomicParsley
+            if not success:
+                success = True
+                atomicparsley = next((
+                    x for x in ['AtomicParsley', 'atomicparsley']
+                    if check_executable(x, ['-v'])), None)
+                if atomicparsley is None:
+                    self.to_screen('Neither mutagen nor AtomicParsley was found. Falling back to ffmpeg')
+                    success = False
+                else:
+                    if not prefer_atomicparsley:
+                        self.to_screen('mutagen was not found. Falling back to AtomicParsley')
+                    cmd = [encodeFilename(atomicparsley, True),
+                           encodeFilename(filename, True),
+                           encodeArgument('--artwork'),
+                           encodeFilename(thumbnail_filename, True),
+                           encodeArgument('-o'),
+                           encodeFilename(temp_filename, True)]
+                    cmd += [encodeArgument(o) for o in self._configuration_args('AtomicParsley')]
+
+                    self._report_run('atomicparsley', filename)
+                    self.write_debug('AtomicParsley command line: %s' % shell_quote(cmd))
+                    p = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+                    stdout, stderr = p.communicate_or_kill()
+                    if p.returncode != 0:
+                        msg = stderr.decode('utf-8', 'replace').strip()
+                        self.report_warning(f'Unable to embed thumbnails using AtomicParsley; {msg}')
+                    # for formats that don't support thumbnails (like 3gp) AtomicParsley
+                    # won't create to the temporary file
+                    if b'No changes' in stdout:
+                        self.report_warning('The file format doesn\'t support embedding a thumbnail')
+                        success = False
+
+            # Method 3: Use ffmpeg+ffprobe
+            # Thumbnails attached using this method doesn't show up as cover in some cases
+            # See https://github.com/yt-dlp/yt-dlp/issues/2125, https://github.com/yt-dlp/yt-dlp/issues/411
+            if not success:
                 success = True
                 try:
-                    options = ['-c', 'copy', '-map', '0', '-dn', '-map', '1']
+                    options = [*self.stream_copy_opts(), '-map', '1']
 
                     old_stream, new_stream = self.get_stream_number(
                         filename, ('disposition', 'attached_pic'), 1)
@@ -161,38 +196,8 @@ def run(self, info):
                     self._report_run('ffmpeg', filename)
                     self.run_ffmpeg_multiple_files([filename, thumbnail_filename], temp_filename, options)
                 except PostProcessingError as err:
-                    self.report_warning('unable to embed using ffprobe & ffmpeg; %s' % error_to_compat_str(err))
-                    success = False
-
-            # Method 3: Use AtomicParsley
-            if not success:
-                success = True
-                atomicparsley = next((
-                    x for x in ['AtomicParsley', 'atomicparsley']
-                    if check_executable(x, ['-v'])), None)
-                if atomicparsley is None:
-                    raise EmbedThumbnailPPError('AtomicParsley was not found. Please install')
-
-                cmd = [encodeFilename(atomicparsley, True),
-                       encodeFilename(filename, True),
-                       encodeArgument('--artwork'),
-                       encodeFilename(thumbnail_filename, True),
-                       encodeArgument('-o'),
-                       encodeFilename(temp_filename, True)]
-                cmd += [encodeArgument(o) for o in self._configuration_args('AtomicParsley')]
-
-                self._report_run('atomicparsley', filename)
-                self.write_debug('AtomicParsley command line: %s' % shell_quote(cmd))
-                p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-                stdout, stderr = process_communicate_or_kill(p)
-                if p.returncode != 0:
-                    msg = stderr.decode('utf-8', 'replace').strip()
-                    raise EmbedThumbnailPPError(msg)
-                # for formats that don't support thumbnails (like 3gp) AtomicParsley
-                # won't create to the temporary file
-                if b'No changes' in stdout:
-                    self.report_warning('The file format doesn\'t support embedding a thumbnail')
                     success = False
+                    raise EmbedThumbnailPPError(f'Unable to embed using ffprobe & ffmpeg; {err}')
 
         elif info['ext'] in ['ogg', 'opus', 'flac']:
             if not has_mutagen:
@@ -222,8 +227,7 @@ def run(self, info):
             raise EmbedThumbnailPPError('Supported filetypes for thumbnail embedding are: mp3, mkv/mka, ogg/opus/flac, m4a/mp4/mov')
 
         if success and temp_filename != filename:
-            os.remove(encodeFilename(filename))
-            os.rename(encodeFilename(temp_filename), encodeFilename(filename))
+            os.replace(temp_filename, filename)
 
         self.try_utime(filename, mtime, mtime)