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

Remove residual fragments from failed video #157

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deldesir
Copy link
Collaborator

When a video failed due to unavailable fragments, a subsequent redownload will try to use the residual fragments from the previous download. This is because the download process did not go through to the point to have a video file and a thumbnail downloaded. Since the media.path did not synchronize with the actual calibre-web book path and no timestamp wasn't applied to the media.webpath, these residual fragments affects subsequent redownloads, thus their removal.

When a video failed due to unavailable fragments, a subsequent redownload will try to use the residual fragments from the previous download. This is because the download process did not go through to the point to have a video file and a thumbnail downloaded. Since the media.path did not synchronize with the actual calibre-web book path and no timestamp wasn't applied to the media.webpath, these residual fragments affects need to be removed.
@deldesir deldesir added the bug Something isn't working label Apr 17, 2024
@deldesir deldesir self-assigned this Apr 17, 2024
@deldesir deldesir marked this pull request as draft April 17, 2024 12:29
@holta
Copy link
Member

holta commented Apr 17, 2024

Awesome if diagnostic understanding of yt-dlp and xklb is progressing!

(Any prior tickets we should link to for community context?)

@holta
Copy link
Member

holta commented May 30, 2024

@EMG70 just FYI progress is cooking, as we try to release for you ASAP:

  1. This PR (Remove residual fragments from failed video #157) is also high priority to be tested and merged, alongside:

  2. These 2 URLs from Subsequent playlists redownloads fail #150 appear to be most useful for testing playlist redownloading:

    Whereas these 2 other URLs might not reproduce the problem??

ASIDE: @deldesir tested different versions of yt-dlp a month ago, and they generally ALL WORKED — so let's not over-interpret this 1 data point on #158:

@deldesir
Copy link
Collaborator Author

deldesir commented May 31, 2024

I tried to downloaded https://www.youtube.com/playlist?list=PL_c9BZzLwBRLVh9OdCBYFEql6esA6aRsi (102 videos) into t4 (Ubuntu 24.10) using #151, #155, #157. This 9-hour video failed to download:

Download: https://www.youtube.com/watch?v=9M4XKi25I2M failed to download: 'NoneType' object is not subscriptable

@holta
Copy link
Member

holta commented May 31, 2024

Full results (from VM t4 above) — these 7 of 100 videos failed to download:

The other 6 videos were not successfully copied in the filesystem — and likewise each has the wrong path in xklb-metadata.db (/library/downloads/calibre-web instead of /library/calibre-web). These 6 failures might be a timeout issue, possibly related to... ?

CLARIF / JUST FYI: Only 100 of 102 were attempted, due to

# Maximum number of videos to download, from a playlist or channel
MAX_VIDEOS_PER_DOWNLOAD = 100

@deldesir
Copy link
Collaborator Author

image

@holta
Copy link
Member

holta commented May 31, 2024

@deldesir and I agreed to dig deeper before merging this PR:

We need to try to understand the above timeout-like issue, which intermittently prevents videos from being copied from /library/downloads/calibre-web to /library/calibre-web (and likewise fails to update their path in xklb-metadata.db).

Very likely related:

@holta
Copy link
Member

holta commented Jun 13, 2024

@deldesir resolve this merge conflict?

(To keep this PR #157 discussion active!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants