jellyfin
Start work on offline download support
This PR was inspired by @JacobKingDev. It improves upon the current framework for media downloading provided by @LePips. It is far from finished, and I intended to improve on it in the coming weeks. I would love any feedback, as this is the first time I'm working with Swift, so my code is probably riddled with nonsensical rubbish. That being said here is a list of the changes I've implemented.
- Fixed negative percentage with progress text while downloading (the way I get the excepted size is kind of hacky, I would love pointers on how to improve it)
- Improved design of download list view
- Added size label to download item view (added extension to the FixedWidthInteger extension)
- Added temporary OfflineTabCoordinator for testing I'll try to integrate it with the MainTabCoordinator ASAP
- Changed the default behaviour when a NSURLErrorNotConnectedToInternet to showing a view using the OffileTabCoordinator.
- Added the ability to remove downloaded media
I'm glad that this is able to pick up starting from my latest work, but is there a reason why we didn't ask on the previous draft PR to continue the work elsewhere?
I'm sorry I don't quite understand what you mean by:
but is there a reason why we didn't ask on the previous draft PR to continue the work elsewhere?
There is already a pre-existing PR for working on downloads. While it has been a while since work was done on it, and that's okay, now we have 2 PRs for the same feature.
I'm going to have to ask you to communicate with @JacobKingDev about taking over the feature, or at the very least it would have been nice ask if there was interest to continue the previous work. Even if after a few days there isn't an answer, then another PR would have been fine.
@LePips I have asked in the previous PR, but I'm yet to get a response. In a mean time, I've completed a very rough version of my idea of the downloads section. It can be enabled in the experiments section, and I'd love some feedback on how you think it looks and feels. The code for the UI is largely a verbatim copy of the ItemView group (there is a lot of duplicate code that can be removed) the reason I did it like this is I wanted to match the current design as much as possible. If you have any feedback on the rest of the code or the way I've decided to handle the downloads, please let me know.
What currently works:
- Downloading any episode or film
- Both online and offline playback of downloaded media
- Properly grouping episodes on season and series
- Resume items, Next up items and adjacent items for downloaded media
What doesn't work (yet) / what I haven't got around to:
- Playback progress syncing
- Estimated time left on download
- Batch downloading (e.g. season)
- Proper support for different users
- Picking specific media source
It will take me a while to look at this as I work on the video player again. Due to that, I see that some things will probably have to be rewritten, but I don't anticipate it to be too much. I will also have to think a lot about the overall architecture that my initial work had and whether it needs to be redone.
With two forks already and multiple folks wanting to pitch in (myself included), I think it would be helpful to have these PRs put into a feature branch until it's ready for main. Ideally we will see smaller change-sets, too.
@chickdan I would like that. If you want to make a feature branch, I would be happy to contribute.
I'd also like to contribute if a feature branch could be created. I am unsure how ios will handle transcoded downloads and if it will have the same issue as the ExoPlayer on android (transcoded downloads have malformed data from ffmpeg.) More details can be had here: https://github.com/jellyfin/jellyfin-meta/discussions/66
Seems like BenStokamns repo has gone stale but I would love to help if he is still working on it.
Hi everyone, good to see so much interest for this feature. Currently, I am on holiday until late August. The way the offline feature is currently written doesn't really sit well with me and would need a significant refactor to make it work nicely with the existing code base. If you still wish to contribute you can make pull requests to the downloads branch in my repo which I will still check and merge.
Hi Ethan thanks for your response it reminded me that I should make some time to look at this again sometime soon. To address your comments, yes, as I mentioned a lot is copied and is redundant, this was because I wanted a working version quick and dirty before a holiday. That being said, yes views can be created based on BaseItemDto's but any functionality derived from that class automatically attempts to retrieve data from the server which is of course not available when offline. So we would have to find a solution for that that covers both online and offline usage. What that would look like I do not know. If you have any suggestions I would love to hear them.
Also small side note. I plan to discard almost all changes I made as I have since realized that I made a huge mess.
That sounds good to me, especially since there were many changes since the beginning of this PR and even more especially with app navigation/server connection expectations. If you would like, you can wait until after #1203 so I can assist with how to use the video player.
Hi @BenStokmans @LePips , I want to check if this PR is still active? I am working on offline support and Im not sure if I should contribute to this PR or open new one since there was no activity lately. For me is this feature crucial for switching to this app from paid one so I am actively working on this.
Hi @BenStokmans @LePips , I want to check if this PR is still active?
Any work on downloaded playback should wait until after https://github.com/jellyfin/Swiftfin/pull/1581. This was originally the aforementioned #1203. This PR changes how playback is called/used so any work done today would need to be reworked after #1581 so it's more prudent to hold off on that portion until completed.
This PR also comes after our routing changes so there is likely work needed for the PR to resolve outstanding build issues and make this functional in a post-NavigationView Swiftfin.
I don't want to speak on behalf of anyone, purely just my thought on this, but when I made our Help Wanted post, I felt this could be broken into 4 items here: https://github.com/jellyfin/Swiftfin/discussions/1503. That is:
- Downloading Content - Single items, seasons, shows, etc. along with handling for straight file copies vs transcoded downloads. We have our transcoding profiles available that should be used to prevent playback issues. Special consideration needs to be given for items like Collections or People where the full list of items under them is paginated.
- Downloads UI - Ability to browse downloads in Swiftfin via the existing views for our
ItemView,PagingItemView, etc. where possible, reusing existing item views and library views is important to help make this feature maintainable. This somewhat ties into 1) as we'll need to ensure we're downloading enough metadata and formatting in such a way that we can reuse our existing structure. - Manage Downloads - Ability to delete, manage storage, etc with some other QoL items like scheduled downloads or deletion on watched.
- Download Playback - This would be a holdout until after #1581. It should be a fairly small item once the rework is complete but for now this would have to wait.
I'll defer to @BenStokmans on whether they want to proceed with this current PR but, for ease of review/rollback/etc, I personally think this would be best to split this out into a few more manageable PRs. The download/browsing elements can be done independently of playback while #1581 is still in the works. For now, placing the navigation to downloads behind an experimental flag. Alternatively, working on a downloads specific branch.
I think we unfortunately have competing priorities with the playback rework, tvOS release, & downloads right now. So, it may also make sense to keep the pause on downloads to prevent conflicts until after #1581?
This is all a long way of saying, @LePips 's perspective should take priority over mine but I think there is room to divide and conquer here.
Sorry for the giant non-answer but I just thought I would consolidate a few of the comments I have from a couple different random places/discussions. But, whatever direction we feel like we're taking, I'd be happy to contribute as well! Primarily, I just want to avoid stepping on any toes/creating difficult merge conflicts.
@JPKribs
Thanks for the quick response! I’m all in on breaking this into smaller PRs to keep things smooth. Here’s what I had in mind:
- Download Manager
- Handle file retrieval of original and transcoded files and other metadata.
- Background downloads: ensure the process continues even if the app is restarted.
- Manage offline files: deleting old files, scheduling cleanups, etc.
- Downloads UI
- A simple list grouped by show and season, like Netflix or DisneyPlus - more complex UI can be implemented in the future.
- No extra metadata (people, collections) in the first version.
- Download Playback
- Add offline playback once #1581 is merged.
I would also propose to merge and release even small not fully finished features - like basic download functionability for offline playback - without advanced features, they can be finished later. This way features would be shipped faster and users can test it. Also with smaller releases there is smaller chance for breaking app. Also there will be better user feedback and can gather more info about more advanced features what users need.
Let me know what you think! 🚀
Hey all, I'm closing this out in favor of a more modular approach. Downloads are way too big to tackle in a single PR so I've broken this out into more stages, as sub-issues here:
https://github.com/jellyfin/Swiftfin/issues/57
I am going to close out this PR in favor of that approach but if folks have interest in carrying this forward, please let me know on one of those sub-issues.
Download Manager work (the first part that should be done) can start now. Any UI of the downstream UI work should hold off until the library rework is done here:
https://github.com/jellyfin/Swiftfin/pull/1752