From bb9ec9a24d9c3527854a963ecc97611972ad1ca0 Mon Sep 17 00:00:00 2001 From: danilovsergei Date: Mon, 16 Oct 2023 22:35:04 -0700 Subject: [PATCH 1/3] Fix https://github.com/yt-dlp/yt-dlp/issues/8363 by manually adding metadata during ffmpeg split chapters execution --- yt_dlp/postprocessor/ffmpeg.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/yt_dlp/postprocessor/ffmpeg.py b/yt_dlp/postprocessor/ffmpeg.py index 323f4303c..800c14def 100644 --- a/yt_dlp/postprocessor/ffmpeg.py +++ b/yt_dlp/postprocessor/ffmpeg.py @@ -1046,6 +1046,20 @@ def _ffmpeg_args_for_chapter(self, number, chapter, info): destination, ['-ss', str(chapter['start_time']), '-t', str(chapter['end_time'] - chapter['start_time'])]) + # FFmpeg adds metadata about all chapters from parent file to all split m4a files. + # This is incorrect since there must be only single chapter in each file after split. + # Such behavior confuses players who think multiple chapters present + def _set_out_opts(self, ext, chapter_title): + if ext == 'm4a': + return [ + *self.stream_copy_opts(), + # For m4a ffmpeg copies all available parent track chapters to split tracks metadata + # And such behavior confuses players + # Wipe parent track metadata from split tracks and fill out only title + '-metadata', 'title={}'.format(chapter_title), + '-map_metadata','-1'] + else: + return self.stream_copy_opts() @PostProcessor._restrict_to(images=False) def run(self, info): @@ -1061,7 +1075,8 @@ def run(self, info): self.to_screen('Splitting video by chapters; %d chapters found' % len(chapters)) for idx, chapter in enumerate(chapters): destination, opts = self._ffmpeg_args_for_chapter(idx + 1, chapter, info) - self.real_run_ffmpeg([(in_file, opts)], [(destination, self.stream_copy_opts())]) + out_file_opts = self._set_out_opts(info['ext'], chapter['title']) + self.real_run_ffmpeg([(in_file, opts)], [(destination, out_file_opts)]) if in_file != info['filepath']: self._delete_downloaded_files(in_file, msg=None) return [], info From fc46f24dc51d984395fc84c8b6823fbe96bf2476 Mon Sep 17 00:00:00 2001 From: sergeidanilov Date: Wed, 25 Oct 2023 23:18:49 -0700 Subject: [PATCH 2/3] fix pull request lint comments cleaned up implementation and comments --- yt_dlp/postprocessor/ffmpeg.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/yt_dlp/postprocessor/ffmpeg.py b/yt_dlp/postprocessor/ffmpeg.py index 800c14def..6a7264edd 100644 --- a/yt_dlp/postprocessor/ffmpeg.py +++ b/yt_dlp/postprocessor/ffmpeg.py @@ -1046,20 +1046,23 @@ def _ffmpeg_args_for_chapter(self, number, chapter, info): destination, ['-ss', str(chapter['start_time']), '-t', str(chapter['end_time'] - chapter['start_time'])]) + # FFmpeg adds metadata about all chapters from parent file to all split m4a files. # This is incorrect since there must be only single chapter in each file after split. # Such behavior confuses players who think multiple chapters present def _set_out_opts(self, ext, chapter_title): + out_opts = [*self.stream_copy_opts()] + out_opts.extend(['-map_metadata', '0']) + # exclude chapters metadata but keep everything else + out_opts.extend(['-map_chapters', '-1']) + if not chapter_title: + return out_opts + # replace global title with chapter specific title in split files if ext == 'm4a': - return [ - *self.stream_copy_opts(), - # For m4a ffmpeg copies all available parent track chapters to split tracks metadata - # And such behavior confuses players - # Wipe parent track metadata from split tracks and fill out only title - '-metadata', 'title={}'.format(chapter_title), - '-map_metadata','-1'] - else: - return self.stream_copy_opts() + out_opts.extend(['-metadata', 'title={}'.format(chapter_title)]) + if ext == 'opus': + out_opts.extend(['-metadata:s', 'title={}'.format(chapter_title)]) + return out_opts @PostProcessor._restrict_to(images=False) def run(self, info): @@ -1075,7 +1078,7 @@ def run(self, info): self.to_screen('Splitting video by chapters; %d chapters found' % len(chapters)) for idx, chapter in enumerate(chapters): destination, opts = self._ffmpeg_args_for_chapter(idx + 1, chapter, info) - out_file_opts = self._set_out_opts(info['ext'], chapter['title']) + out_file_opts = self._set_out_opts(info['ext'], chapter.get('title', '')) self.real_run_ffmpeg([(in_file, opts)], [(destination, out_file_opts)]) if in_file != info['filepath']: self._delete_downloaded_files(in_file, msg=None) From 523c7e1d442f1bbd93a6749ca9e078159e190bce Mon Sep 17 00:00:00 2001 From: sergeidanilov Date: Sun, 12 Nov 2023 23:40:55 -0800 Subject: [PATCH 3/3] Fixed comments and added more supported formats --- yt_dlp/postprocessor/ffmpeg.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/yt_dlp/postprocessor/ffmpeg.py b/yt_dlp/postprocessor/ffmpeg.py index 6a7264edd..e9d6fa760 100644 --- a/yt_dlp/postprocessor/ffmpeg.py +++ b/yt_dlp/postprocessor/ffmpeg.py @@ -1047,21 +1047,32 @@ def _ffmpeg_args_for_chapter(self, number, chapter, info): ['-ss', str(chapter['start_time']), '-t', str(chapter['end_time'] - chapter['start_time'])]) + # Extends opts with chapter specific metadata for the supported formats. + # + # Tested and supported on opus, m4a, webm and mp4. + def _set_metadata_arg(self, opts, ext, key, value): + if ext == 'opus': + # opus file requires a stream to keep title, artist etc metadata. + # FFmpegMetadataPP already set metadata and created that stream. + # Futher metadata updates should be set on the stream(:s) + # -metadata will do nothing and needs to be -metadata:s + opts.extend(['-metadata:s', f'{key}={value}']) + elif ext in ['m4a', 'webm', 'mp4']: + opts.extend(['-metadata', f'{key}={value}']) + # FFmpeg adds metadata about all chapters from parent file to all split m4a files. # This is incorrect since there must be only single chapter in each file after split. # Such behavior confuses players who think multiple chapters present - def _set_out_opts(self, ext, chapter_title): + def _set_out_opts(self, ext, chapter_title, track_number): out_opts = [*self.stream_copy_opts()] out_opts.extend(['-map_metadata', '0']) # exclude chapters metadata but keep everything else out_opts.extend(['-map_chapters', '-1']) - if not chapter_title: - return out_opts + # replace global title with chapter specific title in split files - if ext == 'm4a': - out_opts.extend(['-metadata', 'title={}'.format(chapter_title)]) - if ext == 'opus': - out_opts.extend(['-metadata:s', 'title={}'.format(chapter_title)]) + if chapter_title: + self._set_metadata_arg(out_opts, ext, "title", chapter_title) + self._set_metadata_arg(out_opts, ext, "track", track_number) return out_opts @PostProcessor._restrict_to(images=False) @@ -1078,7 +1089,7 @@ def run(self, info): self.to_screen('Splitting video by chapters; %d chapters found' % len(chapters)) for idx, chapter in enumerate(chapters): destination, opts = self._ffmpeg_args_for_chapter(idx + 1, chapter, info) - out_file_opts = self._set_out_opts(info['ext'], chapter.get('title', '')) + out_file_opts = self._set_out_opts(info['ext'], chapter.get('title', ''), str(idx + 1)) self.real_run_ffmpeg([(in_file, opts)], [(destination, out_file_opts)]) if in_file != info['filepath']: self._delete_downloaded_files(in_file, msg=None)