Skip to content
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

MP4 download HttpDataSource #11120

Closed
yoobi opened this issue Apr 14, 2023 · 12 comments
Closed

MP4 download HttpDataSource #11120

yoobi opened this issue Apr 14, 2023 · 12 comments
Assignees

Comments

@yoobi
Copy link
Contributor

yoobi commented Apr 14, 2023

Bug

I've been trying to download MP4 file via OkHttpDataSource but it downloads only the chunks that have been played by the player. The download process correctly with other DataSource such as DefaultHttpDataSource and CronetDataSource

I have a repository which shows the issue: https://github.com/yoobi/exoplayer-kotlin/tree/issue/okhttp_download_mp4/downloadvideo

When using OkHttpDataSource to download HLS video there is no issue, but when it comes to MP4 you need to play the video to progress in the downloads.

@yoobi
Copy link
Contributor Author

yoobi commented Apr 14, 2023

After some more testing I found that Exoplayer.mediaSource and DownloadManager was using the same OkHttpDataSource which seems to be the problem in downloading MP4. However I don't understand why this problem arose only for MP4 and not HLS video.

Is it best practice to use different DataSource for ExoPlayer and DownloadManager ?

@yoobi yoobi changed the title MP4 download via OkHttpDataSource MP4 download HttpDataSource Apr 14, 2023
@icbaker icbaker self-assigned this Apr 14, 2023
@icbaker
Copy link
Collaborator

icbaker commented Apr 14, 2023

A DataSource is stateful, so it can't be used to read two different resources at the same time (since it is opened to read a specific URI (or DataSpec)). Generally it's safest to pass around DataSource.Factory instances and let the code that needs to do the reading take care of creating, opening, reading and closing each DataSource. DataSource instances are reusable, i.e. after they've been closed they can be re-opened with another URI.

@icbaker icbaker added question and removed bug labels Apr 14, 2023
@yoobi
Copy link
Contributor Author

yoobi commented Apr 14, 2023

I'm passing HttpDataSource.Factoryeverywhere, could it be using the same instance in the across the DownloadManager and ExoPlayer ?

@Synchronized
fun getHttpDataSourceFactory(): HttpDataSource.Factory {
    if(!DownloadUtil::httpDataSourceFactory.isInitialized) {
        httpDataSourceFactory = OkHttpDataSource.Factory(OkHttpClient.Builder().build())
    }
    return httpDataSourceFactory
}

@Synchronized
private fun ensureDownloadManagerInitialized(context: Context) {
    if(!DownloadUtil::downloadManager.isInitialized) {
        downloadManager = DownloadManager(
            context,
            getDatabaseProvider(context),
            getDownloadCache(context),
            getHttpDataSourceFactory(),
            Executors.newFixedThreadPool(6)
        ).apply {
            maxParallelDownloads = 2
        }
        downloadTracker =
            DownloadTracker(context, getHttpDataSourceFactory(), downloadManager)
    }
}

I have modified my ExoPlayer creation by

ExoPlayer.Builder(this)
    .setMediaSourceFactory(ProgressiveMediaSource.Factory(DownloadUtil.getHttpDataSourceFactory()))
    .build()
    .apply {
        setMediaItem(mediaItem)
        prepare()
    }

The download is still stuck when reading at the same time. Also note that this issue doesn't appear when using DefaultHttpDataSource instead of OkHttpDataSource

@icbaker
Copy link
Collaborator

icbaker commented Apr 14, 2023

I'm confused by your updates. You said:

After some more testing I found that Exoplayer.mediaSource and DownloadManager was using the same OkHttpDataSource

But then you said:

I'm passing HttpDataSource.Factory everywhere

How can you be seeing the same DataSource instance in both your MediaSourceFactory and DownloadManager if both are only provided a DataSource.Factory instance which creates a new DataSource each time createDataSource is called?

@yoobi
Copy link
Contributor Author

yoobi commented Apr 15, 2023

You are totally right, I'm using *.Factory, MediaSourceFactory and DownloadManager are using different DataSource.
You can look at my last commit https://github.com/yoobi/exoplayer-kotlin/tree/issue/okhttp_download_mp4

Could the download be stuck because of FLAG_BLOCK_ON_CACHE from

public CacheDataSource createDataSourceForDownloading() {
      return createDataSourceInternal(
          upstreamDataSourceFactory != null ? upstreamDataSourceFactory.createDataSource() : null,
          flags | FLAG_BLOCK_ON_CACHE,
          C.PRIORITY_DOWNLOAD);
    }

@icbaker
Copy link
Collaborator

icbaker commented Apr 17, 2023

I'm afraid we don't have capacity to provide 1:1 support for individual apps - if you believe there's a bug in the OkHttpDataSource please try reproducing it in the demo app (which supports downloading), and update this issue with instructions we can use to reproduce there. Without that it seems likely that the bug is in your own app.

@yoobi
Copy link
Contributor Author

yoobi commented Apr 17, 2023

It is reproducible with the demo app, I've just added a download button inside the PlayerActivity.
https://github.com/yoobi/ExoPlayer/tree/issue/11120

Step to reproduce:

  1. Go to Misc
  2. Click on Big Bunny (MP4 file) (in order to go to PlayerActivity)
  3. Press Download in the top left corner

You'll see the download is stuck at the beginning while you're on the PlayerActivity. Once you leave the PlayerActivity, the download is "resumed" and progress smoothly

@icbaker
Copy link
Collaborator

icbaker commented Apr 17, 2023

Thanks, I'm able to reproduce what you're seeing. My suspicion is that this is due to OkHttp's connection pooling/re-use. Since the same OkHttp.Client is re-used between multiple DataSource instances (as explicitly recommended in the OkHttp docs). It looks like connection pooling and re-use results in the download re-using the connection that is being used for playback (and is blocked (i.e. isn't being read from) because playback is paused before the whole video is buffered).

I spotted this bit of the OkHttp docs that suggests this might only happen for HTTP/2 connections (emphasis mine):

When an HTTP request is started, OkHttp will attempt to reuse an existing connection from the pool. If there are no existing connections, a new one is created and put into the connection pool. For HTTP/2, the connection can be reused immediately. For HTTP/1, the request must be completed before it can be reused.

Sure enough, I can make the download proceed while playback is paused by forcing OkHttp to use HTTP 1.1:

OkHttpClient okHttpClient =
    new OkHttpClient.Builder()
        .protocols(ImmutableList.of(Protocol.HTTP_1_1))
        .build();
httpDataSourceFactory = new OkHttpDataSource.Factory((Call.Factory) okHttpClient);

I'm not sure what a better solution here is. It seems reasonable to request a resource twice from the same server (or even at the same URI) and leave one request open/pending/consumed while expecting the other to make progress - and I would expect OkHttp to support this, but it doesn't seem to. If you want this to work with OkHttp and HTTP/2 I suggest you ask on https://github.com/square/okhttp/issues (you can reference this issue as context of course).

@yoobi
Copy link
Contributor Author

yoobi commented Apr 17, 2023

Thank you for your in-depth explanation, I was starting to guess the issue lied in OkHttpClient.

I'll create an issue on okhttp side and hopes this can be fixed

@yoobi
Copy link
Contributor Author

yoobi commented Apr 18, 2023

Looks like an issue already exists in media3

@icbaker
Copy link
Collaborator

icbaker commented Apr 18, 2023

Closing because I think square/okhttp#7768 and androidx/media#188 adequately track this now.

@icbaker icbaker closed this as completed Apr 18, 2023
@yschimke
Copy link
Contributor

Fix here square/okhttp#7801

If anyone can test out the snapshot, instructions here square/okhttp#7768 (comment) , it well help give us confidence on the OkHttp fix. Likely in a 5 alpha, and then 4.12 a bit later.

@icbaker I think androidx/media#188 is unrelated, while addressing that would also fix the symptoms of this issue, you should be able to keep streams open and complete downloads at the same time.

@google google locked and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants