Use Executor instead of ExecutorService for parallel downloads

- Executor is a superclass of ExecutorService, so this is arguably a little
  more flexible.
- It removes the need to use null for direct execution, because Runnable::run
  is a direct executor that can be trivially used instead.
- Removing the error-prone "cannot be a direct executor" restriction in the
  parallel version of SegmentDownloader requires not relying on the futures
  returned from ExecutorService.submit() anyway.

Issue: #5978
PiperOrigin-RevId: 308586620
This commit is contained in:
olly 2020-04-27 10:58:58 +01:00 committed by Ian Baker
parent 008d38d832
commit 2e9ed51503
7 changed files with 43 additions and 62 deletions

View File

@ -51,15 +51,15 @@
# Constructors accessed via reflection in DefaultDownloaderFactory # Constructors accessed via reflection in DefaultDownloaderFactory
-dontnote com.google.android.exoplayer2.source.dash.offline.DashDownloader -dontnote com.google.android.exoplayer2.source.dash.offline.DashDownloader
-keepclassmembers class com.google.android.exoplayer2.source.dash.offline.DashDownloader { -keepclassmembers class com.google.android.exoplayer2.source.dash.offline.DashDownloader {
<init>(android.net.Uri, java.util.List, com.google.android.exoplayer2.upstream.cache.CacheDataSource.Factory, java.util.concurrent.ExecutorService); <init>(android.net.Uri, java.util.List, com.google.android.exoplayer2.upstream.cache.CacheDataSource.Factory, java.util.concurrent.Executor);
} }
-dontnote com.google.android.exoplayer2.source.hls.offline.HlsDownloader -dontnote com.google.android.exoplayer2.source.hls.offline.HlsDownloader
-keepclassmembers class com.google.android.exoplayer2.source.hls.offline.HlsDownloader { -keepclassmembers class com.google.android.exoplayer2.source.hls.offline.HlsDownloader {
<init>(android.net.Uri, java.util.List, com.google.android.exoplayer2.upstream.cache.CacheDataSource.Factory, java.util.concurrent.ExecutorService); <init>(android.net.Uri, java.util.List, com.google.android.exoplayer2.upstream.cache.CacheDataSource.Factory, java.util.concurrent.Executor);
} }
-dontnote com.google.android.exoplayer2.source.smoothstreaming.offline.SsDownloader -dontnote com.google.android.exoplayer2.source.smoothstreaming.offline.SsDownloader
-keepclassmembers class com.google.android.exoplayer2.source.smoothstreaming.offline.SsDownloader { -keepclassmembers class com.google.android.exoplayer2.source.smoothstreaming.offline.SsDownloader {
<init>(android.net.Uri, java.util.List, com.google.android.exoplayer2.upstream.cache.CacheDataSource.Factory, java.util.concurrent.ExecutorService); <init>(android.net.Uri, java.util.List, com.google.android.exoplayer2.upstream.cache.CacheDataSource.Factory, java.util.concurrent.Executor);
} }
# Constructors accessed via reflection in DefaultMediaSourceFactory and DownloadHelper # Constructors accessed via reflection in DefaultMediaSourceFactory and DownloadHelper

View File

