From ece93cadd523e297383d0990d5152cf343c74b61 Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Tue, 27 Mar 2018 12:10:48 -0700 Subject: [PATCH] -Added better player exception handling to player. -Added expired media source cleaning to media source manager. --- .../local/bookmark/BookmarkFragment.java | 3 +- .../org/schabi/newpipe/player/BasePlayer.java | 29 ++++---------- .../player/mediasource/FailedMediaSource.java | 32 ++++++++++++--- .../ManagedMediaSourcePlaylist.java | 13 ++++--- .../player/playback/MediaSourceManager.java | 39 ++++++++++--------- 5 files changed, 65 insertions(+), 51 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java index b740cb15e..e1f724b6e 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java @@ -56,7 +56,8 @@ public final class BookmarkFragment @Override public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - final AppDatabase database = NewPipeDatabase.getInstance(getContext()); + if (activity == null) return; + final AppDatabase database = NewPipeDatabase.getInstance(activity); localPlaylistManager = new LocalPlaylistManager(database); remotePlaylistManager = new RemotePlaylistManager(database); disposables = new CompositeDisposable(); diff --git a/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java b/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java index 8d0c22a4d..e2fd2e6f7 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java @@ -64,6 +64,7 @@ import org.schabi.newpipe.player.helper.LoadController; import org.schabi.newpipe.player.helper.MediaSessionManager; import org.schabi.newpipe.player.helper.PlayerDataSource; import org.schabi.newpipe.player.helper.PlayerHelper; +import org.schabi.newpipe.player.mediasource.FailedMediaSource; import org.schabi.newpipe.player.playback.BasePlayerMediaSession; import org.schabi.newpipe.player.playback.CustomTrackSelector; import org.schabi.newpipe.player.playback.MediaSourceManager; @@ -700,26 +701,6 @@ public abstract class BasePlayer implements } } - /** - * Processes {@link ExoPlaybackException} tagged with {@link ExoPlaybackException#TYPE_SOURCE}. - *

- * If the current {@link com.google.android.exoplayer2.Timeline.Window window} is valid, - * then we know the error is produced by transitioning into a bad window, therefore we report - * an error to the play queue based on if the current error can be skipped. - *

- * This is done because ExoPlayer reports the source exceptions before window is - * transitioned on seamless playback. Because player error causes ExoPlayer to go - * back to {@link Player#STATE_IDLE STATE_IDLE}, we reset and prepare the media source - * again to resume playback. - *

- * In the event that this error is produced during a valid stream playback, we save the - * current position so the playback may be recovered and resumed manually by the user. This - * happens only if the playback is {@link #RECOVERY_SKIP_THRESHOLD} milliseconds until complete. - *

- * In the event of livestreaming being lagged behind for any reason, most notably pausing for - * too long, a {@link BehindLiveWindowException} will be produced. This will trigger a reload - * instead of skipping or removal. - * */ private void processSourceError(final IOException error) { if (simpleExoPlayer == null || playQueue == null) return; @@ -733,8 +714,14 @@ public abstract class BasePlayer implements reload(); } else if (cause instanceof UnknownHostException) { playQueue.error(/*isNetworkProblem=*/true); + } else if (isCurrentWindowValid()) { + playQueue.error(/*isTransitioningToBadStream=*/true); + } else if (error instanceof FailedMediaSource.MediaSourceResolutionException) { + playQueue.error(/*recoverableWithNoAvailableStream=*/false); + } else if (error instanceof FailedMediaSource.StreamInfoLoadException) { + playQueue.error(/*recoverableIfLoadFailsWhenNetworkIsFine=*/false); } else { - playQueue.error(isCurrentWindowValid()); + playQueue.error(/*noIdeaWhatHappenedAndLetUserChooseWhatToDo=*/true); } } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediasource/FailedMediaSource.java b/app/src/main/java/org/schabi/newpipe/player/mediasource/FailedMediaSource.java index 5f029cc50..625c5d416 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediasource/FailedMediaSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediasource/FailedMediaSource.java @@ -14,13 +14,35 @@ import java.io.IOException; public class FailedMediaSource implements ManagedMediaSource { private final String TAG = "FailedMediaSource@" + Integer.toHexString(hashCode()); + public static class FailedMediaSourceException extends IOException { + FailedMediaSourceException(String message) { + super(message); + } + + FailedMediaSourceException(Throwable cause) { + super(cause); + } + } + + public static final class MediaSourceResolutionException extends FailedMediaSourceException { + public MediaSourceResolutionException(String message) { + super(message); + } + } + + public static final class StreamInfoLoadException extends FailedMediaSourceException { + public StreamInfoLoadException(Throwable cause) { + super(cause); + } + } + private final PlayQueueItem playQueueItem; - private final Throwable error; + private final FailedMediaSourceException error; private final long retryTimestamp; public FailedMediaSource(@NonNull final PlayQueueItem playQueueItem, - @NonNull final Throwable error, + @NonNull final FailedMediaSourceException error, final long retryTimestamp) { this.playQueueItem = playQueueItem; this.error = error; @@ -32,7 +54,7 @@ public class FailedMediaSource implements ManagedMediaSource { * The error will always be propagated to ExoPlayer. * */ public FailedMediaSource(@NonNull final PlayQueueItem playQueueItem, - @NonNull final Throwable error) { + @NonNull final FailedMediaSourceException error) { this.playQueueItem = playQueueItem; this.error = error; this.retryTimestamp = Long.MAX_VALUE; @@ -42,7 +64,7 @@ public class FailedMediaSource implements ManagedMediaSource { return playQueueItem; } - public Throwable getError() { + public FailedMediaSourceException getError() { return error; } @@ -57,7 +79,7 @@ public class FailedMediaSource implements ManagedMediaSource { @Override public void maybeThrowSourceInfoRefreshError() throws IOException { - throw new IOException(error); + throw error; } @Override diff --git a/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSourcePlaylist.java b/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSourcePlaylist.java index 30a5f9e76..3cbc75395 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSourcePlaylist.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSourcePlaylist.java @@ -129,15 +129,16 @@ public class ManagedMediaSourcePlaylist { if (index < 0 || index >= internalSource.getSize()) return; // Add and remove are sequential on the same thread, therefore here, the exoplayer - // message queue must receive and process add before remove. + // message queue must receive and process add before remove, effectively treating them + // as atomic. - // However, finalizing action occurs strictly after the timeline has completed - // all its changes on the playback thread, so it is possible, in the meantime, other calls - // that modifies the playlist media source may occur in between. Therefore, - // it is not safe to call remove as the finalizing action of add. + // Since the finalizing action occurs strictly after the timeline has completed + // all its changes on the playback thread, thus, it is possible, in the meantime, + // other calls that modifies the playlist media source occur in between. This makes + // it unsafe to call remove as the finalizing action of add. internalSource.addMediaSource(index + 1, source); - // Also, because of the above, it is thus only safe to synchronize the player + // Because of the above race condition, it is thus only safe to synchronize the player // in the finalizing action AFTER the removal is complete and the timeline has changed. internalSource.removeMediaSource(index, finalizingAction); } diff --git a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java index 38b0bf9a4..33cd7cdc0 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java +++ b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java @@ -2,6 +2,7 @@ package org.schabi.newpipe.player.playback; import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import android.support.v4.util.ArraySet; import android.util.Log; import com.google.android.exoplayer2.source.DynamicConcatenatingMediaSource; @@ -24,7 +25,6 @@ import org.schabi.newpipe.playlist.events.ReorderEvent; import org.schabi.newpipe.util.ServiceHelper; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -39,6 +39,7 @@ import io.reactivex.functions.Consumer; import io.reactivex.internal.subscriptions.EmptySubscription; import io.reactivex.subjects.PublishSubject; +import static org.schabi.newpipe.player.mediasource.FailedMediaSource.*; import static org.schabi.newpipe.playlist.PlayQueue.DEBUG; public class MediaSourceManager { @@ -144,7 +145,7 @@ public class MediaSourceManager { this.playlist = new ManagedMediaSourcePlaylist(); - this.loadingItems = Collections.synchronizedSet(new HashSet<>()); + this.loadingItems = Collections.synchronizedSet(new ArraySet<>()); playQueue.getBroadcastReceiver() .observeOn(AndroidSchedulers.mainThread()) @@ -321,9 +322,9 @@ public class MediaSourceManager { } private void maybeSynchronizePlayer() { - cleanSweep(); maybeUnblock(); maybeSync(); + cleanPlaylist(); } /*////////////////////////////////////////////////////////////////////////// @@ -366,7 +367,7 @@ public class MediaSourceManager { final int leftBound = Math.max(0, currentIndex - WINDOW_SIZE); final int rightLimit = currentIndex + WINDOW_SIZE + 1; final int rightBound = Math.min(playQueue.size(), rightLimit); - final Set items = new HashSet<>( + final Set items = new ArraySet<>( playQueue.getStreams().subList(leftBound,rightBound)); // Do a round robin @@ -402,19 +403,19 @@ public class MediaSourceManager { return stream.getStream().map(streamInfo -> { final MediaSource source = playbackListener.sourceOf(stream, streamInfo); if (source == null) { - final Exception exception = new IllegalStateException( - "Unable to resolve source from stream info." + - " URL: " + stream.getUrl() + - ", audio count: " + streamInfo.getAudioStreams().size() + - ", video count: " + streamInfo.getVideoOnlyStreams().size() + - streamInfo.getVideoStreams().size()); - return new FailedMediaSource(stream, exception); + final String message = "Unable to resolve source from stream info." + + " URL: " + stream.getUrl() + + ", audio count: " + streamInfo.getAudioStreams().size() + + ", video count: " + streamInfo.getVideoOnlyStreams().size() + + streamInfo.getVideoStreams().size(); + return new FailedMediaSource(stream, new MediaSourceResolutionException(message)); } final long expiration = System.currentTimeMillis() + ServiceHelper.getCacheExpirationMillis(streamInfo.getServiceId()); return new LoadedMediaSource(source, stream, expiration); - }).onErrorReturn(throwable -> new FailedMediaSource(stream, throwable)); + }).onErrorReturn(throwable -> new FailedMediaSource(stream, + new StreamInfoLoadException(throwable))); } private void onMediaSourceReceived(@NonNull final PlayQueueItem item, @@ -478,13 +479,15 @@ public class MediaSourceManager { } /** - * Scans the entire playlist for {@link MediaSource}s that requires correction, - * and replace these sources with a {@link PlaceholderMediaSource}. + * Scans the entire playlist for {@link ManagedMediaSource}s that requires correction, + * and replaces these sources with a {@link PlaceholderMediaSource} if they are not part + * of the excluded items. * */ - private void cleanSweep() { - for (int index = 0; index < playlist.size(); index++) { - if (isCorrectionNeeded(playQueue.getItem(index))) { - playlist.invalidate(index); + private void cleanPlaylist() { + if (DEBUG) Log.d(TAG, "cleanPlaylist() called."); + for (final PlayQueueItem item : playQueue.getStreams()) { + if (isCorrectionNeeded(item)) { + playlist.invalidate(playQueue.indexOf(item)); } } }