Skip to content

Features from animelover1984/youtube-dl #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 22 commits into from
Jan 27, 2021
Merged

Features from animelover1984/youtube-dl #31

merged 22 commits into from
Jan 27, 2021

Conversation

pukkandan
Copy link
Member

@pukkandan pukkandan commented Jan 24, 2021 •
edited
Loading

This PR will merge many features from https://github.com/animelover1984/youtube-dl
Related: animelover1984/youtube-dl#18

Features added:

  • [VideoRemuxer] Conditionally remux video
  • [VideoRemuxer] Add -movflags +faststart when remuxing from mp4
  • Write playlist data to infojson
  • [avanto] Use NFLTokenGenerator if possible
  • [ffmpeg] Print entire stderr in verbose when there is error
  • [ffmpeg] Handle URL filenames correctly
  • [ffmpeg] Warn when embedding ass in mp4
  • [FFmpegMetadata] Embed infojson inside the video
  • [embedthumbnail] Treat mka like mkv and mov like mp4
  • [embedthumbnail] Try embedding in mp4 using ffprobe and -disposition
  • [embedthumbnail] Embed in ogg/opus
  • [billibilli] Better video extraction
  • [billibilli] Extract comments
  • [billibilli] Added BiliBiliSearchIE, BilibiliChannelIE
  • [youtube] comment extraction

Features not added:

  • Improvements to niconico - Meeds python 3.5+
  • [billibilli] Danmaku to subtitles - Licenced under GPL3
  • Split video by chapters - @femaref has a more updated implementation
  • rtmp fix - RTMP is outdated anyway. No point in trying to fix it

PS: I haven't tested a lot of this code yet. Will do it before merging

@bbepis Please go though this and confirm whether you are willing to provide all of this code under Unlicence. I would appreciate it if you could also review the code

@pukkandan
[VideoRemuxer] Conditionally remux video
ae12c9a
@pukkandan
[VideoRemuxer] Add -movflags +faststart when remuxing from mp4
1f87429
@pukkandan
Write playlist data to infojson
4c3632a
@pukkandan
[avanto] Use NFLTokenGenerator if possible
f178bfe
@pukkandan
[ffmpeg] Minor improvements
e912067 
* Print entire stderr in verbose when there is error
* Handle URL filenames correctly
* Warn when embedding ass in mp4
@pukkandan
[FFmpegMetadata] Embed infojson inside the video
496ab66
@pukkandan
[embedthumbnail] Minor improvements
69669ff 
* Treat mka like mkv and mov like mp4
* Embedding in mka similar to mkv
* Try embedding in mp4 using ffprobe and `-disposition`
@pukkandan
[embedthumbnail] Embed in ogg/opus
8fc5bad
@pukkandan
[billibilli] Major Improvements
e5005ec 
* Better video extraction
* Extract comments
* Added BiliBiliSearchIE, BilibiliChannelIE
@pukkandan pukkandan force-pushed the master branch 2 times, most recently from fd17148 to 68debc7 Compare January 24, 2021 08:40
@Jules-A
Copy link
Contributor

Jules-A commented Jan 24, 2021

'ASS subtitles cannot be properly embedded in mp4 files; expect issues'

I'm pretty oblivious when it comes to subtitles but I have experienced some issues with embedding ass in mp4. The problem is I have no idea on the best solution. I tried -remux video mkv but it took ages remuxing videos (maybe 1f87429 will help that) and because it was mkv instead of mp4 it would re-download the file when trying to continue the playlist. Is it possible to get some sort of solution in the warning?

@pukkandan
Copy link
Member Author

pukkandan commented Jan 24, 2021 •
edited
Loading

@Jules-A

I'm pretty oblivious when it comes to subtitles

So am I.

Is it possible to get some sort of solution in the warning?

I also don't know of any possible solutions other than remuxing. If I come across one, I will add it

maybe 1f87429 will help that

I believe so

because it was mkv instead of mp4 it would re-download the file when trying to continue the playlist

I am working on a solution for that. In the meantime, you can use --download-archive

@Jules-A
Copy link
Contributor

Jules-A commented Jan 24, 2021

I am working on a solution for that. In the meantime, you can use --download-archive

Sorry I meant I'm batch downloading videos from a txt file with line-separated urls using a gui program. --download-archive doesn't really have very good documentation and I have no idea what it's doing unless I look at the code...

@bbepis
Copy link
Contributor

bbepis commented Jan 25, 2021

@pukkandan
After reconsideration you have permission to take everything from the repo under Unlicense, including the comment downloading code. Note that there are still some issues with it and currently has a roughly 6000 comment cap, which I've yet to determine the fix for

I think it would be mutually beneficial if you were to receive features that I implement from now on. I've realized I became the thing that I was so adamant against

@Jules-A
-a batch-list.txt is what downloads from line-separated links in a file.

--download-archive archive.txt records the video IDs of every video you've downloaded, such that anything listed in the file is never redownloaded again. It's useful in your situation because it ignores the extension of already downloaded videos

bbepis
bbepis suggested changes Jan 25, 2021
View reviewed changes
Copy link
Contributor

@bbepis bbepis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please let me know what you think about these suggestions

@pukkandan pukkandan force-pushed the al branch 4 times, most recently from 53a0c71 to fbea61a Compare January 25, 2021 11:47
@pukkandan pukkandan marked this pull request as ready for review January 25, 2021 11:52
@pukkandan
linter
8200762 
:ci skip all
@pukkandan
Copy link
Member Author

@bbepis I have added the code for youtube comment extraction as well (with some changes)

@pukkandan
Copy link
Member Author

@bbepis Could you add some tests for BiliBiliChannelIE and BiliBiliSearchIE?

@pukkandan
[bilibili] fixes
558a416 
:ci skip all
@pukkandan pukkandan merged commit 06167fb into master Jan 27, 2021
@pukkandan pukkandan deleted the al branch January 27, 2021 15:37
@pukkandan pukkandan mentioned this pull request Jun 6, 2021
Closed
3 tasks
@pukkandan pukkandan mentioned this pull request Apr 9, 2022
Open
3 tasks
@theandychung theandychung mentioned this pull request Feb 4, 2024
Closed
5 tasks
dannyakb pushed a commit to dannyakb/yt-dlp that referenced this pull request Feb 27, 2025
@felixonmars
[Installation] Update Arch URL (yt-dlp#31)
033fca7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jules-A Jules-A Jules-A left review comments

@bbepis bbepis bbepis requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pukkandan @Jules-A @bbepis