Merge pull request #10570 from Artemych:fix/progressive_downloader_infinite_loop

PiperOrigin-RevId: 472475124
(cherry picked from commit cec6f045ea216b2cb54075d3c8503c0923b06802)
This commit is contained in:
Marc Baechinger 2022-09-30 17:32:37 +00:00 committed by microkatz
parent 18188ebe01
commit 047310b31d
3 changed files with 68 additions and 15 deletions

View File

@ -11,6 +11,10 @@
([#10057](https://github.com/google/ExoPlayer/issues/10057)).
* Limit parallel download removals to 1 to avoid excessive thread creation
([#10458](https://github.com/google/ExoPlayer/issues/10458)).
* Downloads:
* Fix potential infinite loop in `ProgressiveDownloader` caused by
simultaneous download and playback with the same `PriorityTaskManager`
([#10570](https://github.com/google/ExoPlayer/pull/10570)).
* Audio:
* Adds `AudioOffloadListener.onExperimentalOffloadedPlayback` for the
AudioTrack offload state.

View File

@ -15,6 +15,8 @@
*/
package androidx.media3.exoplayer.offline;
import static androidx.media3.common.util.Assertions.checkNotNull;
import androidx.annotation.Nullable;
import androidx.media3.common.C;
import androidx.media3.common.MediaItem;
@ -90,26 +92,26 @@ public final class ProgressiveDownloader implements Downloader {
public void download(@Nullable ProgressListener progressListener)
throws IOException, InterruptedException {
this.progressListener = progressListener;
downloadRunnable =
new RunnableFutureTask<Void, IOException>() {
@Override
protected Void doWork() throws IOException {
cacheWriter.cache();
return null;
}
@Override
protected void cancelWork() {
cacheWriter.cancel();
}
};
if (priorityTaskManager != null) {
priorityTaskManager.add(C.PRIORITY_DOWNLOAD);
}
try {
boolean finished = false;
while (!finished && !isCanceled) {
// Recreate downloadRunnable on each loop iteration to avoid rethrowing a previous error.
downloadRunnable =
new RunnableFutureTask<Void, IOException>() {
@Override
protected Void doWork() throws IOException {
cacheWriter.cache();
return null;
}
@Override
protected void cancelWork() {
cacheWriter.cancel();
}
};
if (priorityTaskManager != null) {
priorityTaskManager.proceed(C.PRIORITY_DOWNLOAD);
}
@ -132,7 +134,7 @@ public final class ProgressiveDownloader implements Downloader {
} finally {
// If the main download thread was interrupted as part of cancelation, then it's possible that
// the runnable is still doing work. We need to wait until it's finished before returning.
downloadRunnable.blockUntilFinished();
checkNotNull(downloadRunnable).blockUntilFinished();
if (priorityTaskManager != null) {
priorityTaskManager.remove(C.PRIORITY_DOWNLOAD);
}

View File

@ -19,7 +19,9 @@ import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
import android.net.Uri;
import androidx.media3.common.C;
import androidx.media3.common.MediaItem;
import androidx.media3.common.PriorityTaskManager;
import androidx.media3.common.util.Util;
import androidx.media3.database.DatabaseProvider;
import androidx.media3.datasource.DataSource;
@ -126,6 +128,51 @@ public class ProgressiveDownloaderTest {
assertThat(progressListener.bytesDownloaded).isEqualTo(1024);
}
@Test
public void download_afterPriorityTooLow_succeeds() throws Exception {
PriorityTaskManager priorityTaskManager = new PriorityTaskManager();
AtomicBoolean hasSetPlaybackPriority = new AtomicBoolean(/* initialValue= */ false);
Uri uri = Uri.parse("test:///test.mp4");
// Fake data which briefly sets the playback priority while downloading for the first time.
FakeDataSet data = new FakeDataSet();
data.newData(uri)
.appendReadAction(
() -> {
if (!hasSetPlaybackPriority.getAndSet(true)) {
// This only interrupts the download the next time the DataSource checks for the
// priority, so delay the removal of the playback priority slightly. As we can't
// check when the DataSource throws the PriorityTooLowException exactly, we need to
// use an arbitrary delay.
priorityTaskManager.add(C.PRIORITY_PLAYBACK);
new Thread(
() -> {
try {
Thread.sleep(200);
} catch (InterruptedException e) {
// Ignore.
}
priorityTaskManager.remove(C.PRIORITY_PLAYBACK);
})
.start();
}
})
.appendReadData(2_000_000);
DataSource.Factory upstreamDataSource = new FakeDataSource.Factory().setFakeDataSet(data);
MediaItem mediaItem = MediaItem.fromUri(uri);
CacheDataSource.Factory cacheDataSourceFactory =
new CacheDataSource.Factory()
.setCache(downloadCache)
.setUpstreamDataSourceFactory(upstreamDataSource)
.setUpstreamPriorityTaskManager(priorityTaskManager);
ProgressiveDownloader downloader = new ProgressiveDownloader(mediaItem, cacheDataSourceFactory);
TestProgressListener progressListener = new TestProgressListener();
// Download expected to finish (despite the interruption by the higher playback priority).
downloader.download(progressListener);
assertThat(progressListener.bytesDownloaded).isEqualTo(2_000_000);
}
private static final class TestProgressListener implements Downloader.ProgressListener {
public long bytesDownloaded;