From 5167fe078bb066677644393bc52e6117d1e83c89 Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Sat, 17 Mar 2018 16:04:02 -0700 Subject: [PATCH] -Refactored synchronization checks out from MediaSourceManager to ManagedMediaSource. -Fixed null input causing potential NPE on PlayQueueItem. --- .../player/mediasource/FailedMediaSource.java | 8 ++++- .../player/mediasource/LoadedMediaSource.java | 10 ++++-- .../mediasource/ManagedMediaSource.java | 18 ++++++++++- .../mediasource/PlaceholderMediaSource.java | 8 ++++- .../player/playback/MediaSourceManager.java | 21 ++++--------- .../newpipe/playlist/PlayQueueItem.java | 31 ++++++++++--------- 6 files changed, 61 insertions(+), 35 deletions(-) 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 d07baf2a7..5f029cc50 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 @@ -72,7 +72,13 @@ public class FailedMediaSource implements ManagedMediaSource { public void releaseSource() {} @Override - public boolean canReplace(@NonNull final PlayQueueItem newIdentity) { + public boolean shouldBeReplacedWith(@NonNull final PlayQueueItem newIdentity, + final boolean isInterruptable) { return newIdentity != playQueueItem || canRetry(); } + + @Override + public boolean isStreamEqual(@NonNull PlayQueueItem stream) { + return playQueueItem == stream; + } } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediasource/LoadedMediaSource.java b/app/src/main/java/org/schabi/newpipe/player/mediasource/LoadedMediaSource.java index f523667f9..fe7508ecc 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediasource/LoadedMediaSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediasource/LoadedMediaSource.java @@ -59,7 +59,13 @@ public class LoadedMediaSource implements ManagedMediaSource { } @Override - public boolean canReplace(@NonNull final PlayQueueItem newIdentity) { - return newIdentity != stream || isExpired(); + public boolean shouldBeReplacedWith(@NonNull PlayQueueItem newIdentity, + final boolean isInterruptable) { + return newIdentity != stream || (isInterruptable && isExpired()); + } + + @Override + public boolean isStreamEqual(@NonNull PlayQueueItem stream) { + return this.stream == stream; } } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSource.java b/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSource.java index 3bb7ca429..46fd149bb 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediasource/ManagedMediaSource.java @@ -7,5 +7,21 @@ import com.google.android.exoplayer2.source.MediaSource; import org.schabi.newpipe.playlist.PlayQueueItem; public interface ManagedMediaSource extends MediaSource { - boolean canReplace(@NonNull final PlayQueueItem newIdentity); + /** + * Determines whether or not this {@link ManagedMediaSource} can be replaced. + * + * @param newIdentity a stream the {@link ManagedMediaSource} should encapsulate over, if + * it is different from the existing stream in the + * {@link ManagedMediaSource}, then it should be replaced. + * @param isInterruptable specifies if this {@link ManagedMediaSource} potentially + * being played. + * */ + boolean shouldBeReplacedWith(@NonNull final PlayQueueItem newIdentity, + final boolean isInterruptable); + + /** + * Determines if the {@link PlayQueueItem} is the one the + * {@link ManagedMediaSource} encapsulates over. + * */ + boolean isStreamEqual(@NonNull final PlayQueueItem stream); } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediasource/PlaceholderMediaSource.java b/app/src/main/java/org/schabi/newpipe/player/mediasource/PlaceholderMediaSource.java index 0d3436a01..2c57f2f9c 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediasource/PlaceholderMediaSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediasource/PlaceholderMediaSource.java @@ -19,7 +19,13 @@ public class PlaceholderMediaSource implements ManagedMediaSource { @Override public void releaseSource() {} @Override - public boolean canReplace(@NonNull final PlayQueueItem newIdentity) { + public boolean shouldBeReplacedWith(@NonNull PlayQueueItem newIdentity, + final boolean isInterruptable) { return true; } + + @Override + public boolean isStreamEqual(@NonNull PlayQueueItem stream) { + return false; + } } 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 170668169..477358113 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 @@ -268,15 +268,10 @@ public class MediaSourceManager { private boolean isPlaybackReady() { if (sources.getSize() != playQueue.size()) return false; - final MediaSource mediaSource = sources.getMediaSource(playQueue.getIndex()); + final ManagedMediaSource mediaSource = + (ManagedMediaSource) sources.getMediaSource(playQueue.getIndex()); final PlayQueueItem playQueueItem = playQueue.getItem(); - - if (mediaSource instanceof LoadedMediaSource) { - return playQueueItem == ((LoadedMediaSource) mediaSource).getStream(); - } else if (mediaSource instanceof FailedMediaSource) { - return playQueueItem == ((FailedMediaSource) mediaSource).getStream(); - } - return false; + return mediaSource.isStreamEqual(playQueueItem); } private void maybeBlock() { @@ -453,12 +448,8 @@ public class MediaSourceManager { if (index == -1 || index >= sources.getSize()) return false; final ManagedMediaSource mediaSource = (ManagedMediaSource) sources.getMediaSource(index); - - if (index == playQueue.getIndex() && mediaSource instanceof LoadedMediaSource) { - return item != ((LoadedMediaSource) mediaSource).getStream(); - } else { - return mediaSource.canReplace(item); - } + return mediaSource.shouldBeReplacedWith(item, + /*mightBeInProgress=*/index != playQueue.getIndex()); } /** @@ -479,7 +470,7 @@ public class MediaSourceManager { final ManagedMediaSource currentSource = (ManagedMediaSource) sources.getMediaSource(currentIndex); final PlayQueueItem currentItem = playQueue.getItem(); - if (!currentSource.canReplace(currentItem)) { + if (!currentSource.shouldBeReplacedWith(currentItem, /*canInterruptOnRenew=*/true)) { maybeSynchronizePlayer(); return; } diff --git a/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueItem.java b/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueItem.java index 752dc223d..df4d19720 100644 --- a/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueItem.java +++ b/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueItem.java @@ -11,20 +11,19 @@ import org.schabi.newpipe.util.ExtractorHelper; import java.io.Serializable; import io.reactivex.Single; -import io.reactivex.android.schedulers.AndroidSchedulers; -import io.reactivex.functions.Consumer; import io.reactivex.schedulers.Schedulers; public class PlayQueueItem implements Serializable { - final public static long RECOVERY_UNSET = Long.MIN_VALUE; + public final static long RECOVERY_UNSET = Long.MIN_VALUE; + private final static String EMPTY_STRING = ""; - final private String title; - final private String url; + @NonNull final private String title; + @NonNull final private String url; final private int serviceId; final private long duration; - final private String thumbnailUrl; - final private String uploader; - final private StreamType streamType; + @NonNull final private String thumbnailUrl; + @NonNull final private String uploader; + @NonNull final private StreamType streamType; private long recoveryPosition; private Throwable error; @@ -42,15 +41,16 @@ public class PlayQueueItem implements Serializable { item.getThumbnailUrl(), item.getUploaderName(), item.getStreamType()); } - private PlayQueueItem(final String name, final String url, final int serviceId, - final long duration, final String thumbnailUrl, final String uploader, - final StreamType streamType) { - this.title = name; - this.url = url; + private PlayQueueItem(@Nullable final String name, @Nullable final String url, + final int serviceId, final long duration, + @Nullable final String thumbnailUrl, @Nullable final String uploader, + @NonNull final StreamType streamType) { + this.title = name != null ? name : EMPTY_STRING; + this.url = url != null ? url : EMPTY_STRING; this.serviceId = serviceId; this.duration = duration; - this.thumbnailUrl = thumbnailUrl; - this.uploader = uploader; + this.thumbnailUrl = thumbnailUrl != null ? thumbnailUrl : EMPTY_STRING; + this.uploader = uploader != null ? uploader : EMPTY_STRING; this.streamType = streamType; this.recoveryPosition = RECOVERY_UNSET; @@ -84,6 +84,7 @@ public class PlayQueueItem implements Serializable { return uploader; } + @NonNull public StreamType getStreamType() { return streamType; }