Support moving download location (e.g., between internal and external storage)

[tfcporciuncula]

[REQUIRED] Use case description

I'm trying to implement a way for users to switch from internal storage to external storage (e.g. SD card), and I want to preserve all downloads when doing that. I'm releasing the current cache, copying all the files, and creating a new cache pointing to the new directory (I'm using SimpleCache). I'm also creating a new DownloadManager with the new cache, but in the end that doesn't matter because the DownloadService will still be stuck to the old DownloadManager (pointing to the old cache) thanks to the static downloadManagerListeners that hold the DownloadManagerHelper which holds the DownloadManager.

Proposed solution

Ideally it'd be great if we had an API that would do all the work needed to switch between internal storage to external storage. But if we have a way to simply update the DownloadManager instance held by DownloadService, we'd be able to do the rest ourselves.

Alternatives considered

At first I thought we could simply have a protected method that clears the downloadManagerListeners, but I don't fully understand its purpose yet, so I'm not sure whether that's reasonable or not.

On our end one solution is to simply kill the process and restart the app, but that's definitely overkill and would be far from good to the users.

[AquilesCanta]

@erdemguven Could you please take a look?

[tfcporciuncula]

Another alternative is to use reflection to clear the downloadManagerListeners whenever I need to reinitialize things. I've tested it and it works pretty well, so I guess a quick fix for this would be to just expose a method to clear that.

[luiscurini]

I'm experiencing the same issue, I had to use reflection as suggested by @tfcporciuncula in order to reinitialize it on storage change.

[szaboa]

We would like to have the same functionality (be able to toggle internal storage and external storage). Any information about the status? Is this something planned?

[tfcporciuncula]

Here's my current reflection code for future reference, since it seems this might be useful for other people:

public static void cleanDownloadManagerReference() throws NoSuchFieldException, IllegalAccessException {
  Field field = DownloadService.class.getDeclaredField("downloadManagerListeners");
  field.setAccessible(true);
  ((Map) field.get(null)).clear();
}

[tfcporciuncula]

Turns out there's a big downside that I missed: if, for some reason, we run that reflection code while the service is running, we'll get a crash when the service is being destroyed:

@Override
public void onDestroy() {
  isDestroyed = true;
  // downloadManagerHelper will be null
  DownloadManagerHelper downloadManagerHelper = downloadManagerListeners.get(getClass());
  boolean unschedule = !downloadManager.isWaitingForRequirements();
  // so this leads to a NullPointerException
  downloadManagerHelper.detachService(this, unschedule);
  if (foregroundNotificationUpdater != null) {
    foregroundNotificationUpdater.stopPeriodicUpdates();
  }
}

This is just the onDestroy() implementation of the DownloadService. We'll get a null from that downloadManagerListeners.get(getClass()) call, and we don't do any null checks, so we get a NPE.

It'd be great if we could add a null check there so our hack could work, but I understand how that request is weird, given that on the long run we don't want to depend on the hack. Still, might be a good quick win for now cc @tonihei @ojw28

[janeriksamsonsen]

Ability to switch between internal-storage and sd-card would be a very welcome enhancement!

[anonrootkit]

Any update on the same? @janeriksamsonsen @tfcporciuncula @ojw28 @marcbaechinger @AquilesCanta @luiscurini

[karamgharam]

any news about this ? @marcbaechinger

[mickael-qobuz]

Hi, do you plan to fix that issue? Would be great. Thank you.

[Merlinkoss]

Any updates here?

[anonrootkit]

@Merlinkoss In my case, I created two different download managers for internal and external storage and it worked perfectly fine.

[Merlinkoss]

@Merlinkoss In my case, I created two different download managers for internal and external storage and it worked perfectly fine.

really? topicstarter says that did not worked, Because DownloadService cached DownloadManager

[billyjoker]

@tfcporciuncula your "hack fix" seems to work as expected, do you know any additional side effects ?

[tfcporciuncula]

Nope, nothing other than what I described here. It's been a long time I have worked with this or on the original project I introduced this, though, so I'm out of the loop if anything changed there.

[billyjoker]

UPDATE: the name field downloadManagerListeners seems it has been replaced by a new one named downloadManagerHelpers. So if you do not update it, maybe this hack stopped working well.

[pavelperc]

downloadManagerHelpers is a HashMap with the class name as a key: https://github.com/google/ExoPlayer/blob/7fe9ecc1c592b758e76f6b458e0dc43cce282e5f/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadService.java#L176

So I implemented two DownloadService classes - for sd card and for internal storage. Works perfectly.

[yoobi]

@pavelperc do you have a small poc of this ?

[brozikcz]

I have a PoC with a custom DownloaderFactory and SimpleCache:

class DownloaderFactory(private val simpleCache: CustomSimpleCache, private val upstreamFactory: DataSource.Factory, private val executor: Executor) : DownloaderFactory {

    override fun createDownloader(request: DownloadRequest): Downloader {
        val mediaItem = MediaItem.Builder()
            .setUri(request.uri)
            .setStreamKeys(request.streamKeys)
            .setCustomCacheKey(request.customCacheKey) // TODO custom key!?
            .build()

        val data = DownloadData.fromByteArray(request.data) // get external path from custom data in request
        val cache = simpleCache.get(data?.path) 

        return DashDownloader(
            mediaItem,
            CacheDataSource.Factory()
                .setCache(cache)
                .setUpstreamDataSourceFactory(upstreamFactory),
            executor
        )
    }
}

@Singleton
class CustomSimpleCache @Inject internal constructor(
    private val context: Context,
    private val exoDatabaseProvider: StandaloneDatabaseProvider,
    private val contentDirFactory: ContentDirFactory
) {

    private val cache = mutableMapOf<String, SimpleCache>()
    private val noOpCacheEvictor by lazy { NoOpCacheEvictor() }
    private val fallbackDir by lazy { context.getExternalFilesDir(null) }

    @Synchronized
    fun get(path: String?): SimpleCache {
        val safePath = path ?: fallbackDir!!.path

        val exoDir = contentDirFactory.getExoDir(safePath) // TODO exception unmounted

        var simpleCache = cache[safePath]
        if (simpleCache != null) {
            return simpleCache
        }

        simpleCache = SimpleCache(
            exoDir,
            noOpCacheEvictor,
            exoDatabaseProvider
        )

        cache[safePath] = simpleCache
        return simpleCache
    }
}

The CustomSimpleCache is used in the player as well.

[pavelperc]

@yoobi Made a sample for you: https://github.com/pavelperc/ExoPlayerSdCardSample

[yoobi]

Thank you ! appreciate a lot