google / ExoPlayer

This project is deprecated and stale. The latest ExoPlayer code is available in https://github.com/androidx/media
https://developer.android.com/media/media3/exoplayer
Apache License 2.0
21.74k stars 6.03k forks source link

Downloads don't take into account the stream keys #9233

Closed romainpiel closed 3 years ago

romainpiel commented 3 years ago

ExoPlayer version 2.11.8

[REQUIRED] Searched documentation and issues Very similar to

6911 and #7682

[REQUIRED] Question When I download a HLS media source using the DownloadHelper, all qualities get downloaded.

I initialise the DownloadHelper this way:

            val helper = DownloadHelper.forHls(
                uri,
                createHttpDataSourceFactory(),
                DefaultRenderersFactory(context),
                null,
                DefaultTrackSelector.Parameters.getDefaults(context)
                    .buildUpon()
                    .setForceHighestSupportedBitrate(true)
                    .build()
            )

Then I start the download with this code:

            helper.prepare(object : DownloadHelper.Callback {
                override fun onPrepared(helper: DownloadHelper) {
                    DownloadService.sendAddDownload(
                        context,
                        VideoDownloadService::class.java,
                        helper.getDownloadRequest(ByteArray(CacheUtil.DEFAULT_BUFFER_SIZE_BYTES)),
                        false
                    )
                }

                override fun onPrepareError(helper: DownloadHelper, e: IOException) {
                }
            })

I have checked the stream keys created in getDownloadRequest() and there's always one stream key with

Any idea what could cause this?

romainpiel commented 3 years ago

I've updated to 2.14.2 and it now works like a charm 🎉

romainpiel commented 3 years ago

Actually I could reproduce it again with 2.14.2. I'm a bit unsure why this is happening but I've done some debugging in my app and in the demo app and I observed this:

So there's definitely an issue with my code but I'm curious why I would observe different behaviours when the request is completely identical 🤔

romainpiel commented 3 years ago

Finally got to the bottom of this. I only had to do a fresh install of the app to fix the issue. Basically at some point the database stored the test stream download with an empty streamKeys. And as the mergeRequest() function in DownloadManager ignores the request stream keys when the database record has an empty stream keys, the resulting stream keys array was always empty. It's quite a niche problem but out of curiosity, why do we have that if and why don't we always take the latest request stream keys?

ojw28 commented 3 years ago

I think this is working as intended. If there's already a download with a given ID, then adding a download with the same ID is additive. In other words, if the existing download has stream key A and the new request has stream key B, then the end result will be that stream keys A and B will be downloaded. It's also true that empty stream keys means "download everything". In your case, since the existing download is already downloading everything, adding a request to download a subset of stream keys doesn't add anything by this logic.

Whether an "additive" design (i.e., the current one) is better than a "replacement" design (i.e., the one you suggest) is debatable. The "replacement" approach is more flexible, in that it allows removing stream keys as part of re-adding a download. The current design only allows removal of the whole download (at which point you can re-download a new set of stream keys). That said, removing/replacing part of a download feels like an edge case, and it's likely (I didn't confirm) more complicated to implement.

Closing as working as intended.