From e9413b0d4180036e0163e8e38c41d244b6454bd3 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 18 Apr 2019 19:19:38 +0100 Subject: [PATCH] Simplify DownloadManager constructors PiperOrigin-RevId: 244223870 --- .../exoplayer2/demo/DemoApplication.java | 8 +- .../exoplayer2/offline/DownloadManager.java | 105 +++++++----------- .../offline/DownloadManagerTest.java | 12 +- .../dash/offline/DownloadManagerDashTest.java | 6 +- .../dash/offline/DownloadServiceDashTest.java | 6 +- 5 files changed, 46 insertions(+), 91 deletions(-) diff --git a/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoApplication.java b/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoApplication.java index 446184e56b..2c9cd43d1e 100644 --- a/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoApplication.java +++ b/demos/main/src/main/java/com/google/android/exoplayer2/demo/DemoApplication.java @@ -49,7 +49,6 @@ public class DemoApplication extends Application { private static final String DOWNLOAD_ACTION_FILE = "actions"; private static final String DOWNLOAD_TRACKER_ACTION_FILE = "tracked_actions"; private static final String DOWNLOAD_CONTENT_DIRECTORY = "downloads"; - private static final int MAX_SIMULTANEOUS_DOWNLOADS = 2; protected String userAgent; @@ -122,12 +121,7 @@ public class DemoApplication extends Application { new DownloaderConstructorHelper(getDownloadCache(), buildHttpDataSourceFactory()); downloadManager = new DownloadManager( - this, - downloadIndex, - new DefaultDownloaderFactory(downloaderConstructorHelper), - MAX_SIMULTANEOUS_DOWNLOADS, - DownloadManager.DEFAULT_MIN_RETRY_COUNT, - DownloadManager.DEFAULT_REQUIREMENTS); + this, downloadIndex, new DefaultDownloaderFactory(downloaderConstructorHelper)); downloadTracker = new DownloadTracker(/* context= */ this, buildDataSourceFactory(), downloadManager); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java index c34a5e233a..f914c861f9 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java @@ -34,7 +34,6 @@ import android.os.Message; import androidx.annotation.IntDef; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; -import com.google.android.exoplayer2.database.DatabaseProvider; import com.google.android.exoplayer2.scheduler.Requirements; import com.google.android.exoplayer2.scheduler.RequirementsWatcher; import com.google.android.exoplayer2.upstream.cache.CacheUtil.CachingCounters; @@ -111,8 +110,8 @@ public final class DownloadManager { @Requirements.RequirementFlags int notMetRequirements) {} } - /** The default maximum number of simultaneous downloads. */ - public static final int DEFAULT_MAX_SIMULTANEOUS_DOWNLOADS = 1; + /** The default maximum number of parallel downloads. */ + public static final int DEFAULT_MAX_PARALLEL_DOWNLOADS = 3; /** The default minimum number of times a download must be retried before failing. */ public static final int DEFAULT_MIN_RETRY_COUNT = 5; /** The default requirement is that the device has network connectivity. */ @@ -151,8 +150,6 @@ public final class DownloadManager { private static final String TAG = "DownloadManager"; private static final boolean DEBUG = false; - private final int maxSimultaneousDownloads; - private final int minRetryCount; private final Context context; private final WritableDownloadIndex downloadIndex; private final DownloaderFactory downloaderFactory; @@ -180,51 +177,11 @@ public final class DownloadManager { // Mutable fields that are accessed on the internal thread. @Requirements.RequirementFlags private int notMetRequirements; private boolean downloadsResumed; - private int simultaneousDownloads; + private int parallelDownloads; - /** - * Constructs a {@link DownloadManager}. - * - * @param context Any context. - * @param databaseProvider Provides the database that holds the downloads. - * @param downloaderFactory A factory for creating {@link Downloader}s. - */ - public DownloadManager( - Context context, DatabaseProvider databaseProvider, DownloaderFactory downloaderFactory) { - this( - context, - databaseProvider, - downloaderFactory, - DEFAULT_MAX_SIMULTANEOUS_DOWNLOADS, - DEFAULT_MIN_RETRY_COUNT, - DEFAULT_REQUIREMENTS); - } - - /** - * Constructs a {@link DownloadManager}. - * - * @param context Any context. - * @param databaseProvider Provides the database that holds the downloads. - * @param downloaderFactory A factory for creating {@link Downloader}s. - * @param maxSimultaneousDownloads The maximum number of simultaneous downloads. - * @param minRetryCount The minimum number of times a download must be retried before failing. - * @param requirements The requirements needed to be met to start downloads. - */ - public DownloadManager( - Context context, - DatabaseProvider databaseProvider, - DownloaderFactory downloaderFactory, - int maxSimultaneousDownloads, - int minRetryCount, - Requirements requirements) { - this( - context, - new DefaultDownloadIndex(databaseProvider), - downloaderFactory, - maxSimultaneousDownloads, - minRetryCount, - requirements); - } + // TODO: Fix these to properly support changes at runtime. + private volatile int maxParallelDownloads; + private volatile int minRetryCount; /** * Constructs a {@link DownloadManager}. @@ -232,22 +189,14 @@ public final class DownloadManager { * @param context Any context. * @param downloadIndex The download index used to hold the download information. * @param downloaderFactory A factory for creating {@link Downloader}s. - * @param maxSimultaneousDownloads The maximum number of simultaneous downloads. - * @param minRetryCount The minimum number of times a download must be retried before failing. - * @param requirements The requirements needed to be met to start downloads. */ public DownloadManager( - Context context, - WritableDownloadIndex downloadIndex, - DownloaderFactory downloaderFactory, - int maxSimultaneousDownloads, - int minRetryCount, - Requirements requirements) { + Context context, WritableDownloadIndex downloadIndex, DownloaderFactory downloaderFactory) { this.context = context.getApplicationContext(); this.downloadIndex = downloadIndex; this.downloaderFactory = downloaderFactory; - this.maxSimultaneousDownloads = maxSimultaneousDownloads; - this.minRetryCount = minRetryCount; + maxParallelDownloads = DEFAULT_MAX_PARALLEL_DOWNLOADS; + minRetryCount = DEFAULT_MIN_RETRY_COUNT; downloadInternals = new ArrayList<>(); downloads = new ArrayList<>(); @@ -262,7 +211,8 @@ public final class DownloadManager { internalThread.start(); internalHandler = new Handler(internalThread.getLooper(), this::handleInternalMessage); - requirementsWatcher = new RequirementsWatcher(context, requirementsListener, requirements); + requirementsWatcher = + new RequirementsWatcher(context, requirementsListener, DEFAULT_REQUIREMENTS); int notMetRequirements = requirementsWatcher.start(); pendingMessages = 1; @@ -332,6 +282,27 @@ public final class DownloadManager { onRequirementsStateChanged(requirementsWatcher, notMetRequirements); } + /** + * Sets the maximum number of parallel downloads. + * + * @param maxParallelDownloads The maximum number of parallel downloads. + */ + // TODO: Fix to properly support changes at runtime. + public void setMaxParallelDownloads(int maxParallelDownloads) { + this.maxParallelDownloads = maxParallelDownloads; + } + + /** + * Sets the minimum number of times that a download will be retried. A download will fail if the + * specified number of retries is exceeded without any progress being made. + * + * @param minRetryCount The minimum number of times that a download will be retried. + */ + // TODO: Fix to properly support changes at runtime. + public void setMinRetryCount(int minRetryCount) { + this.minRetryCount = minRetryCount; + } + /** Returns the used {@link DownloadIndex}. */ public DownloadIndex getDownloadIndex() { return downloadIndex; @@ -696,15 +667,15 @@ public final class DownloadManager { downloadThreads.remove(downloadId); boolean tryToStartDownloads = false; if (!downloadThread.isRemove) { - // If maxSimultaneousDownloads was hit, there might be a download waiting for a slot. - tryToStartDownloads = simultaneousDownloads == maxSimultaneousDownloads; - simultaneousDownloads--; + // If maxParallelDownloads was hit, there might be a download waiting for a slot. + tryToStartDownloads = parallelDownloads == maxParallelDownloads; + parallelDownloads--; } getDownload(downloadId) .onDownloadThreadStopped(downloadThread.isCanceled, downloadThread.finalError); if (tryToStartDownloads) { for (int i = 0; - simultaneousDownloads < maxSimultaneousDownloads && i < downloadInternals.size(); + parallelDownloads < maxParallelDownloads && i < downloadInternals.size(); i++) { downloadInternals.get(i).start(); } @@ -760,10 +731,10 @@ public final class DownloadManager { } boolean isRemove = downloadInternal.isInRemoveState(); if (!isRemove) { - if (simultaneousDownloads == maxSimultaneousDownloads) { + if (parallelDownloads == maxParallelDownloads) { return START_THREAD_TOO_MANY_DOWNLOADS; } - simultaneousDownloads++; + parallelDownloads++; } Downloader downloader = downloaderFactory.createDownloader(request); DownloadThread downloadThread = diff --git a/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java index 7808a63012..7973174f6e 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java @@ -517,7 +517,7 @@ public class DownloadManagerTest { assertEqualIgnoringTimeFields(mergedDownload, expectedDownload); } - private void setUpDownloadManager(final int maxActiveDownloadTasks) throws Exception { + private void setUpDownloadManager(final int maxParallelDownloads) throws Exception { if (downloadManager != null) { releaseDownloadManager(); } @@ -526,12 +526,10 @@ public class DownloadManagerTest { () -> { downloadManager = new DownloadManager( - ApplicationProvider.getApplicationContext(), - downloadIndex, - downloaderFactory, - maxActiveDownloadTasks, - MIN_RETRY_COUNT, - new Requirements(0)); + ApplicationProvider.getApplicationContext(), downloadIndex, downloaderFactory); + downloadManager.setMaxParallelDownloads(maxParallelDownloads); + downloadManager.setMinRetryCount(MIN_RETRY_COUNT); + downloadManager.setRequirements(new Requirements(0)); downloadManager.resumeDownloads(); downloadManagerListener = new TestDownloadManagerListener(downloadManager, dummyMainThread); diff --git a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java index 18f9c0bd20..8cd6154373 100644 --- a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java +++ b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java @@ -32,7 +32,6 @@ import com.google.android.exoplayer2.offline.DownloadManager; import com.google.android.exoplayer2.offline.DownloadRequest; import com.google.android.exoplayer2.offline.DownloaderConstructorHelper; import com.google.android.exoplayer2.offline.StreamKey; -import com.google.android.exoplayer2.scheduler.Requirements; import com.google.android.exoplayer2.testutil.DummyMainThread; import com.google.android.exoplayer2.testutil.DummyMainThread.TestRunnable; import com.google.android.exoplayer2.testutil.FakeDataSet; @@ -259,10 +258,7 @@ public class DownloadManagerDashTest { ApplicationProvider.getApplicationContext(), downloadIndex, new DefaultDownloaderFactory( - new DownloaderConstructorHelper(cache, fakeDataSourceFactory)), - /* maxSimultaneousDownloads= */ 1, - /* minRetryCount= */ 3, - new Requirements(0)); + new DownloaderConstructorHelper(cache, fakeDataSourceFactory))); downloadManagerListener = new TestDownloadManagerListener( diff --git a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java index 2ded452bb1..c0da9251ec 100644 --- a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java +++ b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java @@ -35,7 +35,6 @@ import com.google.android.exoplayer2.offline.DownloadRequest; import com.google.android.exoplayer2.offline.DownloadService; import com.google.android.exoplayer2.offline.DownloaderConstructorHelper; import com.google.android.exoplayer2.offline.StreamKey; -import com.google.android.exoplayer2.scheduler.Requirements; import com.google.android.exoplayer2.scheduler.Scheduler; import com.google.android.exoplayer2.testutil.DummyMainThread; import com.google.android.exoplayer2.testutil.FakeDataSet; @@ -119,10 +118,7 @@ public class DownloadServiceDashTest { ApplicationProvider.getApplicationContext(), downloadIndex, new DefaultDownloaderFactory( - new DownloaderConstructorHelper(cache, fakeDataSourceFactory)), - /* maxSimultaneousDownloads= */ 1, - /* minRetryCount= */ 3, - new Requirements(0)); + new DownloaderConstructorHelper(cache, fakeDataSourceFactory))); downloadManagerListener = new TestDownloadManagerListener(dashDownloadManager, dummyMainThread); dashDownloadManager.resumeDownloads();