Quality/AudioTrack: ensure correct indices at construction

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.
This commit is contained in:
Profpatsch 2024-01-06 19:05:11 +01:00
parent e0cbddfe54
commit 743e16a8a7
7 changed files with 74 additions and 77 deletions

View File

@ -106,19 +106,28 @@ public interface MediaItemTag {
final class Quality {
@NonNull
private final List<VideoStream> sortedVideoStreams;
/** Invariant: Index exists in sortedVideoStreams. */
private final int selectedVideoStreamIndex;
private Quality(@NonNull final List<VideoStream> 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<VideoStream> 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<VideoStream> sortedVideoStreams,
final int selectedVideoStreamIndex) {
return new Quality(sortedVideoStreams, selectedVideoStreamIndex);
}
@NonNull
public List<VideoStream> 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<AudioStream> audioStreams;
/** Invariant: Index exists in audioStreams. */
private final int selectedAudioStreamIndex;
private AudioTrack(@NonNull final List<AudioStream> 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<AudioStream> 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<AudioStream> audioStreams,
final int selectedAudioStreamIndex) {
return new AudioTrack(audioStreams, selectedAudioStreamIndex);
}
@NonNull
public List<AudioStream> 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);
}
}
}

View File

@ -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<VideoStream> sortedVideoStreams,
final int selectedVideoStreamIndex,
@NonNull final List<AudioStream> 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<AudioStream> 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<Exception> getErrors() {
return Collections.emptyList();

View File

@ -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<VideoStream> 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<? extends Stream> 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;
}

View File

@ -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()) {

View File

@ -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) {

View File

@ -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;
}

View File

@ -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<AudioStream> 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;