diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 3be8af67631..5ed1dfd44f9 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -145,6 +145,9 @@ ([#7992](https://github.com/google/ExoPlayer/issues/7992)). * FLV: Make files seekable by using the key frame index ([#7378](https://github.com/google/ExoPlayer/issues/7378)). +* Downloads: Fix issue retrying progressive downloads, which could also result + in a crash in `DownloadManager.InternalHandler.onContentLengthChanged` + ([#8078](https://github.com/google/ExoPlayer/issues/8078). * HLS: Fix crash affecting chunkful preparation of master playlists that start with an I-FRAME only variant ([#8025](https://github.com/google/ExoPlayer/issues/8025)). diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java index 09fa444cf34..a4cbe17b82c 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java @@ -38,6 +38,7 @@ public final class ProgressiveDownloader implements Downloader { private final Executor executor; private final DataSpec dataSpec; private final CacheDataSource dataSource; + private final CacheWriter cacheWriter; @Nullable private final PriorityTaskManager priorityTaskManager; @Nullable private ProgressListener progressListener; @@ -101,6 +102,15 @@ public ProgressiveDownloader( .setFlags(DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION) .build(); dataSource = cacheDataSourceFactory.createDataSourceForDownloading(); + @SuppressWarnings("methodref.receiver.bound.invalid") + CacheWriter.ProgressListener progressListener = this::onProgress; + cacheWriter = + new CacheWriter( + dataSource, + dataSpec, + /* allowShortContent= */ false, + /* temporaryBuffer= */ null, + progressListener); priorityTaskManager = cacheDataSourceFactory.getUpstreamPriorityTaskManager(); } @@ -108,28 +118,19 @@ public ProgressiveDownloader( public void download(@Nullable ProgressListener progressListener) throws IOException, InterruptedException { this.progressListener = progressListener; - if (downloadRunnable == null) { - CacheWriter cacheWriter = - new CacheWriter( - dataSource, - dataSpec, - /* allowShortContent= */ false, - /* temporaryBuffer= */ null, - this::onProgress); - downloadRunnable = - new RunnableFutureTask() { - @Override - protected Void doWork() throws IOException { - cacheWriter.cache(); - return null; - } + downloadRunnable = + new RunnableFutureTask() { + @Override + protected Void doWork() throws IOException { + cacheWriter.cache(); + return null; + } - @Override - protected void cancelWork() { - cacheWriter.cancel(); - } - }; - } + @Override + protected void cancelWork() { + cacheWriter.cancel(); + } + }; if (priorityTaskManager != null) { priorityTaskManager.add(C.PRIORITY_DOWNLOAD); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/offline/ProgressiveDownloaderTest.java b/library/core/src/test/java/com/google/android/exoplayer2/offline/ProgressiveDownloaderTest.java new file mode 100644 index 00000000000..52d83c133ab --- /dev/null +++ b/library/core/src/test/java/com/google/android/exoplayer2/offline/ProgressiveDownloaderTest.java @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.android.exoplayer2.offline; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import android.net.Uri; +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.MediaItem; +import com.google.android.exoplayer2.database.DatabaseProvider; +import com.google.android.exoplayer2.testutil.FakeDataSet; +import com.google.android.exoplayer2.testutil.FakeDataSource; +import com.google.android.exoplayer2.testutil.TestUtil; +import com.google.android.exoplayer2.upstream.DataSource; +import com.google.android.exoplayer2.upstream.cache.Cache; +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 com.google.android.exoplayer2.util.Util; +import java.io.File; +import java.io.IOException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Unit tests for {@link ActionFile}. */ +@SuppressWarnings("deprecation") +@RunWith(AndroidJUnit4.class) +public class ProgressiveDownloaderTest { + + private File testDir; + private Cache downloadCache; + + @Before + public void createDownloadCache() throws Exception { + testDir = + Util.createTempFile( + ApplicationProvider.getApplicationContext(), "ProgressiveDownloaderTest"); + assertThat(testDir.delete()).isTrue(); + assertThat(testDir.mkdirs()).isTrue(); + + DatabaseProvider databaseProvider = TestUtil.getInMemoryDatabaseProvider(); + downloadCache = new SimpleCache(testDir, new NoOpCacheEvictor(), databaseProvider); + } + + @After + public void deleteDownloadCache() { + downloadCache.release(); + Util.recursiveDelete(testDir); + } + + @Test + public void download_afterSingleFailure_succeeds() throws Exception { + Uri uri = Uri.parse("test:///test.mp4"); + + // Fake data has a built in failure after 10 bytes. + FakeDataSet data = new FakeDataSet(); + data.newData(uri).appendReadData(10).appendReadError(new IOException()).appendReadData(20); + DataSource.Factory upstreamDataSource = new FakeDataSource.Factory().setFakeDataSet(data); + + MediaItem mediaItem = MediaItem.fromUri(uri); + CacheDataSource.Factory cacheDataSourceFactory = + new CacheDataSource.Factory() + .setCache(downloadCache) + .setUpstreamDataSourceFactory(upstreamDataSource); + ProgressiveDownloader downloader = new ProgressiveDownloader(mediaItem, cacheDataSourceFactory); + + TestProgressListener progressListener = new TestProgressListener(); + + // Failure expected after 10 bytes. + assertThrows(IOException.class, () -> downloader.download(progressListener)); + assertThat(progressListener.bytesDownloaded).isEqualTo(10); + + // Retry should succeed. + downloader.download(progressListener); + assertThat(progressListener.bytesDownloaded).isEqualTo(30); + } + + private static final class TestProgressListener implements Downloader.ProgressListener { + + public long bytesDownloaded; + + @Override + public void onProgress(long contentLength, long bytesDownloaded, float percentDownloaded) { + this.bytesDownloaded = bytesDownloaded; + } + } +}