Improve offline capabilities, "fix" subscription import & error messages

[JamesCullum]

1. Refactored Sponsorblock, so that when videos are downloaded, segments are displayed and skipped offline as well
2. Removed unused imports (CTRL+SHIFT+O) and auto-indented (CTRL+SHIFT+L) some files I worked with
3. Reworded the "subscription.json" hint to correctly call it "subscription.csv". There were some reports of this not working (Not able to import my subscriptions from Youtube #1046 & Importing subscriptions not working #1041), but I tested it with the current build and it works. I assume it either was fixed upstream or through a different contribution. Hence we only really need to change the instructions, rebuild and publish to fix this.
4. Fixed error toasts during offline mode - simply saying that the internet connection is unavailable is better than using the technical error. Also an error with "Javascript player" is not useful and the toast is suppressed.

[gzsombor]

About the imports - there are a couple of file formats which is supported by NewPipeExtractor, json, csv and even zip, and I've fixed a couple of bugs in my latest commit, but I still received exceptions with the zip format, so I haven't closed the tickets yet.

[JamesCullum]

I see - thanks for the clarification! With CSV it works fine for me, and with the updated phrasing this is what most users would expect to select (before looking into the code, I didn't know ZIP was even supported).

Ready to merge then?

[gzsombor]

Sorry for not replying earlier, I have a couple of concerns:

• generally updating the translations directly could be a pain, as translator could modify those files in weblate, and the only way to interact with that service, is to call "https://hosted.weblate.org/hooks/update/skytube/" - then the weblate service tries to fetch+merge the latest files from this repository. It could fail, then you need to do manually. It's not end of the world, but some extra work is needed.
• As NewPipeExtractor supports csv/json and zip formats, probably it would be better, to change the label accordingly (sure, the zip file import, maybe still buggy 😞 )
• About serializing YoutubeVideo objects into jsons and storing in the database - honestly, I'm not a big fan of that. The problem is, that it's much harder to modify the code, when you need to keep in mind, that YoutubeVideo is also used as a storage format, so we can't add fields which are non-nulls, we can't modify fields type, etc. One other problem, is that with json columns, you can't filter, sort, update the tables. I've already converted the subscriptionVideos table to a more flat structure, so we (or I) can in the future implement searching/filtering/categorization of videos. This is not against your patch, as I see, that you've added a separate column for the SBVideoInfo to mitigate the problems, but this also means that SBVideoInfo is stored twice in that table, once, inside the serialized YoutubeVideo json, and once in a separate column. And as a lesser concern, that it will be also stored in the bookmarks table. I understand, why you've added SBVideoInfo into YoutubeVideo - actually the two are heavily linked, but this makes certain things uglier. For example, when you get the SBVideoInfo from YoutubeVideo, and it's null, you wont know : do we already tried to retrieve the SBVideoInfo from the network? Should we try it again? Maybe, we tried, but the service returned a 404 - or there was a DNS problem, or a connection refused, maybe we won't tried, because that YoutubeVideo comes the database. And there could be also valid questions, if we get back an SBVideoInfo from YoutubeVideo - maybe that info was retrieved couple of days ago, since than, there could be changes, so it could be worth checking again (I'm not sure, if it's a valid scenario that those segments are edited heavily). Maybe instead of putting SBVideoInfo into YoutubeVideo, we can create a wrapper class, which contains: YoutubeVideo, an SBVideoInfo, and some metadata, to distinguish between the cases, when we should try to refetch from the network the latest blocks, and when we shouldn't. What do you think?
• As the network could potentially block the UI for seconds - or more - could you please add SkyTubeApp.nonUiThread() calls which throws an exception if that happens. So we will notice earlier, if we introduce a potentially blocking behaviour - and we can fix it quickly 😉

[JamesCullum]

generally updating the translations directly could be a pain

So this means we should change only english, or modify english as well only via weblate?

As NewPipeExtractor supports csv/json and zip formats, probably it would be better, to change the label accordingly

Agreed - done already

About serializing YoutubeVideo objects into jsons and storing in the database - honestly, I'm not a big fan of that.

Agreed, this method for the current approach might make sense to change going forward. Not sure if this needs to happen as part of this PR, as this is an already existing implementation.

but this also means that SBVideoInfo is stored twice in that table

It actually isnt - the download does not set SBVideoInfo for the youtube object itself, only for the new column.

For example, when you get the SBVideoInfo from YoutubeVideo, and it's null, you wont know : do we already tried to retrieve the SBVideoInfo from the network? Should we try it again?

The information is only retrieved in two scenarios - once when the video is downloaded (then its cached), or everytime the video is played. As the request is only executed once, there is not really a need for advanced data handling - if the file is cached, we expect the data to be embedded (or null, if download failed), otherwise we request it and if the data is valid, we load it.

And there could be also valid questions, if we get back an SBVideoInfo from YoutubeVideo - maybe that info was retrieved couple of days ago, since than, there could be changes, so it could be worth checking again

You are right that theoretically this would be a concern. However as mentioned above - this would only happen for downloaded videos, for which I assume users expect to load data offline and save data as much as possible (if an internet connection is available at all). If someone wants to refresh the download video, they can delete and re-download the video. We could try to pull updated data for downloaded videos as well, if we want that.

Maybe instead of putting SBVideoInfo into YoutubeVideo

I think as I don't store the blob as video anymore (first draft I did), I can just remove it from the YoutubeVideo and deal with it directly in the player.

could you please add SkyTubeApp.nonUiThread()

Sure, will do

[gzsombor]

If you opened up the YoutubePlayerActivityV2, then the code retrieved that sponsorblock segment, and set it on the YoutubeVideo object. If you later clicked on the download menu item, that YoutubeVideo object was added to the download database. But maybe, I've screwed up something during my debugging.
Thank you for your patch, it looks great :)

[JamesCullum]

Thanks for merging!

I suspected that it would do that, but during my tests this wasnt the case. Maybe you waited for the video to finish completely loading? Anyways, now the issue is fixed either way and the code is more clean this way anyways, thanks for the discussion 👍