androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
Apache License 2.0
1.36k stars 322 forks source link

Why does DownloadManager.mergeRequest ignore DownloadProgress? #1382

Open stoyicker opened 1 month ago

stoyicker commented 1 month ago

DownloadManager.mergeRequest is used in certain cases when adding a Download to DownloadManager. In my particular scenario, I re-add ongoing downloads from the index to DownloadManager when my app starts. The problem with this is that this ends up in the branch of DownloadManager.addDownload that calls DownloadManager.mergeRequest. This returns a Download constructed using the Download constructor with the overload that does not have a DownloadRequest, meaning that the returned Download representation has no recorded progress until a progress update happens - but since these ignore downloads in states other than downloading (see DownloadManager.updateProgress), I have no way of knowing if a download has progress until I resume it.

I am not sure if this is a bug or just something I am misunderstanding.

Tested with pre-androidx 2.18.7 but from looking at DownloadManager.updateProgress in current tip of main (dc4f20e) I believe it's a situation probably present in there too.

marcbaechinger commented 1 month ago

Thanks for your question!

I think not copying the progress information is on purpose. If a user is adding a DownloadRequest with an existing ID again, then it is merged and in case of an adaptive stream, the new request may have different streamKeys which means further data has to be downloaded from the beginning. When in such a case a Download is attempted to be downloaded again, the process just starts again from the beginning and tries to find the cached data. If it's already in the cache, then it advances until it finds a cache miss that needs downloading. See [1] for where the stream keys are merged.

Accordingly, the progress is reset and when attempting to download again, the download would download additional tracks for an adaptive stream which starts from the beginning.

It looks like, given the stream keys are kept the same, the progress could be kept as well, assuming the media up to the progress will be found in the cache. The code isn't that clever to check this though.

I'm also not sure how save this is but I'd think that at least for Download instances in a certain state (for instance excluding STATE_DOWNLOADING and others), it would be save to keep the progress.

I'm marking this issue as an enhancement. I'm not sure when we get around looking into this. So I can't give you an idea on when that would be I'm afraid.

May I ask what the intention is of re-adding these downloads request?

[1] https://github.com/androidx/media/blob/release/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/DownloadRequest.java#L220-L231

stoyicker commented 1 month ago

Thanks, that clarifies things.

May I ask what the intention is of re-adding these downloads request?

I'd say that can be considered a bug on my end.