@ -20,7 +20,7 @@ import androidx.annotation.Nullable;
import com.google.android.exoplayer2.upstream.cache.CacheDataSource; import com.google.android.exoplayer2.upstream.cache.CacheDataSource;
import java.lang.reflect.Constructor; import java.lang.reflect.Constructor;
import java.util.List; import java.util.List;
import java.util.concurrent.ExecutorService; import java.util.concurrent.Executor;
/** /**
* Default {@link DownloaderFactory}, supporting creation of progressive, DASH, HLS and * Default {@link DownloaderFactory}, supporting creation of progressive, DASH, HLS and
@ -71,7 +71,7 @@ public class DefaultDownloaderFactory implements DownloaderFactory {
} }
private final CacheDataSource.Factory cacheDataSourceFactory; private final CacheDataSource.Factory cacheDataSourceFactory;
@Nullable private final ExecutorService executorService; private final Executor executor;
/** /**
* Creates an instance. * Creates an instance.
@ -80,7 +80,7 @@ public class DefaultDownloaderFactory implements DownloaderFactory {
* downloads will be written. * downloads will be written.
*/ */
public DefaultDownloaderFactory(CacheDataSource.Factory cacheDataSourceFactory) { public DefaultDownloaderFactory(CacheDataSource.Factory cacheDataSourceFactory) {
this(cacheDataSourceFactory, /* executorService= */ null); this(cacheDataSourceFactory, Runnable::run);
} }
/** /**
@ -88,16 +88,14 @@ public class DefaultDownloaderFactory implements DownloaderFactory {
* *
* @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which * @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which
* downloads will be written. * downloads will be written.
* @param executorService An {@link ExecutorService} used to make requests for media being * @param executor An {@link Executor} used to make requests for media being downloaded. Providing
* downloaded. Must not be a direct executor, but may be {@code null} if each download should * an {@link Executor} that uses multiple threads will speed up download tasks that can be
* make its requests directly on its own thread. Providing an {@link ExecutorService} that * split into smaller parts for parallel execution.
* uses multiple threads will speed up download tasks that can be split into smaller parts for
* parallel execution.
*/ */
public DefaultDownloaderFactory( public DefaultDownloaderFactory(
CacheDataSource.Factory cacheDataSourceFactory, @Nullable ExecutorService executorService) { CacheDataSource.Factory cacheDataSourceFactory, Executor executor) {
this.cacheDataSourceFactory = cacheDataSourceFactory; this.cacheDataSourceFactory = cacheDataSourceFactory;
this.executorService = executorService; this.executor = executor;
} }
@Override @Override
@ -105,7 +103,7 @@ public class DefaultDownloaderFactory implements DownloaderFactory {
switch (request.type) { switch (request.type) {
case DownloadRequest.TYPE_PROGRESSIVE: case DownloadRequest.TYPE_PROGRESSIVE:
return new ProgressiveDownloader( return new ProgressiveDownloader(
request.uri, request.customCacheKey, cacheDataSourceFactory, executorService); request.uri, request.customCacheKey, cacheDataSourceFactory, executor);
case DownloadRequest.TYPE_DASH: case DownloadRequest.TYPE_DASH:
return createDownloader(request, DASH_DOWNLOADER_CONSTRUCTOR); return createDownloader(request, DASH_DOWNLOADER_CONSTRUCTOR);
case DownloadRequest.TYPE_HLS: case DownloadRequest.TYPE_HLS:
@ -117,10 +115,6 @@ public class DefaultDownloaderFactory implements DownloaderFactory {
} }
} }
// Checker framework has no way of knowing whether arguments to newInstance can be null or not,
// and so opts to be conservative and assumes they cannot. In this case executorService can be
// nullable, so we suppress the warning.
@SuppressWarnings("nullness:argument.type.incompatible")
private Downloader createDownloader( private Downloader createDownloader(
DownloadRequest request, @Nullable Constructor<? extends Downloader> constructor) { DownloadRequest request, @Nullable Constructor<? extends Downloader> constructor) {
if (constructor == null) { if (constructor == null) {
@ -128,7 +122,7 @@ public class DefaultDownloaderFactory implements DownloaderFactory {
} }
try { try {
return constructor.newInstance( return constructor.newInstance(
request.uri, request.streamKeys, cacheDataSourceFactory, executorService); request.uri, request.streamKeys, cacheDataSourceFactory, executor);
} catch (Exception e) { } catch (Exception e) {
throw new RuntimeException("Failed to instantiate downloader for: " + request.type, e); throw new RuntimeException("Failed to instantiate downloader for: " + request.type, e);
} }
@ -139,8 +133,7 @@ public class DefaultDownloaderFactory implements DownloaderFactory {
try { try {
return clazz return clazz
.asSubclass(Downloader.class) .asSubclass(Downloader.class)
.getConstructor( .getConstructor(Uri.class, List.class, CacheDataSource.Factory.class, Executor.class);
Uri.class, List.class, CacheDataSource.Factory.class, ExecutorService.class);
} catch (NoSuchMethodException e) { } catch (NoSuchMethodException e) {
// The downloader is present, but the expected constructor is missing. // The downloader is present, but the expected constructor is missing.
throw new RuntimeException("Downloader constructor missing", e); throw new RuntimeException("Downloader constructor missing", e);

View File

@ -23,7 +23,7 @@ import com.google.android.exoplayer2.upstream.cache.CacheDataSource;
import com.google.android.exoplayer2.upstream.cache.CacheUtil; import com.google.android.exoplayer2.upstream.cache.CacheUtil;
import com.google.android.exoplayer2.util.PriorityTaskManager; import com.google.android.exoplayer2.util.PriorityTaskManager;
import java.io.IOException; import java.io.IOException;
import java.util.concurrent.ExecutorService; import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
/** A downloader for progressive media streams. */ /** A downloader for progressive media streams. */
@ -44,7 +44,7 @@ public final class ProgressiveDownloader implements Downloader {
*/ */
public ProgressiveDownloader( public ProgressiveDownloader(
Uri uri, @Nullable String customCacheKey, CacheDataSource.Factory cacheDataSourceFactory) { Uri uri, @Nullable String customCacheKey, CacheDataSource.Factory cacheDataSourceFactory) {
this(uri, customCacheKey, cacheDataSourceFactory, /* executorService= */ null); this(uri, customCacheKey, cacheDataSourceFactory, Runnable::run);
} }
/** /**
@ -53,17 +53,15 @@ public final class ProgressiveDownloader implements Downloader {
* indexing. May be null. * indexing. May be null.
* @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the * @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the
* download will be written. * download will be written.
* @param executorService An {@link ExecutorService} used to make requests for the media being * @param executor An {@link Executor} used to make requests for the media being downloaded. In
* downloaded. Must not be a direct executor, but may be {@code null} if the requests should * the future, providing an {@link Executor} that uses multiple threads may speed up the
* be made directly from the thread that calls {@link #download(ProgressListener)}. In the
* future, providing an {@link ExecutorService} that uses multiple threads may speed up the
* download by allowing parts of it to be executed in parallel. * download by allowing parts of it to be executed in parallel.
*/ */
public ProgressiveDownloader( public ProgressiveDownloader(
Uri uri, Uri uri,
@Nullable String customCacheKey, @Nullable String customCacheKey,
CacheDataSource.Factory cacheDataSourceFactory, CacheDataSource.Factory cacheDataSourceFactory,
@Nullable ExecutorService executorService) { Executor executor) {
dataSpec = dataSpec =
new DataSpec.Builder() new DataSpec.Builder()
.setUri(uri) .setUri(uri)

View File

@ -33,7 +33,7 @@ import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.concurrent.ExecutorService; import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
/** /**
@ -70,7 +70,7 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
private final DataSpec manifestDataSpec; private final DataSpec manifestDataSpec;
private final ArrayList<StreamKey> streamKeys; private final ArrayList<StreamKey> streamKeys;
private final CacheDataSource.Factory cacheDataSourceFactory; private final CacheDataSource.Factory cacheDataSourceFactory;
@Nullable private final ExecutorService executorService; private final Executor executor;
private final AtomicBoolean isCanceled; private final AtomicBoolean isCanceled;
/** /**
@ -79,21 +79,19 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
* If empty, all streams are downloaded. * If empty, all streams are downloaded.
* @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the * @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the
* download will be written. * download will be written.
* @param executorService An {@link ExecutorService} used to make requests for the media being * @param executor An {@link Executor} used to make requests for the media being downloaded.
* downloaded. Must not be a direct executor, but may be {@code null} if the requests should * Providing an {@link Executor} that uses multiple threads will speed up the download by
* be made directly from the thread that calls {@link #download(ProgressListener)}. Providing
* an {@link ExecutorService} that uses multiple threads will speed up the download by
* allowing parts of it to be executed in parallel. * allowing parts of it to be executed in parallel.
*/ */
public SegmentDownloader( public SegmentDownloader(
Uri manifestUri, Uri manifestUri,
List<StreamKey> streamKeys, List<StreamKey> streamKeys,
CacheDataSource.Factory cacheDataSourceFactory, CacheDataSource.Factory cacheDataSourceFactory,
@Nullable ExecutorService executorService) { Executor executor) {
this.manifestDataSpec = getCompressibleDataSpec(manifestUri); this.manifestDataSpec = getCompressibleDataSpec(manifestUri);
this.streamKeys = new ArrayList<>(streamKeys); this.streamKeys = new ArrayList<>(streamKeys);
this.cacheDataSourceFactory = cacheDataSourceFactory; this.cacheDataSourceFactory = cacheDataSourceFactory;
this.executorService = executorService; this.executor = executor;
isCanceled = new AtomicBoolean(); isCanceled = new AtomicBoolean();
} }

View File

@ -38,7 +38,7 @@ import com.google.android.exoplayer2.upstream.cache.CacheDataSource;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.concurrent.ExecutorService; import java.util.concurrent.Executor;
/** /**
* A downloader for DASH streams. * A downloader for DASH streams.
@ -74,7 +74,7 @@ public final class DashDownloader extends SegmentDownloader<DashManifest> {
*/ */
public DashDownloader( public DashDownloader(
Uri manifestUri, List<StreamKey> streamKeys, CacheDataSource.Factory cacheDataSourceFactory) { Uri manifestUri, List<StreamKey> streamKeys, CacheDataSource.Factory cacheDataSourceFactory) {
this(manifestUri, streamKeys, cacheDataSourceFactory, /* executorService= */ null); this(manifestUri, streamKeys, cacheDataSourceFactory, Runnable::run);
} }
/** /**
@ -83,18 +83,16 @@ public final class DashDownloader extends SegmentDownloader<DashManifest> {
* download. If empty, all representations are downloaded. * download. If empty, all representations are downloaded.
* @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the * @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the
* download will be written. * download will be written.
* @param executorService An {@link ExecutorService} used to make requests for the media being * @param executor An {@link Executor} used to make requests for the media being downloaded.
* downloaded. Must not be a direct executor, but may be {@code null} if the requests should * Providing an {@link Executor} that uses multiple threads will speed up the download by
* be made directly from the thread that calls {@link #download(ProgressListener)}. Providing
* an {@link ExecutorService} that uses multiple threads will speed up the download by
* allowing parts of it to be executed in parallel. * allowing parts of it to be executed in parallel.
*/ */
public DashDownloader( public DashDownloader(
Uri manifestUri, Uri manifestUri,
List<StreamKey> streamKeys, List<StreamKey> streamKeys,
CacheDataSource.Factory cacheDataSourceFactory, CacheDataSource.Factory cacheDataSourceFactory,
@Nullable ExecutorService executorService) { Executor executor) {
super(manifestUri, streamKeys, cacheDataSourceFactory, executorService); super(manifestUri, streamKeys, cacheDataSourceFactory, executor);
} }
@Override @Override

View File

@ -16,7 +16,6 @@
package com.google.android.exoplayer2.source.hls.offline; package com.google.android.exoplayer2.source.hls.offline;
import android.net.Uri; import android.net.Uri;
import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.offline.SegmentDownloader; import com.google.android.exoplayer2.offline.SegmentDownloader;
import com.google.android.exoplayer2.offline.StreamKey; import com.google.android.exoplayer2.offline.StreamKey;
@ -33,7 +32,7 @@ import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.concurrent.ExecutorService; import java.util.concurrent.Executor;
/** /**
* A downloader for HLS streams. * A downloader for HLS streams.
@ -69,7 +68,7 @@ public final class HlsDownloader extends SegmentDownloader<HlsPlaylist> {
*/ */
public HlsDownloader( public HlsDownloader(
Uri playlistUri, List<StreamKey> streamKeys, CacheDataSource.Factory cacheDataSourceFactory) { Uri playlistUri, List<StreamKey> streamKeys, CacheDataSource.Factory cacheDataSourceFactory) {
this(playlistUri, streamKeys, cacheDataSourceFactory, /* executorService= */ null); this(playlistUri, streamKeys, cacheDataSourceFactory, Runnable::run);
} }
/** /**
@ -78,18 +77,16 @@ public final class HlsDownloader extends SegmentDownloader<HlsPlaylist> {
* download. If empty, all renditions are downloaded. * download. If empty, all renditions are downloaded.
* @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the * @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the
* download will be written. * download will be written.
* @param executorService An {@link ExecutorService} used to make requests for the media being * @param executor An {@link Executor} used to make requests for the media being downloaded.
* downloaded. Must not be a direct executor, but may be {@code null} if the requests should * Providing an {@link Executor} that uses multiple threads will speed up the download by
* be made directly from the thread that calls {@link #download(ProgressListener)}. Providing
* an {@link ExecutorService} that uses multiple threads will speed up the download by
* allowing parts of it to be executed in parallel. * allowing parts of it to be executed in parallel.
*/ */
public HlsDownloader( public HlsDownloader(
Uri playlistUri, Uri playlistUri,
List<StreamKey> streamKeys, List<StreamKey> streamKeys,
CacheDataSource.Factory cacheDataSourceFactory, CacheDataSource.Factory cacheDataSourceFactory,
@Nullable ExecutorService executorService) { Executor executor) {
super(playlistUri, streamKeys, cacheDataSourceFactory, executorService); super(playlistUri, streamKeys, cacheDataSourceFactory, executor);
} }
@Override @Override

View File

@ -16,7 +16,6 @@
package com.google.android.exoplayer2.source.smoothstreaming.offline; package com.google.android.exoplayer2.source.smoothstreaming.offline;
import android.net.Uri; import android.net.Uri;
import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.offline.SegmentDownloader; import com.google.android.exoplayer2.offline.SegmentDownloader;
import com.google.android.exoplayer2.offline.StreamKey; import com.google.android.exoplayer2.offline.StreamKey;
@ -31,7 +30,7 @@ import com.google.android.exoplayer2.upstream.cache.CacheDataSource;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.concurrent.ExecutorService; import java.util.concurrent.Executor;
/** /**
* A downloader for SmoothStreaming streams. * A downloader for SmoothStreaming streams.
@ -68,7 +67,7 @@ public final class SsDownloader extends SegmentDownloader<SsManifest> {
*/ */
public SsDownloader( public SsDownloader(
Uri manifestUri, List<StreamKey> streamKeys, CacheDataSource.Factory cacheDataSourceFactory) { Uri manifestUri, List<StreamKey> streamKeys, CacheDataSource.Factory cacheDataSourceFactory) {
this(manifestUri, streamKeys, cacheDataSourceFactory, /* executorService= */ null); this(manifestUri, streamKeys, cacheDataSourceFactory, Runnable::run);
} }
/** /**
@ -77,18 +76,16 @@ public final class SsDownloader extends SegmentDownloader<SsManifest> {
* If empty, all streams are downloaded. * If empty, all streams are downloaded.
* @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the * @param cacheDataSourceFactory A {@link CacheDataSource.Factory} for the cache into which the
* download will be written. * download will be written.
* @param executorService An {@link ExecutorService} used to make requests for the media being * @param executor An {@link Executor} used to make requests for the media being downloaded.
* downloaded. Must not be a direct executor, but may be {@code null} if the requests should * Providing an {@link Executor} that uses multiple threads will speed up the download by
* be made directly from the thread that calls {@link #download(ProgressListener)}. Providing
* an {@link ExecutorService} that uses multiple threads will speed up the download by
* allowing parts of it to be executed in parallel. * allowing parts of it to be executed in parallel.
*/ */
public SsDownloader( public SsDownloader(
Uri manifestUri, Uri manifestUri,
List<StreamKey> streamKeys, List<StreamKey> streamKeys,
CacheDataSource.Factory cacheDataSourceFactory, CacheDataSource.Factory cacheDataSourceFactory,
@Nullable ExecutorService executorService) { Executor executor) {
super(SsUtil.fixManifestUri(manifestUri), streamKeys, cacheDataSourceFactory, executorService); super(SsUtil.fixManifestUri(manifestUri), streamKeys, cacheDataSourceFactory, executor);
} }
@Override @Override