Handle restricted/unavailable videos [and list them in "Tasks" view, when downloading (stale!) playlists] #179
Conversation
Unavailable videos do not have a duration
deldesir
changed the title
|
|
Yes.
This PR deals with these issues specifically. As a debugging aid, I list all unavailable videos that used to block a playlist download at the bottom of the task message. I think this is useful to test the reliability of this PR. You can see an example in the tasks page of LRN2 with @nzola's playlist of playlists. |
|
I will disable the listing of unavailable videos in the tasks page before this PR is merged. Let me know what you think. Update: Oops, already merged. If the list needs to go away, let me know please. |
"Tasks" view list of unavailable videos seems very useful, no? Example below from https://box/live/tasks on server LRN2:
|
|
I don't find them useful per se, especially when they are more than a couple. I was thinking of putting the bookshelf title link here instead. |
Librarians & Archivists & Curators very much want to know how many videos (and which videos) were "disappeared" by hosting sites like Google/YouTube. That info needs to be available without too much hassle, and http://box/books > "Tasks" view is a very good place for it in my opinion. Regardless whether video(s) disappeared due to censorship, copyright, privacy, geo-block, or any other reason (-:
Certainly, that should also be included somewhere in "Tasks" view, yes please if possible! |
holta
changed the title
holta
changed the title
|
Can you give this PR a quick review for suggestions? (Even though it was merged urgently earlier this morning, to help users & testers!) |
| requested_urls = {row[0]: {"duration": row[1]} if row[1] is not None and row[1] > 0 else self.unavailable.append(row[0]) for row in rows} | ||
| for url in self.unavailable: | ||
| requested_urls.pop(url) | ||
| return requested_urls |
There was a problem hiding this comment.
Totally untested code that seems simpler and more readable than the list comprehension with conditionals.
requested_urls = {}
for path, duration in rows:
if duration is not None and duration > 0:
requested_urls[path] = {"duration": duration};
else:
self.unavailable.append(path)
return requested_urls
There was a problem hiding this comment.
@deldesir I'm in favor of simple + more readable if you can consider?!
There was a problem hiding this comment.
Done! In 2 steps...
| return | ||
| self.message = f"{self.media_url_link} failed: Video not available." | ||
| conn.execute("DELETE FROM media WHERE path = ?", (self.media_url,)) | ||
| self.stat = STAT_FAIL |
There was a problem hiding this comment.
This gets overridden with STAT_FINISH_SUCCESS at line 178. Is it ok?
There was a problem hiding this comment.
Also, previously this line was skipped when requested_urls is empty, now it's executed every time:
self._add_download_tasks_to_worker(requested_urls)Are there any unwanted side-effects?
There was a problem hiding this comment.
Yes. STAT_FINISH_SUCCESS and STAT_FAIL both end the task.
There was a problem hiding this comment.
Also, previously this line was skipped when requested_urls is empty, now it's executed every time:
self._add_download_tasks_to_worker(requested_urls)Are there any unwanted side-effects?
This one send every requested_url to the download task. It could be done in a single task, but was discarded in favor of individual task for every download. See #99
There was a problem hiding this comment.
The list of requested URLs is retrieved from xklb-metadata.db. It's basically a search for every row which have no text in its error column and path is a URL.
This list can be empty if these conditions are not met.
There was a problem hiding this comment.
The list of requested URLs is retrieved from xklb-metadata.db. It's basically a search for every row which have no text in its error column and path is a URL.
This list can be empty if these conditions are not met.
My original question was: is it ok to call _add_download_tasks_to_worker() even when requested_urls is an empty list?
I looked at the body of _add_download_tasks_to_worker(), and it does nothing when called with an empty list.
So... OK.
This PR handles various issues related to restricted/unavailable videos. Those videos won't fail an entire playlist anymore.
N.B This PR should also fix the hanging upon redownloading a failed video.
Tested on Ubuntu 24.04 with playlist https://www.youtube.com/playlist?list=PLfiHW0cuXSvQSZHsA5zpo3JdrY5EJp1yT