Major performance and bug fixes to downloads #1164
Conversation
I tracked down underlying issues to setPersistentId. So I instead removed that entire function here and instead we send the byte data from the view model to it. This also does minor improvements to view model. This also fixes the underlying cause of many bugs like the byte mismatch and icon bug, while my previous fix for that was just a patch-job, this should fix the underlying cause of them as well. Overall when testing this makes downloads load with 100% zero lag now.
|
I just realized this has some big issues with it... my bad... |
|
There has got to be a way to make it work... but this way breaks in progress downloads indicator and download progress in EpisodeAdapter, etc... so is not the way to do it. But there must be a way to make this performant... |
Try doing it in a IO context then switch back to main thread as getDownloadFileInfoAndUpdateSettings checks the file on disk and doing that on the main thread lags it. |
…more in IO context This makes it use the view model more when in downloads view, because otherwise when running savedData in an IO context it causes it to wait before updating progress which causes visible UI delays. This prevents that with downloads with the IsDone status. This also has a UI change: If a download is complete, it no longer shows bytes / bytes, it only shows bytes once. This is both a UI and performance improvement to have to do less. Finally, this does some cleanup to DownloadAdapter.
Done but a bit different as when just doing that it caused a delay where all icons showed the default download icon, for a noticeable time when opening downloads so I made it use the view model where possible. If you want me to do this another way I can certainly give it a try. |
This does work for me when testing though... in both places... |
No. Try downloading an episode in a long running series and then flip between the seasons. I was able to reliably make it show it is currently downloading an episode (or several) on other seasons from 1 download. You MUST use something like |
|
Properly using recycleviews without invalid data or memory leaks can be a real PITA to find, debug and resolve. Therefore it is extra important to verify that all UI is re-rendered on bind if it is a different item. |
|
I can't seem to reproduce that but I do see how the issue could potentially happen. I will push a fix in a little bit. |
|
@fire-light42 I think the issue you had may be fixed, but I still could not reproduce to confirm |
There was a problem hiding this comment.
The code works and looks good. However I found an issue that also needs fixing, after further testing I found a bug that has been in cs3 for some time. It might be good to solve it, however I am now unsure if it is UI or downloader related
The issue is as follows:
- Go into a resultpage with multiple seasons
- Download any episode
- Force quit
- Reenter the app and all download will automatically resume
- Go into the same series
- Pause that download
- Switch to another season
- Switch back to the season
If you do this correctly then you will see that the progress on the paused download does not match with the progress downloaded and it will need to resume to update to the correct progress.
error.mp4
app/src/main/java/com/lagradost/cloudstream3/ui/download/DownloadAdapter.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Ye, the issue I found is not related to this or your last pr but might need an real https://github.com/recloudstream/cloudstream/issues issue.
That's a good catch. I will try and fix that in a follow-up as well. But I'd rather not further complicate things in this PR and I'm not sure how to fix it at the moment. |
I tracked down underlying issues to setPersistentId. So I instead improved this using IO context, and this makes it use the view model more when in the downloads view, because otherwise when running savedData in an IO context it causes it to wait before updating progress which causes visible UI delays. This prevents that with downloads with the IsDone status.
This also has a UI change: If a download is complete, it no longer shows bytes / bytes, it only shows bytes once. This is both a UI and performance improvement to have to do less. Finally, this does some cleanup to DownloadAdapter and DownloadViewModel as well and fixes the display of the no downloads messages which previously seemed to have intended implementation but did nothing.
This also fixes the underlying cause of many bugs like the byte mismatch, while my previous fix for that was just a patch-job, this should fix the underlying cause of them as well.
Overall when testing this makes downloads load with almost no lag at all now.