[PR #1587] Android download rewrite #1627

Open
opened 2026-04-24 23:59:48 +02:00 by adam · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/advplyr/audiobookshelf-app/pull/1587
Author: @nichwall
Created: 5/26/2025
Status: 🔄 Open

Base: masterHead: android_download_rewrite


📝 Commits (10+)

  • feef527 Delet unused file
  • ee0ab43 Auto formatting AbsDownloader.kt
  • 4a09238 Refactor AbsDownloader
  • 19b1f3b Remove: unused json file from FolderScanner
  • 5d8574e Simplify local library item creation in folder scanner
  • f31a8f0 Revert "Refactor AbsDownloader"
  • 841527e Simplify AbsDownloader download item creation
  • 640d8cd Combine cover download logic
  • 7e2ac27 Fix: cover printout for book
  • e11b8cb Initial internal download continue incomplete download

📊 Changes

8 files changed (+766 additions, -844 deletions)

View changed files

📝 android/app/src/main/java/com/audiobookshelf/app/data/DeviceClasses.kt (+133 -113)
android/app/src/main/java/com/audiobookshelf/app/data/FolderScanResult.kt (+0 -15)
📝 android/app/src/main/java/com/audiobookshelf/app/device/FolderScanner.kt (+189 -415)
📝 android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt (+82 -120)
📝 android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt (+80 -38)
📝 android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt (+69 -50)
📝 android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt (+212 -92)
📝 strings/en-us.json (+1 -1)

📄 Description

Brief summary

This PR refactors the Android download logic to use a single code path for both Internal App Storage and Shared Storage. This PR also allows for resuming an incomplete download.

This PR does not add support for downloading with the app in the background (see Further Work below).

Which issue is fixed?

Fixes https://github.com/advplyr/audiobookshelf-app/issues/862
Fixes https://github.com/advplyr/audiobookshelf-app/issues/34

This PR is mostly just refactoring without fixing any specific issues. I asked for feedback in the following issues to see if the refactor fixed the issue, but still unsure if those issues are fixed so we should leave them open.

Downloading:

Associating downloaded media with server connection:

Pull Request Type

Android backend

In-depth Description

This PR is a continuation of https://github.com/advplyr/audiobookshelf-app/pull/1469. Previously, there were two nearly identical code paths for Internal App Storage (using File) and Shared Storage (using DocumentFile). We also performed an expensive scan (long running) in Shared Storage once all of the files completed downloading.

As part of this PR, we now:

  • Always download to Internal App Storage, then move to Shared Storage if desired.
    • This allows us to reuse the same download logic for both internal and shared storage (using InternalDownloadManager). This also simplifies continuing incomplete downloads, discussed below.
  • LocalFile class updates:
    • Remove simplePath because it is one of several slow properties (SAF) and is unused anywhere in the codebase.
    • Added extension to improve code reuse.
  • Build the LocalLibraryItem object from the DownloadItem object instead of scanning folder in Shared Storage.
    • This still takes about 50-100ms per file in my testing, but is much faster than scanning the folder and then still building the object through SAF.
  • Say "Moving and processing files" instead of just "Processing"
    • More descriptive about what is happening. When downloading a single large file to Shared Storage, moving the file takes a long time and processing is very quick. When downloading multiple smaller files, moving is very quick (every file is moved as soon as it finishes downloading), but scanning can take a few seconds. Downloading to Internal App Storage does not require moving files and scans many files almost instantly.
  • Do not delete temporary or destination files.
    • This allows continuing a long-running download (large single-file M4B), or multi-file downloads.
    • Every file is still added to the download queue. If the file already exists in Internal Storage, continue downloading from the end of the file. If the file is already complete, then the file completes immediately. If the file already exists in Shared Storage, it was already successfully downloaded and moved and is marked as moved.
    • This also allows users to "redownload" a book after resetting the app or manually creating the file structure without actually downloading the entire book again.
    • This drew inspiration from https://github.com/advplyr/audiobookshelf-app/pull/1230 and https://github.com/advplyr/audiobookshelf-app/pull/1231.
  • Check free space in Internal App Storage before downloading any file
    • The "minimum free space" required is the size of the downloaded file multiplied by the maxSimultaneousDownloads
    • If there is not enough free space, the AbsLogger shows an error and the file is not downloaded. The download queue can continue with other files, so larger files can still be downloaded in series instead of parallel.

Further work

Building the LocalLibraryItem as each individual file is moved to Shared Storage would reduce the processing time to nearly constant (time to move file + 100 ms), but complicates the logic a lot more with incomplete downloads and the queue system.

I would like to move the download manager to a Foreground service to help with downloads being stopped in the background when a user switches to another app. Further testing would be needed before adding a Foreground service. https://github.com/advplyr/audiobookshelf-app/issues/956

How have you tested this?

Tested with Android 15 on a Pixel 6a with both books and podcasts.
This should have additional testing done with other devices, especially older versions of Android.

Screenshots

N/A


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/advplyr/audiobookshelf-app/pull/1587 **Author:** [@nichwall](https://github.com/nichwall) **Created:** 5/26/2025 **Status:** 🔄 Open **Base:** `master` ← **Head:** `android_download_rewrite` --- ### 📝 Commits (10+) - [`feef527`](https://github.com/advplyr/audiobookshelf-app/commit/feef527bede069b06ecb0b39001b3ec1e3d5fe47) Delet unused file - [`ee0ab43`](https://github.com/advplyr/audiobookshelf-app/commit/ee0ab43fc57d5d23bed80f6be1f353402df896ab) Auto formatting `AbsDownloader.kt` - [`4a09238`](https://github.com/advplyr/audiobookshelf-app/commit/4a09238a8e13409e4052b4bfb0c42843b46d333d) Refactor `AbsDownloader` - [`19b1f3b`](https://github.com/advplyr/audiobookshelf-app/commit/19b1f3b6317b80cf70c56f77f76cbc5fe59c29ba) Remove: unused json file from FolderScanner - [`5d8574e`](https://github.com/advplyr/audiobookshelf-app/commit/5d8574eb8712fc73bf292fe426aa2502f1395d5b) Simplify local library item creation in folder scanner - [`f31a8f0`](https://github.com/advplyr/audiobookshelf-app/commit/f31a8f04825a7bc39854e4f1355f63c8411efa85) Revert "Refactor `AbsDownloader`" - [`841527e`](https://github.com/advplyr/audiobookshelf-app/commit/841527eb74ef28d88fe4873b36bbc5b4c40867c7) Simplify `AbsDownloader` download item creation - [`640d8cd`](https://github.com/advplyr/audiobookshelf-app/commit/640d8cdd4103942c2a52cbb6622d0b3537d9f7c5) Combine cover download logic - [`7e2ac27`](https://github.com/advplyr/audiobookshelf-app/commit/7e2ac27eba9f717249318bf88f0690b183c01bad) Fix: cover printout for book - [`e11b8cb`](https://github.com/advplyr/audiobookshelf-app/commit/e11b8cb66ce97c4b047104e7849d864035c81a4e) Initial internal download continue incomplete download ### 📊 Changes **8 files changed** (+766 additions, -844 deletions) <details> <summary>View changed files</summary> 📝 `android/app/src/main/java/com/audiobookshelf/app/data/DeviceClasses.kt` (+133 -113) ➖ `android/app/src/main/java/com/audiobookshelf/app/data/FolderScanResult.kt` (+0 -15) 📝 `android/app/src/main/java/com/audiobookshelf/app/device/FolderScanner.kt` (+189 -415) 📝 `android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt` (+82 -120) 📝 `android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt` (+80 -38) 📝 `android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt` (+69 -50) 📝 `android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt` (+212 -92) 📝 `strings/en-us.json` (+1 -1) </details> ### 📄 Description <!-- For Work In Progress Pull Requests, please use the Draft PR feature, see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for further details. If you do not follow this template, the PR may be closed without review. Please ensure all checks pass. If you are a new contributor, the workflows will need to be manually approved before they run. --> ## Brief summary This PR refactors the Android download logic to use a single code path for both Internal App Storage and Shared Storage. This PR also allows for resuming an incomplete download. **This PR does not add support for downloading with the app in the background (see Further Work below).** ## Which issue is fixed? Fixes https://github.com/advplyr/audiobookshelf-app/issues/862 Fixes https://github.com/advplyr/audiobookshelf-app/issues/34 This PR is mostly just refactoring without fixing any specific issues. I asked for feedback in the following issues to see if the refactor fixed the issue, but still unsure if those issues are fixed so we should leave them open. Downloading: - https://github.com/advplyr/audiobookshelf-app/issues/1479 - https://github.com/advplyr/audiobookshelf-app/issues/1273 - https://github.com/advplyr/audiobookshelf-app/issues/1253 - https://github.com/advplyr/audiobookshelf-app/issues/1167 - https://github.com/advplyr/audiobookshelf-app/issues/215 Associating downloaded media with server connection: - https://github.com/advplyr/audiobookshelf-app/issues/1386 - https://github.com/advplyr/audiobookshelf-app/issues/1362 - https://github.com/advplyr/audiobookshelf-app/issues/594 ## Pull Request Type Android backend ## In-depth Description This PR is a continuation of https://github.com/advplyr/audiobookshelf-app/pull/1469. Previously, there were two nearly identical code paths for Internal App Storage (using `File`) and Shared Storage (using `DocumentFile`). We also performed an expensive scan (long running) in Shared Storage once all of the files completed downloading. As part of this PR, we now: - Always download to Internal App Storage, then move to Shared Storage if desired. - This allows us to reuse the same download logic for both internal and shared storage (using `InternalDownloadManager`). This also simplifies continuing incomplete downloads, discussed below. - `LocalFile` class updates: - Remove `simplePath` because it is one of several slow properties (SAF) and is unused anywhere in the codebase. - Added `extension` to improve code reuse. - Build the `LocalLibraryItem` object from the `DownloadItem` object instead of scanning folder in Shared Storage. - This still takes about 50-100ms per file in my testing, but is much faster than scanning the folder and *then* still building the object through SAF. - Say "Moving and processing files" instead of just "Processing" - More descriptive about what is happening. When downloading a single large file to Shared Storage, moving the file takes a long time and processing is very quick. When downloading multiple smaller files, moving is very quick (every file is moved as soon as it finishes downloading), but scanning can take a few seconds. Downloading to Internal App Storage does not require moving files and scans many files almost instantly. - Do not delete temporary or destination files. - This allows continuing a long-running download (large single-file M4B), or multi-file downloads. - Every file is still added to the download queue. If the file already exists in Internal Storage, continue downloading from the end of the file. If the file is already complete, then the file completes immediately. If the file already exists in Shared Storage, it was already successfully downloaded and moved and is marked as moved. - This also allows users to "redownload" a book after resetting the app or manually creating the file structure without actually downloading the entire book again. - This drew inspiration from https://github.com/advplyr/audiobookshelf-app/pull/1230 and https://github.com/advplyr/audiobookshelf-app/pull/1231. - Check free space in Internal App Storage before downloading any file - The "minimum free space" required is the size of the downloaded file multiplied by the `maxSimultaneousDownloads` - If there is not enough free space, the AbsLogger shows an error and the file is not downloaded. The download queue can continue with other files, so larger files can still be downloaded in series instead of parallel. ### Further work Building the `LocalLibraryItem` as each individual file is moved to Shared Storage would reduce the processing time to nearly constant (time to move file + 100 ms), but complicates the logic a lot more with incomplete downloads and the queue system. I would like to move the download manager to a Foreground service to help with downloads being stopped in the background when a user switches to another app. Further testing would be needed before adding a Foreground service. https://github.com/advplyr/audiobookshelf-app/issues/956 ## How have you tested this? Tested with Android 15 on a Pixel 6a with both books and podcasts. *This should have additional testing done with other devices, especially older versions of Android.* ## Screenshots N/A --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2026-04-24 23:59:48 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/audiobookshelf-app#1627