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

Enable subsequent [re]downloads of playlists #151

Merged
merged 5 commits into from
May 31, 2024

Conversation

deldesir
Copy link
Collaborator

@deldesir deldesir commented Mar 28, 2024

Fixes #150. This PR adds a timestamp to playlists' paths after they are downloaded.

[ 2024-05-30 SUMMARY: xklb generally does not support subsequent redownloading of playlists, so let's try this workaround! ]

Fixes iiab#150, enabling subsequent [re]downloads of playlists
@deldesir deldesir added the enhancement New feature or request label Mar 28, 2024
@deldesir deldesir requested a review from holta March 28, 2024 02:55
@deldesir deldesir self-assigned this Mar 28, 2024
@deldesir deldesir marked this pull request as draft March 28, 2024 03:03
@deldesir deldesir changed the title Enable subsequent [re]downloads of playlists Enable subsequent [re]downloads of playlists [WIP] Mar 28, 2024
This prevents UNIQUE CONSTRAINT error
@deldesir deldesir changed the title Enable subsequent [re]downloads of playlists [WIP] Enable subsequent [re]downloads of playlists Mar 28, 2024
@deldesir deldesir marked this pull request as ready for review March 28, 2024 03:41
@deldesir
Copy link
Collaborator Author

Tested with subsequent [re]downloading of a small playlist. Couldn't test with long playlists due to potential disk IO error.

image

@EMG70
Copy link

EMG70 commented Mar 28, 2024

@deldesir
Copy link
Collaborator Author

deldesir commented Mar 28, 2024

Hi @EMG70. Reading your log, it looks like xklb-metadata.db is missing. Can you check by doing ls /library/calibre-web/xklb-metadata.db using your terminal?

@holta
Copy link
Member

holta commented Mar 28, 2024

Hi @EMG70. Reading your log, it looks like xklb-metadata.db is missing. Can you check by doing ls /library/calibre-web/xklb-metadata.db using your terminal?

Or run this as root...

apt install tree
cd /library/calibre-web
tree

@EMG70
Copy link

EMG70 commented Mar 28, 2024

Hi @EMG70. Reading your log, it looks like xklb-metadata.db is missing. Can you check by doing ls /library/calibre-web/xklb-metadata.db using your terminal?
Here is the output
Screenshot from 2024-03-28 18-31-47

@deldesir
Copy link
Collaborator Author

deldesir commented Mar 28, 2024

Line 1090 in your log says you don't have XKLB installed. How come you have the database and no XKLB installed? Weird!

@deldesir
Copy link
Collaborator Author

To confirm run:
which lb

If it is found, do

lb --version

@EMG70
Copy link

EMG70 commented Mar 28, 2024

Line 1090 in your log says you don't have XKLB installed. How come you have the database and no XKLB installed? Weird!

Allow me to reinstall and try again in case i missed something

@deldesir
Copy link
Collaborator Author

Please go ahead, no worry.

@EMG70
Copy link

EMG70 commented Mar 28, 2024

To confirm run: which lb

If it is found, do

lb --version

sorry a bit late now,VM destroyed.

@EMG70
Copy link

EMG70 commented Mar 28, 2024

Please check if all is ok before proceeding with testing.

image

@deldesir
Copy link
Collaborator Author

Looks good so far

@holta
Copy link
Member

holta commented Mar 28, 2024

Plz post ideas(s) as to why the earlier XKLB install failed / disappeared, if possible?

@EMG70
Copy link

EMG70 commented Mar 28, 2024

IIAB-DIAGNOSTICS - http://sprunge.us/tjeC5u?en

Downloading and video ranking showing OK .Subsequent playlist downloads all failed no matter number of videos in playlist,however a single video downloads well.
IMG_3162

This playlist downloaded all 53 videos ok but failed on second attempt. https://www.youtube.com/playlist?list=PLr6-GrHUlVf96NLj3PQq-tmEB6woZjwEl
https://www.youtube.com/playlist?list=PLiMWaCMwGJXlQj0BFEwew0fvhOPr833GS 7 videos fails

Screenshot from 2024-03-28 22-14-44

@deldesir
Copy link
Collaborator Author

deldesir commented Mar 29, 2024

I cannot reproduce your issue @EMG70, sorry. Both Playlists work for me, all videos were downloaded, except 1 (https://www.youtube.com/watch?v=7m4_kZOObzw). I am afraid you might be subject to some restrictions either by your ISP or YouTube itself

@EMG70
Copy link

EMG70 commented Mar 29, 2024

You could be right.I have not done any testing since I changed ISP two weeks ago.
I will try and see what's going on.

@holta
Copy link
Member

holta commented Mar 29, 2024

might be subject to some limitations either by your ISP or YouTube itself

We can and should arrange for @EMG70[*] to do the exact same test remotely (i.e. with a different ISP and different YouTube geographic restrictions) to confirm what exactly is happening and why.

[*] And others too, if possible!

@deldesir can you help spell out the exact steps of what most needs to be tested, to help everyone avoid ambiguity here?

@deldesir
Copy link
Collaborator Author

We're dealing with a download issue. We know we have a successful test if we provide 2-3 playlists one after the other and the following conditions are met for them:

  • metadata are fetched and total duration is displayed
  • videos are downloaded (a few failures are ok)
  • shelves are created and populated

@holta
Copy link
Member

holta commented Mar 29, 2024

We know we have a successful test if we provide 2-3 playlists one after the other and the following conditions are met [,,,]

Thanks for explaining.

Are we also trying to test: repeatedly entering the same playlist URL?

(i.e. clicking "Download to IIAB" each time, and entering the same YouTube playlist URL each time, a few times in succession)

@deldesir
Copy link
Collaborator Author

deldesir commented Mar 29, 2024

I looked into t5. Everything looks Ok. Can't spot the issue right now. I will delete xklb-metadata and run the playlists again.

@deldesir
Copy link
Collaborator Author

deldesir commented Apr 9, 2024

Thanks a lot for testing again. The UNIQUE constraint error is a database issue. I'll let you know more once I am done examining your log.

@EMG70
Copy link

EMG70 commented Apr 9, 2024

Sorry did i post on wrong PR.I have just posted same on PR#150

@EMG70
Copy link

EMG70 commented Apr 9, 2024

Thanks a lot for testing again. The UNIQUE constraint error is a database issue. I'll let you know more once I am done examining your log.

IIAB-DIAGNOSTICS - http://sprunge.us/Kv2wBX?en
https://www.youtube.com/playlist?list=PL_c9BZzLwBRLVh9OdCBYFEql6esA6aRsi - 102 short videos in playlist ✅
https://www.youtube.com/playlist?list=PLr6-GrHUlVf96NLj3PQq-tmEB6woZjwEl - 53 short videos in playlist ✅
97/102 Videos were downloaded OK.A few videos failed https://www.youtube.com/watch?v=4e3dIeP93E8 and https://www.youtube.com/watch?v=D2KvDBSZA0w although they do play well on Youtube.I am not sure why,see screenshots:
Screenshot from 2024-04-09 14-31-01
Screenshot from 2024-04-09 14-55-18
Screenshot from 2024-04-09 15-03-05
Screenshot from 2024-04-09 15-03-36

@deldesir
Copy link
Collaborator Author

deldesir commented Apr 11, 2024

For the 102 videos url (https://www.youtube.com/playlist?list=PL_c9BZzLwBRLVh9OdCBYFEql6esA6aRsi - 102 short videos in playlist) I also had 3 failed videos:

We have one in common: https://www.youtube.com/watch?v=4e3dIeP93E8

When I forced the redownload of any failed video I got the UNIQUE constraint failed: media.path error. It's because :
1- no error was reported by xklb for the failed video. It's like the video succeeded and even got a download/local "path"
2- since the video really failed, calibre-web did not have it, so no timestamp was added to the path

image

@holta
Copy link
Member

holta commented Apr 11, 2024

Thanks @deldesir for explaining the progress:

Is targeted testing needed (e.g. from @EMG70) in coming days?

@deldesir
Copy link
Collaborator Author

deldesir commented Apr 11, 2024

Redownloading a playlist twice still doesn't work as expected. This will require some adjustments for sure. Hope I fix it today or tomorrow so Ed can battletest it.

@deldesir
Copy link
Collaborator Author

deldesir commented Apr 15, 2024

@EMG70 and @holta, I just got 100% success downloading and redownloading https://www.youtube.com/playlist?list=PL_c9BZzLwBRLVh9OdCBYFEql6esA6aRsi. What changed? No code additions, I only updated my branches based on iiab:master and resolved 2 merged conflicts that led to the closing of #146. I also notice the VM where failures from redownloading happened did not have this commit d9e4fc4. Examining the [ xklb-metadata.db ] confirmed this because the downloaded playlists do not have any timestamps added to them.

http://sprunge.us/lPRkPg

@holta
Copy link
Member

holta commented Apr 15, 2024

@deldesir what do you suggest?

@deldesir
Copy link
Collaborator Author

I'd like a second look into this. I want to be sure this works for @EMG70 too.

@holta
Copy link
Member

holta commented Apr 15, 2024

Thanks @deldesir:

Please make a recommendation if additional testing is required?

@deldesir
Copy link
Collaborator Author

deldesir commented Apr 15, 2024

A clean test environment @holta.
@EMG70, If you plan to test this again, ensure you don't include #152. I don't understand yet what went wrong so a clean VM with only #151 is recommended. I have to acknowledge we benefit from the enhancements made recently to yt-dlp and xklb so your test will probably use:

  • yt-dlp 2024.04.09
  • xklb 2.6.009

@EMG70
Copy link

EMG70 commented Apr 15, 2024

I will test tomorrow and update you.

@deldesir
Copy link
Collaborator Author

@EMG70 and @holta, downloading channel with this PR is not solid yet due to "Shorts" not having time_uploaded. I'll let you know once I'm done handling this in a better way. For your tests, please consider playlists only for now.

@EMG70
Copy link

EMG70 commented Apr 16, 2024

IIAB-DIAGNOSTICS - http://sprunge.us/Rf3QgC?en

Screenshot from 2024-04-16 20-36-29
There were two failed videos ,one of them is 9hrs,not sure why the content creator included it as part of a playlist JavaScript Programming All-in-One Tutorial Series (9 HOURS!) https://www.youtube.com/watch?v=9M4XKi25I2M .
This probably explains why Task output seemed to be hung (failed to download:download appears to be stuck)

The second video that failed https://www.youtube.com/watch?v=NBRdd18N5X0 was also tried individually and still returned a failed output in Task ( failed to download:UNIQUE constraint failed.media.path).
Screenshot from 2024-04-16 20-47-13

@deldesir
Copy link
Collaborator Author

deldesir commented Apr 17, 2024

Happy this is getting more robust. Thanks @EMG70.

The UNIQUE constraint failed.media.path is not difficult to tackle. It's because the stuck videos are not post-timestamped. I'm looking for an elegant way to fix it 🤔.

@holta
Copy link
Member

holta commented May 30, 2024

@EMG70 we're hoping a quick test with YouTube Shorts (@deldesir will help) confirms this PR can be merged quickly, ideally today!

@holta
Copy link
Member

holta commented May 31, 2024

This PR was tested with #155 and #157 on t4 as explained here:

@deldesir confirms this PR #151 is ready.

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.

Subsequent playlists redownloads fail
3 participants