Repository: yt-dlp/yt-dlp
Pull request #5260 (2022-10-16 22:47:55)

[SponsorBlock] Add support for chapter category

### Description of your *pull request* and other information

Renamed `_categories` to `_titles` and added in the chapter description when applicable. In tests, renamed the function to be clearer with sponsor chapters also existing separately, but if that's too confusing, I can change it to something else.

<details open><summary>Template</summary> <!-- OPEN is intentional -->

<!--

# PLEASE FOLLOW THE GUIDE BELOW

- You will be asked some questions, please read them **carefully** and answer honestly
- Put an `x` into all the boxes `[ ]` relevant to your *pull request* (like [x])
- Use *Preview* tab to see how your *pull request* will actually look like

-->

### Before submitting a *pull request* make sure you have:
- [x] At least skimmed through [contributing guidelines](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#developer-instructions) including [yt-dlp coding conventions](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#yt-dlp-coding-conventions)
- [x] [Searched](https://github.com/yt-dlp/yt-dlp/search?q=is%3Apr&type=Issues) the bugtracker for similar pull requests
- [x] Checked the code with [flake8](https://pypi.python.org/pypi/flake8) and [ran relevant tests](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#developer-instructions)

### In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under [Unlicense](http://unlicense.org/). Check one of the following options:
- [x] I am the original author of this code and I am willing to release it under [Unlicense](http://unlicense.org/)
- [ ] I am not the original author of this code but it is in public domain or released under [Unlicense](http://unlicense.org/) (provide reliable evidence)

### What is the purpose of your *pull request*?
- [x] Fix or improvement to an extractor (Make sure to add/update tests)
- [ ] New extractor ([Piracy websites will not be accepted](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#is-the-website-primarily-used-for-piracy))
- [ ] Core bug fix/improvement
- [ ] New feature (It is strongly [recommended to open an issue first](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#adding-new-feature-or-making-overarching-changes))


ajayyy: okay, need to figure out why tests aren't running locally then...
pukkandan: I thought the `chapter` category was a paid feature 🤔

Could you give an example URL that has chapters? so I can verify everything is working correctly
ajayyy: Only in the extension

https://www.youtube.com/watch?v=qD6bPNZRRbQ

Patch:
diff --git a/README.md b/README.md
index 7b2c6ba71..e7fc6886a 100644
--- a/README.md
+++ b/README.md
@@ -1042,7 +1042,7 @@ ## SponsorBlock Options:
                                     for, separated by commas. Available
                                     categories are sponsor, intro, outro,
                                     selfpromo, preview, filler, interaction,
-                                    music_offtopic, poi_highlight, all and
+                                    music_offtopic, poi_highlight, chapter, all and
                                     default (=all). You can prefix the category
                                     with a "-" to exclude it. See [1] for
                                     description of the categories. E.g.
@@ -1054,8 +1054,8 @@ ## SponsorBlock Options:
                                     remove takes precedence. The syntax and
                                     available categories are the same as for
                                     --sponsorblock-mark except that "default"
-                                    refers to "all,-filler" and poi_highlight is
-                                    not available
+                                    refers to "all,-filler" and poi_highlight and
+                                    chapter are not available
     --sponsorblock-chapter-title TEMPLATE
                                     An output template for the title of the
                                     SponsorBlock chapters created by
diff --git a/yt_dlp/options.py b/yt_dlp/options.py
index 5ff375fcf..d3dfee820 100644
--- a/yt_dlp/options.py
+++ b/yt_dlp/options.py
@@ -1737,7 +1737,7 @@ def _alias_callback(option, opt_str, value, parser, opts, nargs):
         '--sponsorblock-remove', metavar='CATS',
         dest='sponsorblock_remove', default=set(), action='callback', type='str',
         callback=_set_from_options_callback, callback_kwargs={
-            'allowed_values': set(SponsorBlockPP.CATEGORIES.keys()) - set(SponsorBlockPP.POI_CATEGORIES.keys()),
+            'allowed_values': set(SponsorBlockPP.CATEGORIES.keys()) - set(SponsorBlockPP.NON_SKIPPABLE_CATEGORIES.keys()),
             # Note: From https://wiki.sponsor.ajay.app/w/Types:
             # The filler category is very aggressive.
             # It is strongly recommended to not use this in a client by default.
@@ -1747,7 +1747,7 @@ def _alias_callback(option, opt_str, value, parser, opts, nargs):
             'If a category is present in both mark and remove, remove takes precedence. '
             'The syntax and available categories are the same as for --sponsorblock-mark '
             'except that "default" refers to "all,-filler" '
-            f'and {", ".join(SponsorBlockPP.POI_CATEGORIES.keys())} is not available'))
+            f'and {", ".join(SponsorBlockPP.NON_SKIPPABLE_CATEGORIES.keys())} are not available'))
     sponsorblock.add_option(
         '--sponsorblock-chapter-title', metavar='TEMPLATE',
         default=DEFAULT_SPONSORBLOCK_CHAPTER_TITLE, dest='sponsorblock_chapter_title',
diff --git a/yt_dlp/postprocessor/modify_chapters.py b/yt_dlp/postprocessor/modify_chapters.py
index 6959222c8..b2b1acca4 100644
--- a/yt_dlp/postprocessor/modify_chapters.py
+++ b/yt_dlp/postprocessor/modify_chapters.py
@@ -16,7 +16,7 @@ def __init__(self, downloader, remove_chapters_patterns=None, remove_sponsor_seg
                  *, sponsorblock_chapter_title=DEFAULT_SPONSORBLOCK_CHAPTER_TITLE, force_keyframes=False):
         FFmpegPostProcessor.__init__(self, downloader)
         self._remove_chapters_patterns = set(remove_chapters_patterns or [])
-        self._remove_sponsor_segments = set(remove_sponsor_segments or []) - set(SponsorBlockPP.POI_CATEGORIES.keys())
+        self._remove_sponsor_segments = set(remove_sponsor_segments or []) - set(SponsorBlockPP.NON_SKIPPABLE_CATEGORIES.keys())
         self._ranges_to_remove = set(remove_ranges or [])
         self._sponsorblock_chapter_title = sponsorblock_chapter_title
         self._force_keyframes = force_keyframes
@@ -99,7 +99,7 @@ def _mark_chapters_to_remove(self, chapters, sponsor_chapters):
             'start_time': start,
             'end_time': end,
             'category': 'manually_removed',
-            '_categories': [('manually_removed', start, end)],
+            '_categories': [('manually_removed', start, end, 'Manually removed')],
             'remove': True,
         } for start, end in self._ranges_to_remove)
 
@@ -290,13 +290,12 @@ def _remove_tiny_rename_sponsors(self, chapters):
             c.pop('_was_cut', None)
             cats = c.pop('_categories', None)
             if cats:
-                category = min(cats, key=lambda c: c[2] - c[1])[0]
-                cats = orderedSet(x[0] for x in cats)
+                category, _, _, category_name = min(cats, key=lambda c: c[2] - c[1])
                 c.update({
                     'category': category,
-                    'categories': cats,
-                    'name': SponsorBlockPP.CATEGORIES[category],
-                    'category_names': [SponsorBlockPP.CATEGORIES[c] for c in cats]
+                    'categories': orderedSet(x[0] for x in cats),
+                    'name': category_name,
+                    'category_names': orderedSet(x[3] for x in cats),
                 })
                 c['title'] = self._downloader.evaluate_outtmpl(self._sponsorblock_chapter_title, c.copy())
                 # Merge identically named sponsors.
diff --git a/yt_dlp/postprocessor/sponsorblock.py b/yt_dlp/postprocessor/sponsorblock.py
index d79ed7ae7..befff0e1f 100644
--- a/yt_dlp/postprocessor/sponsorblock.py
+++ b/yt_dlp/postprocessor/sponsorblock.py
@@ -14,6 +14,10 @@ class SponsorBlockPP(FFmpegPostProcessor):
     POI_CATEGORIES = {
         'poi_highlight': 'Highlight',
     }
+    NON_SKIPPABLE_CATEGORIES = {
+        **POI_CATEGORIES,
+        'chapter': 'Chapter',
+    }
     CATEGORIES = {
         'sponsor': 'Sponsor',
         'intro': 'Intermission/Intro Animation',
@@ -23,7 +27,7 @@ class SponsorBlockPP(FFmpegPostProcessor):
         'filler': 'Filler Tangent',
         'interaction': 'Interaction Reminder',
         'music_offtopic': 'Non-Music Section',
-        **POI_CATEGORIES,
+        **NON_SKIPPABLE_CATEGORIES
     }
 
     def __init__(self, downloader, categories=None, api='https://sponsor.ajay.app'):
@@ -68,12 +72,13 @@ def duration_filter(s):
 
         def to_chapter(s):
             (start, end), cat = s['segment'], s['category']
+            title = s['description'] if cat == 'chapter' else self.CATEGORIES[cat]
             return {
                 'start_time': start,
                 'end_time': end,
                 'category': cat,
-                'title': self.CATEGORIES[cat],
-                '_categories': [(cat, start, end)]
+                'title': title,
+                '_categories': [(cat, start, end, title)],
             }
 
         sponsor_chapters = [to_chapter(s) for s in duration_match]
@@ -89,7 +94,7 @@ def _get_sponsor_segments(self, video_id, service):
         url = f'{self._API_URL}/api/skipSegments/{hash[:4]}?' + urllib.parse.urlencode({
             'service': service,
             'categories': json.dumps(self._categories),
-            'actionTypes': json.dumps(['skip', 'poi'])
+            'actionTypes': json.dumps(['skip', 'poi', 'chapter'])
         })
         for d in self._download_json(url) or []:
             if d['videoID'] == video_id:


Test patch:
diff --git a/test/test_postprocessors.py b/test/test_postprocessors.py
index c49e3ede0..52e558772 100644
--- a/test/test_postprocessors.py
+++ b/test/test_postprocessors.py
@@ -16,6 +16,7 @@
     MetadataFromFieldPP,
     MetadataParserPP,
     ModifyChaptersPP,
+    SponsorBlockPP,
 )
 
 
@@ -76,11 +77,15 @@ def setUp(self):
         self._pp = ModifyChaptersPP(YoutubeDL())
 
     @staticmethod
-    def _sponsor_chapter(start, end, cat, remove=False):
-        c = {'start_time': start, 'end_time': end, '_categories': [(cat, start, end)]}
-        if remove:
-            c['remove'] = True
-        return c
+    def _sponsor_chapter(start, end, cat, remove=False, title=None):
+        if title is None:
+            title = SponsorBlockPP.CATEGORIES[cat]
+        return {
+            'start_time': start,
+            'end_time': end,
+            '_categories': [(cat, start, end, title)],
+            **({'remove': True} if remove else {}),
+        }
 
     @staticmethod
     def _chapter(start, end, title=None, remove=False):
@@ -130,6 +135,19 @@ def test_remove_marked_arrange_sponsors_ChapterWithSponsors(self):
              'c', '[SponsorBlock]: Filler Tangent', 'c'])
         self._remove_marked_arrange_sponsors_test_impl(chapters, expected, [])
 
+    def test_remove_marked_arrange_sponsors_SponsorBlockChapters(self):
+        chapters = self._chapters([70], ['c']) + [
+            self._sponsor_chapter(10, 20, 'chapter', title='sb c1'),
+            self._sponsor_chapter(15, 16, 'chapter', title='sb c2'),
+            self._sponsor_chapter(30, 40, 'preview'),
+            self._sponsor_chapter(50, 60, 'filler')]
+        expected = self._chapters(
+            [10, 15, 16, 20, 30, 40, 50, 60, 70],
+            ['c', '[SponsorBlock]: sb c1', '[SponsorBlock]: sb c1, sb c2', '[SponsorBlock]: sb c1',
+             'c', '[SponsorBlock]: Preview/Recap',
+             'c', '[SponsorBlock]: Filler Tangent', 'c'])
+        self._remove_marked_arrange_sponsors_test_impl(chapters, expected, [])
+
     def test_remove_marked_arrange_sponsors_UniqueNamesForOverlappingSponsors(self):
         chapters = self._chapters([120], ['c']) + [
             self._sponsor_chapter(10, 45, 'sponsor'), self._sponsor_chapter(20, 40, 'selfpromo'),
@@ -173,7 +191,7 @@ def test_remove_marked_arrange_sponsors_ChapterWithSponsorCutInTheMiddle(self):
         self._remove_marked_arrange_sponsors_test_impl(chapters, expected, cuts)
 
     def test_remove_marked_arrange_sponsors_ChapterWithCutHidingSponsor(self):
-        cuts = [self._sponsor_chapter(20, 50, 'selpromo', remove=True)]
+        cuts = [self._sponsor_chapter(20, 50, 'selfpromo', remove=True)]
         chapters = self._chapters([60], ['c']) + [
             self._sponsor_chapter(10, 20, 'intro'),
             self._sponsor_chapter(30, 40, 'sponsor'),
@@ -199,7 +217,7 @@ def test_remove_marked_arrange_sponsors_ChapterWithAdjacentCuts(self):
             self._sponsor_chapter(10, 20, 'sponsor'),
             self._sponsor_chapter(20, 30, 'interaction', remove=True),
             self._chapter(30, 40, remove=True),
-            self._sponsor_chapter(40, 50, 'selpromo', remove=True),
+            self._sponsor_chapter(40, 50, 'selfpromo', remove=True),
             self._sponsor_chapter(50, 60, 'interaction')]
         expected = self._chapters([10, 20, 30, 40],
                                   ['c', '[SponsorBlock]: Sponsor',
@@ -282,7 +300,7 @@ def test_remove_marked_arrange_sponsors_SponsorsNoLongerOverlapAfterCut(self):
         chapters = self._chapters([70], ['c']) + [
             self._sponsor_chapter(10, 30, 'sponsor'),
             self._sponsor_chapter(20, 50, 'interaction'),
-            self._sponsor_chapter(30, 50, 'selpromo', remove=True),
+            self._sponsor_chapter(30, 50, 'selfpromo', remove=True),
             self._sponsor_chapter(40, 60, 'sponsor'),
             self._sponsor_chapter(50, 60, 'interaction')]
         expected = self._chapters(