From 743e16a8a7d285d1e08ec01c67cc762870329659 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 19:05:11 +0100 Subject: [PATCH] Quality/AudioTrack: ensure correct indices at construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of allowing to pass arbitrary out-of-bounds indexes to these bean classes, ensure that the index is always valid for the list. This is always true for our filter functions, except they all return `-1` if the list was empty. We have to check/assert that beforehand. This improves the logic somewhat, because fetching the stream always returns something now. Ideally, we wouldn’t be filtering stuff and then passing indices around everywhere, but that’s the current state of things. ~~~ I took the liberty to remove the `.of`-wrappers, because they don’t really add much compared to just calling the constructor here. --- .../player/mediaitem/MediaItemTag.java | 52 ++++++++++++------- .../player/mediaitem/StreamInfoTag.java | 27 +--------- .../resolver/AudioPlaybackResolver.java | 30 +++++------ .../player/resolver/PlaybackResolver.java | 2 +- .../resolver/VideoPlaybackResolver.java | 30 +++++++---- .../newpipe/player/ui/VideoPlayerUi.java | 2 +- .../org/schabi/newpipe/util/ListHelper.java | 8 +++ 7 files changed, 74 insertions(+), 77 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/mediaitem/MediaItemTag.java b/app/src/main/java/org/schabi/newpipe/player/mediaitem/MediaItemTag.java index 346bb92fa..2846d47ca 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediaitem/MediaItemTag.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediaitem/MediaItemTag.java @@ -106,19 +106,28 @@ public interface MediaItemTag { final class Quality { @NonNull private final List sortedVideoStreams; + + /** Invariant: Index exists in sortedVideoStreams. */ private final int selectedVideoStreamIndex; - private Quality(@NonNull final List sortedVideoStreams, + + /** Create a new video Quality. The index must be valid in `sortedVideoStreams`. + * + * @param sortedVideoStreams + * @param selectedVideoStreamIndex + * @throws ArrayIndexOutOfBoundsException if index does not exist in `sortedVideoStreams` + */ + public Quality(@NonNull final List sortedVideoStreams, final int selectedVideoStreamIndex) { + if (selectedVideoStreamIndex < 0 + || selectedVideoStreamIndex >= sortedVideoStreams.size()) { + throw new ArrayIndexOutOfBoundsException( + "selectedVideoStreamIndex does not exist in sortedVideoStreams"); + } this.sortedVideoStreams = sortedVideoStreams; this.selectedVideoStreamIndex = selectedVideoStreamIndex; } - static Quality of(@NonNull final List sortedVideoStreams, - final int selectedVideoStreamIndex) { - return new Quality(sortedVideoStreams, selectedVideoStreamIndex); - } - @NonNull public List getSortedVideoStreams() { return sortedVideoStreams; @@ -128,30 +137,35 @@ public interface MediaItemTag { return selectedVideoStreamIndex; } - @Nullable + @NonNull public VideoStream getSelectedVideoStream() { - return selectedVideoStreamIndex < 0 - || selectedVideoStreamIndex >= sortedVideoStreams.size() - ? null : sortedVideoStreams.get(selectedVideoStreamIndex); + return sortedVideoStreams.get(selectedVideoStreamIndex); } } final class AudioTrack { @NonNull private final List audioStreams; + /** Invariant: Index exists in audioStreams. */ private final int selectedAudioStreamIndex; - private AudioTrack(@NonNull final List audioStreams, + /** Create a new AudioTrack. The index must be valid in `audioStreams`. + * + * @param audioStreams + * @param selectedAudioStreamIndex + * @throws ArrayIndexOutOfBoundsException if index does not exist in audioStreams. + */ + public AudioTrack(@NonNull final List audioStreams, final int selectedAudioStreamIndex) { + if (selectedAudioStreamIndex < 0 + || selectedAudioStreamIndex >= audioStreams.size()) { + throw new ArrayIndexOutOfBoundsException( + "selectedAudioStreamIndex does not exist in audioStreams"); + } this.audioStreams = audioStreams; this.selectedAudioStreamIndex = selectedAudioStreamIndex; } - static AudioTrack of(@NonNull final List audioStreams, - final int selectedAudioStreamIndex) { - return new AudioTrack(audioStreams, selectedAudioStreamIndex); - } - @NonNull public List getAudioStreams() { return audioStreams; @@ -161,11 +175,9 @@ public interface MediaItemTag { return selectedAudioStreamIndex; } - @Nullable + @NonNull public AudioStream getSelectedAudioStream() { - return selectedAudioStreamIndex < 0 - || selectedAudioStreamIndex >= audioStreams.size() - ? null : audioStreams.get(selectedAudioStreamIndex); + return audioStreams.get(selectedAudioStreamIndex); } } } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediaitem/StreamInfoTag.java b/app/src/main/java/org/schabi/newpipe/player/mediaitem/StreamInfoTag.java index e24a93615..6f4f551dc 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediaitem/StreamInfoTag.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediaitem/StreamInfoTag.java @@ -2,10 +2,8 @@ package org.schabi.newpipe.player.mediaitem; import com.google.android.exoplayer2.MediaItem; -import org.schabi.newpipe.extractor.stream.AudioStream; import org.schabi.newpipe.extractor.stream.StreamInfo; import org.schabi.newpipe.extractor.stream.StreamType; -import org.schabi.newpipe.extractor.stream.VideoStream; import org.schabi.newpipe.util.image.ImageStrategy; import java.util.Collections; @@ -31,7 +29,7 @@ public final class StreamInfoTag implements MediaItemTag { @Nullable private final Object extras; - private StreamInfoTag(@NonNull final StreamInfo streamInfo, + public StreamInfoTag(@NonNull final StreamInfo streamInfo, @Nullable final MediaItemTag.Quality quality, @Nullable final MediaItemTag.AudioTrack audioTrack, @Nullable final Object extras) { @@ -41,29 +39,6 @@ public final class StreamInfoTag implements MediaItemTag { this.extras = extras; } - public static StreamInfoTag of(@NonNull final StreamInfo streamInfo, - @NonNull final List sortedVideoStreams, - final int selectedVideoStreamIndex, - @NonNull final List audioStreams, - final int selectedAudioStreamIndex) { - final Quality quality = Quality.of(sortedVideoStreams, selectedVideoStreamIndex); - final AudioTrack audioTrack = - AudioTrack.of(audioStreams, selectedAudioStreamIndex); - return new StreamInfoTag(streamInfo, quality, audioTrack, null); - } - - public static StreamInfoTag of(@NonNull final StreamInfo streamInfo, - @NonNull final List audioStreams, - final int selectedAudioStreamIndex) { - final AudioTrack audioTrack = - AudioTrack.of(audioStreams, selectedAudioStreamIndex); - return new StreamInfoTag(streamInfo, null, audioTrack, null); - } - - public static StreamInfoTag of(@NonNull final StreamInfo streamInfo) { - return new StreamInfoTag(streamInfo, null, null, null); - } - @Override public List getErrors() { return Collections.emptyList(); diff --git a/app/src/main/java/org/schabi/newpipe/player/resolver/AudioPlaybackResolver.java b/app/src/main/java/org/schabi/newpipe/player/resolver/AudioPlaybackResolver.java index 9c0dc0edb..0540d3c6b 100644 --- a/app/src/main/java/org/schabi/newpipe/player/resolver/AudioPlaybackResolver.java +++ b/app/src/main/java/org/schabi/newpipe/player/resolver/AudioPlaybackResolver.java @@ -60,15 +60,22 @@ public class AudioPlaybackResolver implements PlaybackResolver { if (!audioStreams.isEmpty()) { final int audioIndex = ListHelper.getAudioFormatIndex(context, audioStreams, audioTrack); - stream = getStreamForIndex(audioIndex, audioStreams); - tag = StreamInfoTag.of(info, audioStreams, audioIndex); + assert audioIndex != -1; + final MediaItemTag.AudioTrack audio = + new MediaItemTag.AudioTrack(audioStreams, audioIndex); + tag = new StreamInfoTag(info, null, audio, null); + stream = audio.getSelectedAudioStream(); } else { final List videoStreams = getPlayableStreams(info.getVideoStreams(), info.getServiceId()); if (!videoStreams.isEmpty()) { - final int index = ListHelper.getDefaultResolutionIndex(context, videoStreams); - stream = getStreamForIndex(index, videoStreams); - tag = StreamInfoTag.of(info); + final int videoIndex = ListHelper.getDefaultResolutionIndex(context, videoStreams); + assert videoIndex != -1; + final MediaItemTag.Quality video = + new MediaItemTag.Quality(videoStreams, videoIndex); + // why are we not passing `video` as quality here? + tag = new StreamInfoTag(info, null, null, null); + stream = video.getSelectedVideoStream(); } else { return null; } @@ -83,19 +90,6 @@ public class AudioPlaybackResolver implements PlaybackResolver { } } - @Nullable - Stream getStreamForIndex(final int index, @NonNull final List streams) { - if (index >= 0 && index < streams.size()) { - return streams.get(index); - } - return null; - } - - @Nullable - public String getAudioTrack() { - return audioTrack; - } - public void setAudioTrack(@Nullable final String audioLanguage) { this.audioTrack = audioLanguage; } diff --git a/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java b/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java index de0d5be73..615bd616a 100644 --- a/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java +++ b/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java @@ -199,7 +199,7 @@ public interface PlaybackResolver { } try { - final StreamInfoTag tag = StreamInfoTag.of(info); + final StreamInfoTag tag = new StreamInfoTag(info, null, null, null); if (!info.getHlsUrl().isEmpty()) { return buildLiveMediaSource(dataSource, info.getHlsUrl(), C.CONTENT_TYPE_HLS, tag); } else if (!info.getDashMpdUrl().isEmpty()) { diff --git a/app/src/main/java/org/schabi/newpipe/player/resolver/VideoPlaybackResolver.java b/app/src/main/java/org/schabi/newpipe/player/resolver/VideoPlaybackResolver.java index 9c66764e9..a8b1d79d2 100644 --- a/app/src/main/java/org/schabi/newpipe/player/resolver/VideoPlaybackResolver.java +++ b/app/src/main/java/org/schabi/newpipe/player/resolver/VideoPlaybackResolver.java @@ -110,19 +110,23 @@ public class VideoPlaybackResolver implements PlaybackResolver { final int audioIndex = ListHelper.getAudioFormatIndex(context, audioStreamsList, audioTrack); - final MediaItemTag tag = - StreamInfoTag.of(info, videoStreamsList, videoIndex, audioStreamsList, audioIndex); - @Nullable final VideoStream video = tag.getMaybeQuality() - .map(MediaItemTag.Quality::getSelectedVideoStream) - .orElse(null); - @Nullable final AudioStream audio = tag.getMaybeAudioTrack() - .map(MediaItemTag.AudioTrack::getSelectedAudioStream) - .orElse(null); + + @Nullable MediaItemTag.Quality video = null; + @Nullable MediaItemTag.AudioTrack audio = null; + + if (videoIndex != -1) { + video = new MediaItemTag.Quality(videoStreamsList, videoIndex); + } + if (audioIndex != -1) { + audio = new MediaItemTag.AudioTrack(audioStreamsList, audioIndex); + } + final MediaItemTag tag = new StreamInfoTag(info, video, audio, null); if (video != null) { try { + final VideoStream stream = video.getSelectedVideoStream(); final MediaSource streamSource = PlaybackResolver.buildMediaSource( - dataSource, video, info, PlaybackResolver.cacheKeyOf(info, video), tag); + dataSource, stream, info, PlaybackResolver.cacheKeyOf(info, stream), tag); mediaSources.add(streamSource); } catch (final ResolverException e) { Log.e(TAG, "Unable to create video source", e); @@ -132,10 +136,14 @@ public class VideoPlaybackResolver implements PlaybackResolver { // Use the audio stream if there is no video stream, or // merge with audio stream in case if video does not contain audio - if (audio != null && (video == null || video.isVideoOnly() || audioTrack != null)) { + if (audio != null + && (video == null + || video.getSelectedVideoStream().isVideoOnly() + || audioTrack != null)) { try { + final AudioStream stream = audio.getSelectedAudioStream(); final MediaSource audioSource = PlaybackResolver.buildMediaSource( - dataSource, audio, info, PlaybackResolver.cacheKeyOf(info, audio), tag); + dataSource, stream, info, PlaybackResolver.cacheKeyOf(info, stream), tag); mediaSources.add(audioSource); streamSourceType = SourceType.VIDEO_WITH_SEPARATED_AUDIO; } catch (final ResolverException e) { diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java index b51aaa638..bd6ee46c2 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java @@ -1054,7 +1054,7 @@ public abstract class VideoPlayerUi extends PlayerUi implements SeekBar.OnSeekBa if (player.getCurrentMetadata() != null && player.getCurrentMetadata().getMaybeQuality().isEmpty() || (info.getVideoStreams().isEmpty() - && info.getVideoOnlyStreams().isEmpty())) { + && info.getVideoOnlyStreams().isEmpty())) { break; } diff --git a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java index 15385c0bf..917edd73a 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -116,6 +116,13 @@ public final class ListHelper { return groupedAudioStreams.indexOf(highestRanked); } + /** Get the index of the audio format to play in audioStreams. + * . + * @param context + * @param audioStreams + * @param trackId ??? + * @return index to play, or -1 if audioStreams is empty. + */ public static int getAudioFormatIndex(final Context context, @NonNull final List audioStreams, @Nullable final String trackId) { @@ -658,6 +665,7 @@ public final class ListHelper { return defaultMediaFormat; } + @Nullable private static MediaFormat getMediaFormatFromKey(@NonNull final Context context, @NonNull final String formatKey) { MediaFormat format = null;