Revert "ytdl_hook: add chapters by parsing video's description" #16777
Conversation
This reverts commit df1e71a. The next commit will make this unused since chapters will only be taken from yt-dlp's JSON.
guidocella
force-pushed
the
youtube-chapters
branch
from
b961c8e to
b5440cf
Compare
This reverts commit e9c4325. yt-dlp implemented its own parsing to extract chapters from the descriptions of videos without chapter markers in the player over 3 years ago in yt-dlp/yt-dlp@0fe51254 (the PR is even inspired by mpv). Actually it was present even earlier in youtube-dl but was removed for unknown reasons in ytdl-org/youtube-dl@67299f2. So we can remove our parsing code, as it is dead code that never runs if yt-dlp's JSON already contains chapters. And it seems that such videos without chapter markers are rare or non-existing by now anyway - we can't find any. https://www.youtube.com/watch?v=1v_4dL8l8pQ is linked from the yt-dlp PR with the above commit, but now yt-dlp returns the Key moments as chapters, so it can't be used for testing. Our parsing was actually worse than yt-dlp's, because mpv-player#16085 added an option to disable it to fix the misdetection reported in mpv-player#16081, but yt-dlp correctly returns no chapter for that video (https://www.youtube.com/watch?v=1v_4dL8l8pQ). So this code branch was only running for misdetections, and by removing it that option is not needed.
guidocella
force-pushed
the
youtube-chapters
branch
from
b5440cf to
a2d8a57
Compare
|
Download the artifacts for this pull request: Windows |
If ytdl_hook still supports youtube-dl and youtube-dl does not extract chapters, then this should not be removed. There are valid reasons to use youtube-dl instead of yt-dlp, for example it supports some sites that yt-dlp refuses to or removed support because they are considered "piracy sites". |
|
Do those really have chapters only in the description though? Even yt-dlp only uses it in 3 extractors meaning it is uncommon so there's no difference with yt-dlp there. For youtube itself youtube-dl doesn't work at all for me since the last commit is from 4 months ago so I can't even test it lol, but note that this is only for videos where the web player doesn't show the chapter markers and they are only in the description and I can't find such videos at all now. They were common 3 years ago when we last tested it so youtube must have improved this. If I comment |
There are many youtube alternatives that are not in that list, many are used by uploaders who cannot or do not use youtube for whatever reason. They serve the same purpose as youtube. yt-dlp not including other websites does not mean they are not useful.
Do old videos just magically disappear? In the video you mentioned https://www.youtube.com/watch?v=1v_4dL8l8pQ no chapters shown in web player. yt-dlp priortize extracting the timestamps from "Key moments" which are AI genertated and do not match the timestamps in description.
You can just disable it for youtube only. For other websites, it's better to have something than nothing. |
Why remove this option? Is yt-dlp itself avoiding each and every issue that our custom parser had that prompted addition of this option? |
I don't know when I said they are not useful. I mean that it is not common to have chapters only in the description and not in the API if it's only enabled for 3 extractors. If you grep chapter in
I meant I can't find old videos without chapter markers so I'm assuming they added them later. Ther was a video linked in the relative IRC discussion 3 years ago but is down now so it can't be tested. mpv already uses the Key moments as chapters on master for that video since yt-dlp's JSON takes priority so there is no difference. But either way yt-dlp would still extract chapters from the description while youtube-dl doesn't even work with youtube.
That option never disabled the chapters returned by yt-dlp but only the ones defined by our parser which is removed by the next commit. The only known issue was in our parser. |
4 participants
Revert "ytdl_hook: add option to extract chapters"
This reverts commit df1e71a.
The next commit will make this unused since chapters will only be taken from yt-dlp's JSON.
Revert "ytdl_hook: add chapters by parsing video's description"
This reverts commit e9c4325.
yt-dlp implemented its own parsing to extract chapters from the descriptions of videos without chapter markers in the player over 3 years ago in yt-dlp/yt-dlp@0fe51254 (the PR is even inspired by mpv). Actually it was present even earlier in youtube-dl but was removed for unknown reasons in ytdl-org/youtube-dl@67299f2. So we can remove our parsing code, as it is dead code that never runs if yt-dlp's JSON already contains chapters.
And it seems that such videos without chapter markers are rare or non-existing by now anyway - we can't find any. https://www.youtube.com/watch?v=1v_4dL8l8pQ is linked from the yt-dlp PR with the above commit, but now yt-dlp returns the Key moments as chapters, so it can't be used for testing.
Our parsing was actually worse than yt-dlp's, because #16085 added an option to disable it to fix the misdetection reported in #16081, but yt-dlp correctly returns no chapter for that video (https://www.youtube.com/watch?v=1v_4dL8l8pQ). So this code branch was only running for misdetections, and by removing it that option is not needed.