-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow DownloadManager do automatic track selection #5915
Comments
I can't reproduce this on the demo app. One thing to keep in mind is DownloadService and DownloadManager methods are asynchronous. It takes some time for the internal states to change. |
I'm sorry about my assumption that this was easily reproducible! I'll try to reproduce it on the demo app as well. I was aware about them being asynchronous but even after a while I was never able to see the items queued. |
@erdemguven I just spent some time with the demo app and I was able to reproduce the issue. It's a little tricky there for HLS because we show the track selection dialog before actually starting the download, but the issue is still there: if you start a download while offline, no queued download will show up in the To make things simpler, we can modify the @Override
public void onPrepared(DownloadHelper helper) {
if (helper.getPeriodCount() == 0) {
Log.d(TAG, "No periods found. Downloading entire stream.");
startDownload();
downloadHelper.release();
return;
}
mappedTrackInfo = downloadHelper.getMappedTrackInfo(/* periodIndex= */ 0);
// if (!TrackSelectionDialog.willHaveContent(mappedTrackInfo)) {
Log.d(TAG, "No dialog content. Downloading entire stream.");
startDownload();
downloadHelper.release();
return;
// }
// trackSelectionDialog =
// TrackSelectionDialog.createForMappedTrackInfoAndParameters(
// /* titleId= */ R.string.exo_download_description,
// mappedTrackInfo,
// /* initialParameters= */ DownloadHelper.DEFAULT_TRACK_SELECTOR_PARAMETERS,
// /* allowAdaptiveSelections =*/ false,
// /* allowMultipleOverrides= */ true,
// /* onClickListener= */ this,
// /* onDismissListener= */ this);
// trackSelectionDialog.show(fragmentManager, /* tag= */ null);
} With those parts commented out we can see the issue more easily. Now if you start a download while offline, nothing will happen again and no downloads will ever be queued no matter how much you wait for it. And then once you go back online, the download will start automatically. I get the same issue with DASH, so that's probably not specific to HLS and is related to adaptive streams in general. |
I just realized one of the things in my original description is wrong. I said that I'm starting the download by calling And I just realized that when I'm offline, I hope things are more clear now, but if not just let me know how else I can help. |
Yes, I can see that behaviour with the demo app. If the device is offline a failure message is displayed. but when it becomes online the track selection dialog comes up. @tonihei can you look at this. |
So just to clarify: The download tracking is working as intended because the failed DownloadHelper meant that no download was ever actually started. And the sudden appearance of the download is due to the unexpected DownloadHelper.onPrepared callback. The reason for the unexpected callback is that no one releases the DownloadHelper after onPrepareError (neither our demo app, nor your app apparently) and that's why we may get further manifest updates in the future which are successful (after coming back online). The fix is to trigger the release automatically once the preparation failed to prevent any further callbacks. |
This doesn't seem to be the proper behavior, though. To me the download should be queued and not started because a requirement is missing (network). This doesn't seem to be consistent with how everything else works.
|
That suggestion is to integrate DownloadHelper into the DownloadManager state tracking? Good idea, especially given the requirements tracking like waiting for network. Not sure how the state transitions look like in such a case. It seems we would need states like PREPARING and then a notMetRequirement along the lines of "waiting for DownloadHelper configuration". @erdemguven What do you think about this proposal? We can use this issue to track as an enhancement. |
I initially saw this as a bug, but I guess I wasn't seeing My final goal as a consumer of the library would be to be able to support a queue out of the box with ExoPlayer, so whenever users try to start downloads while offline, everything is properly queued (and I can query whatever is queued from |
Yes, it's a good idea to use this issue to track this feature request. Btw, what I see in Netflix and other apps, is they just show an error when user tries to download anything without connection so they don't queue those requests. |
It behaves the way it does because it's not possible to do track selection without network connectivity, so the alternative approach doesn't work for apps designed like our demo app where user is given the option to select tracks when starting a download. Ditto for any other use cases that require showing the user something about the content to be downloaded, such as its size vs available space on the SD card. +1 to using this to track an enhancement where we allow pushing the download straight into the manager for cases where user interaction isn't required. In the meantime, it shouldn't be difficult for an app with this kind of use case to track downloads that it hasn't managed to initialize by itself. Presumably it's just a list of URLs or content IDs. So I wouldn't consider this particularly high priority. |
Yup! We at Blinkist have always queued downloads, though, so whenever connection is back we start the downloads automatically -- it's definitely a nicer experience. When I first came across ExoPlayer 2.10 new download capabilities I really thought we'd be able to get that behavior by default with it which would be great, and that's my main motivation to try to push for this here.
It's definitely an easy thing to do, as it is to track downloads in general. But 2.10 made things even easier for consumers when it comes to tracking and this inconsistency just seems to be an important missing piece in the whole picture. If I try to download something while connected to a metered connection and I have |
Btw, what kind of media are you downloading? If it's a progressive stream (mp4, mp3) or an adaptive stream with predefined tracks, you can skip DownloadHelper and create DownloadRequest yourself. Which would be faster and DownloadManager would queue it for you. |
It's an adaptive stream but it's not really predefined tracks. We're using |
This prevents further unexpected updates if the MediaSource happens to finish its preparation at a later point. Issue:#5915 PiperOrigin-RevId: 249439246
Original bug (onPrepared after onPrepareError) is now fixed. You can workaround it until the next release but calling downloadHelper.release() from onPrepareError.
That definitely sounds like a use case for DownloadHelper because the track selection will take into account device capabilities etc., which is something you can't do with a manual pre-selection. However, that's also the default parameter set we are using in DownloadHelper (so no need to specify that again) and thus it should be possible to do that automatically once the network is available. |
Thanks for fixing that!
You mean doing automatically that on my end or on ExoPlayer's end? |
On our end, that's the enhancement tracked by this issue. So that you can add the download to the DownloadManager (or Service) and the automatic track selection using these parameters takes place once the network becomes available. |
This prevents further unexpected updates if the MediaSource happens to finish its preparation at a later point. Issue:#5915 PiperOrigin-RevId: 249439246
@tonihei We're facing an interesting (and different) situation where this issue became relevant again, so I thought I'd share it here. We allow users to start a large number of downloads at once in a batch, but we have a good It'd be nice if we could add the downloads to the |
Am I right in assuming that letting Until that is implemented, there is probably not much we can do about this because it's your code calling the |
Yes, I believe you're right. So I think once this issue is resolved/implemented, we'd have that solved.
Yup, make sense. And I don't see any obvious improvement we could do on |
Issue description
Whenever I start a download (
by callingsee this comment) while offline, I don't see the download queued when I check the downloads in theDownloadService.sendAddDownload()
DownloadManager
(either throughcurrentDownloads
ordownloadIndex
). CallingisWaitingForRequirements()
returns false, but as soon as the connection is back, the download starts automatically as if it was queued and waiting for requirements.I did some debugging and was able to see within
DownloadManager
thatnotMetRequirements
was correctly1
but thedownloads
list was empty -- it seems that's where the issue is.I don't think it's relevant, but just in case: I'm downloading HLS content and this is how my helper creation looks like:
Reproduction steps
I can try to reproduce this with the demo app or try to create another demo for this issue if it'd really be helpful, but I think the issue is pretty straightforward to reproduce and observe:
notMetRequirements
is1
, even thoughisWaitingForRequirements()
is false.Link to test content
I believe this isn't relevant.
A full bug report captured from the device
Hopefully also not relevant.
Version of ExoPlayer being used
2.10.0 and 2.10.1
Device(s) and version(s) of Android being used
Samsung Note 8 and Pixel 2 emulator on API 26. It's reproducible every time.
The text was updated successfully, but these errors were encountered: