Skip to content
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.

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

Ignore failed/residual/stuck video downloads: download only newly requested one(s) #147

Merged
merged 5 commits into from
Apr 11, 2024

Conversation

deldesir
Copy link
Collaborator

No description provided.

Videos are sorted by views-per-day and limited by MAX_VIDEOS_PER_DOWNLOAD
size and time_uploaded metadata are missing for YouTube Shorts. We don't count them, failed videos too.
…sion

Other users might be testing or some videos did not download from a previous session. Focus on the requested video(s).
@deldesir deldesir marked this pull request as draft March 27, 2024 05:39
@holta
Copy link
Member

holta commented Mar 27, 2024

What is meant by "trailing videos" ?

(Thanks for explaining!)

@holta holta added the enhancement New feature or request label Mar 27, 2024
@deldesir
Copy link
Collaborator Author

deldesir commented Mar 27, 2024

Videos from another session that were not downloaded or failed for some reason. It might be leftover videos resulting from testing lb-wrapper or from a parallel downloading in case of a multi-user use case.

@holta holta changed the title Ignore trailing videos and download only the requested one(s) Ignore failed/residual/stuck video downloads: download only newly requested one(s) Mar 27, 2024
@holta
Copy link
Member

holta commented Mar 27, 2024

Is the new subject line tolerable?

(Improve on it if nec!)

@deldesir
Copy link
Collaborator Author

deldesir commented Mar 27, 2024

Looks good to me

@deldesir deldesir requested a review from holta March 28, 2024 02:16
@deldesir deldesir self-assigned this Mar 28, 2024
@holta
Copy link
Member

holta commented Mar 28, 2024

Is this PR tested?

Is this PR closely related to... ?

@deldesir
Copy link
Collaborator Author

It's being tested right now.

Is this PR closely related to... ?

Yes. Additionally, xklb considers the playlist already downloaded, same issue we encountered when redownloading single videos. I will address this with a similar fix, adding a timestamp to every playlist downloaded. I have yet to test a channel to check for this behavior.

@deldesir deldesir marked this pull request as ready for review March 28, 2024 02:43
@deldesir
Copy link
Collaborator Author

Tested with #151. Ready for merge.

@holta
Copy link
Member

holta commented Apr 11, 2024

Should this be merged now?

Or better to wait a bit?

Copy link
Collaborator Author

@deldesir deldesir left a comment

Choose a reason for hiding this comment

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

Yes. This works [well] enough.

@holta holta merged commit 4512bf6 into iiab:master Apr 11, 2024
@deldesir deldesir deleted the deldesir-patch-22 branch July 1, 2024 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants