DefaultMediaSourceFactory ignores custom Cronet data source

[davinci26]

Hey folks,

Core Problem

I am trying to enabled QUIC for ExoPlayer and I am running into issues that I believe are specific (in a way that is not clear to me) to ExoPlayer.

For start I have following piece of code:

        CronetEngine.Builder myBuilder = new CronetEngine.Builder(getApplicationContext());
        CronetEngine cronetEngine =
                myBuilder
                        .enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISABLED, 100 * 1024)
                        .addQuicHint("<my_url>", 443, 443)
                        .enableQuic(true)
                        .build();

        Executor executor = Executors.newSingleThreadExecutor();
        UrlRequest.Builder requestBuilder = cronetEngine.newUrlRequestBuilder(
                "<my_url>", new MyUrlRequestCallback(), executor);
        UrlRequest request = requestBuilder.build();
        request.start();

That connection is going over the wire with HTTP3 (validated both via MyUrlRequestCallback and a packetdump)

Now when I use the same cronetEngine on ExoPlayer it never goes through HTTP3.

In the demo APP I change the following (full diff bellow):

        CronetEngine.Builder cronetBuilder = new CronetEngine.Builder(context);
        @Nullable CronetEngine cronetEngine =
            cronetBuilder
                .enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISABLED, 100 * 1024)
                .addQuicHint("<my_url>", 443, 443)
                .enableQuic(true)
                .build();

But the connection goes over TCP when I try it with wireshark.

Debugging Attempts

I have attempted to debug this by following #5134

In the comments @tonihei mentions

You need to overwrite CronetDataSource and read the protocol after opening the source:

public class MyCronetSource extends CronetDataSource {
  public MyCronetSource(CronetEngine cronetEngine) {
    super(cronetEngine, Executors.newSingleThreadExecutor(), null);
  }

  @Override
  public long open(DataSpec dataSpec) throws IOException {
    long result = super.open(dataSpec);
    // Use getCurrentUrlResponseInfo() here and in any other method of the data source.
    // This method is call ed on the background loading thread.
    return result;
  }
}
And to set up the player just create your data source in a factory:

DataSource.Factory factory = () -> new MyCronetDataSource(cronetEngine);

I have tried that approach by implementing:

class MyCronetSource extends CronetDataSource {
  public MyCronetSource(CronetEngine cronetEngine) {
    super(cronetEngine,
        Executors.newSingleThreadExecutor(),
        REQUEST_PRIORITY_MEDIUM,
        DEFAULT_CONNECT_TIMEOUT_MILLIS,
        DEFAULT_READ_TIMEOUT_MILLIS,
        false,
        false,
        "User agent foo",
        null,
        null,
        true
        );
    Log.i("App", "MyCronetSource constructed");
  }

  @Override
  public long open(DataSpec dataSpec) throws HttpDataSourceException {
    Log.i("App", "open called");
    long result = super.open(dataSpec);
    Log.i("App", getCurrentUrlResponseInfo().getNegotiatedProtocol());
    // Use getCurrentUrlResponseInfo() here and in any other method of the data source.
    // This method is call ed on the background loading thread.
    return result;
  }

And then setting it via httpDataSourceFactory = () -> new MyCronetSource(cronetEngine);

But the open method is never called. Which makes me think that something has changed over the past 4 years and this approach is maybe no longer relevant. Any guidance on that?

I am working on top of commit 03569f9e6536e6c60cb585115d6d7352a4deab71 (HEAD -> release-v2, tag: r2.18.0, origin/release-v2, origin/HEAD)

cc @jamesonwilliams

Git Diff Patch

diff --git a/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoUtil.java b/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoUtil.java
index 4343f74830..4aca23cc2c 100644
--- a/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoUtil.java
+++ b/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoUtil.java
@@ -15,7 +15,10 @@
  */
 package com.google.android.exoplayer2.demo;
 
+import static org.chromium.net.UrlRequest.Builder.REQUEST_PRIORITY_MEDIUM;
+
 import android.content.Context;
+import android.util.Log;
 import com.google.android.exoplayer2.DefaultRenderersFactory;
 import com.google.android.exoplayer2.RenderersFactory;
 import com.google.android.exoplayer2.database.DatabaseProvider;
@@ -25,6 +28,7 @@ import com.google.android.exoplayer2.ext.cronet.CronetUtil;
 import com.google.android.exoplayer2.offline.DownloadManager;
 import com.google.android.exoplayer2.ui.DownloadNotificationHelper;
 import com.google.android.exoplayer2.upstream.DataSource;
+import com.google.android.exoplayer2.upstream.DataSpec;
 import com.google.android.exoplayer2.upstream.DefaultDataSource;
 import com.google.android.exoplayer2.upstream.DefaultHttpDataSource;
 import com.google.android.exoplayer2.upstream.cache.Cache;
@@ -32,6 +36,7 @@ import com.google.android.exoplayer2.upstream.cache.CacheDataSource;
 import com.google.android.exoplayer2.upstream.cache.NoOpCacheEvictor;
 import com.google.android.exoplayer2.upstream.cache.SimpleCache;
 import java.io.File;
+import java.io.IOException;
 import java.net.CookieHandler;
 import java.net.CookieManager;
 import java.net.CookiePolicy;
@@ -40,6 +45,49 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.chromium.net.CronetEngine;
 
+
+class MyCronetSource extends CronetDataSource {
+  public MyCronetSource(CronetEngine cronetEngine) {
+    super(cronetEngine,
+        Executors.newSingleThreadExecutor(),
+        REQUEST_PRIORITY_MEDIUM,
+        DEFAULT_CONNECT_TIMEOUT_MILLIS,
+        DEFAULT_READ_TIMEOUT_MILLIS,
+        false,
+        false,
+        "User agent foo",
+        null,
+        null,
+        true
+        );
+    Log.i("App", "MyCronetSource constructed");
+  }
+
+  @Override
+  public long open(DataSpec dataSpec) throws HttpDataSourceException {
+    Log.i("App", "open called");
+    long result = super.open(dataSpec);
+    Log.i("App", getCurrentUrlResponseInfo().getNegotiatedProtocol());
+    // Use getCurrentUrlResponseInfo() here and in any other method of the data source.
+    // This method is call ed on the background loading thread.
+    return result;
+  }
+
+  @Override
+  public int read(byte[] buffer, int offset, int readLength) throws HttpDataSourceException {
+    Log.i("App", "read called");
+    int result = super.read(buffer, offset, readLength);
+    return result;
+  }
+
+
+  @Override
+  public void close() {
+    Log.i("App", "close called");
+    super.close();
+  }
+}
+
 /** Utility methods for the demo app. */
 public final class DemoUtil {
 
@@ -87,14 +135,27 @@ public final class DemoUtil {
   public static synchronized DataSource.Factory getHttpDataSourceFactory(Context context) {
     if (httpDataSourceFactory == null) {
       if (USE_CRONET_FOR_NETWORKING) {
+        Log.i("App", "USE_CRONET_FOR_NETWORKING set to true");
         context = context.getApplicationContext();
-        @Nullable CronetEngine cronetEngine = CronetUtil.buildCronetEngine(context);
+        CronetEngine.Builder cronetBuilder = new CronetEngine.Builder(context);
+        @Nullable CronetEngine cronetEngine =
+            cronetBuilder
+                .enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISABLED, 100 * 1024)
+                .enableQuic(true)
+                .build();
         if (cronetEngine != null) {
-          httpDataSourceFactory =
-              new CronetDataSource.Factory(cronetEngine, Executors.newSingleThreadExecutor());
+          Log.i("App", "cronetEngine is used");
+          httpDataSourceFactory = () -> new MyCronetSource(cronetEngine);
         }
       }
       if (httpDataSourceFactory == null) {
+        Log.e("App", "Engine could not be initialized");
+
+        try {
+          throw new Exception("Exception message");
+        } catch (Exception e) {
+          e.printStackTrace();
+        }
         // We don't want to use Cronet, or we failed to instantiate a CronetEngine.
         CookieManager cookieManager = new CookieManager();
         cookieManager.setCookiePolicy(CookiePolicy.ACCEPT_ORIGINAL_SERVER);

[marcbaechinger]

This is possibly a duplicate of androidx/media#116 which is a bug in the DefaultMediaSourceFactory.

The demo app does

new DefaultMediaSourceFactory(/* context= */ this)
        .setDataSourceFactory(dataSourceFactory);

which internally creates a DefaultHttpDataSource in the constructor and the dataSourceFactory passed to the setter is ignored. This would in a very trivial way explain why you are seeing HTTP.

The quick fix would be to do

new DefaultMediaSourceFactory(dataSourceFactory)

We have fixed this already and this will be shipped with the next release that is soon, like tomorrow or start of next week.

Can you confirm that this is removing this behaviour for you? I that's the case, sorry for the confusion and inconvenience and thanks for the excellent bug report.

[davinci26]

Thank you so much! Verified that it works correctly now.