From 76eb751438b244ac27822372c602e86e55c7f064 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Fri, 5 Jan 2024 19:36:09 +0100 Subject: [PATCH 01/14] Simplify selection of video stream I was trying to understand the logic here, and noticed the indirection via a QualityResolver interfaces is pretty unnecessary. Just branching directly makes the logic a lot easier to follow. The `-999` sentinel value is a bit dumb, but java does not recognize that videoIndex is always initialized. Nice side-effect, the `Resolver` interface was completely unused and can be dropped. --- .../org/schabi/newpipe/player/Player.java | 29 ++------ .../resolver/AudioPlaybackResolver.java | 1 - .../player/resolver/PlaybackResolver.java | 5 +- .../newpipe/player/resolver/Resolver.java | 9 --- .../resolver/VideoPlaybackResolver.java | 66 +++++++++++++------ 5 files changed, 53 insertions(+), 57 deletions(-) delete mode 100644 app/src/main/java/org/schabi/newpipe/player/resolver/Resolver.java diff --git a/app/src/main/java/org/schabi/newpipe/player/Player.java b/app/src/main/java/org/schabi/newpipe/player/Player.java index 49e72328e..8d5978c5f 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -42,8 +42,6 @@ import static org.schabi.newpipe.player.notification.NotificationConstants.ACTIO import static org.schabi.newpipe.player.notification.NotificationConstants.ACTION_RECREATE_NOTIFICATION; import static org.schabi.newpipe.player.notification.NotificationConstants.ACTION_REPEAT; import static org.schabi.newpipe.player.notification.NotificationConstants.ACTION_SHUFFLE; -import static org.schabi.newpipe.util.ListHelper.getPopupResolutionIndex; -import static org.schabi.newpipe.util.ListHelper.getResolutionIndex; import static org.schabi.newpipe.util.Localization.assureCorrectAppLanguage; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -116,7 +114,6 @@ import org.schabi.newpipe.player.ui.PlayerUiList; import org.schabi.newpipe.player.ui.PopupPlayerUi; import org.schabi.newpipe.player.ui.VideoPlayerUi; import org.schabi.newpipe.util.DependentPreferenceHelper; -import org.schabi.newpipe.util.ListHelper; import org.schabi.newpipe.util.NavigationHelper; import org.schabi.newpipe.util.image.PicassoHelper; import org.schabi.newpipe.util.SerializedCache; @@ -292,7 +289,7 @@ public final class Player implements PlaybackListener, Listener { context.getString( R.string.use_exoplayer_decoder_fallback_key), false)); - videoResolver = new VideoPlaybackResolver(context, dataSource, getQualityResolver()); + videoResolver = new VideoPlaybackResolver(context, dataSource); audioResolver = new AudioPlaybackResolver(context, dataSource); currentThumbnailTarget = getCurrentThumbnailTarget(); @@ -306,25 +303,6 @@ public final class Player implements PlaybackListener, Listener { new NotificationPlayerUi(this) ); } - - private VideoPlaybackResolver.QualityResolver getQualityResolver() { - return new VideoPlaybackResolver.QualityResolver() { - @Override - public int getDefaultResolutionIndex(final List sortedVideos) { - return videoPlayerSelected() - ? ListHelper.getDefaultResolutionIndex(context, sortedVideos) - : ListHelper.getPopupDefaultResolutionIndex(context, sortedVideos); - } - - @Override - public int getOverrideResolutionIndex(final List sortedVideos, - final String playbackQuality) { - return videoPlayerSelected() - ? getResolutionIndex(context, sortedVideos, playbackQuality) - : getPopupResolutionIndex(context, sortedVideos, playbackQuality); - } - }; - } //endregion @@ -1908,7 +1886,10 @@ public final class Player implements PlaybackListener, Listener { // Note that the video is not fetched when the app is in background because the video // renderer is fully disabled (see useVideoSource method), except for HLS streams // (see https://github.com/google/ExoPlayer/issues/9282). - return videoResolver.resolve(info); + return videoResolver.resolve(info, videoPlayerSelected() + ? VideoPlaybackResolver.SelectedPlayer.MAIN + : VideoPlaybackResolver.SelectedPlayer.POPUP + ); } public void disablePreloadingOfCurrentTrack() { 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 2d4404b2a..9c0dc0edb 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 @@ -45,7 +45,6 @@ public class AudioPlaybackResolver implements PlaybackResolver { * @param info of the stream * @return the audio source to use or null if none could be found */ - @Override @Nullable public MediaSource resolve(@NonNull final StreamInfo info) { final MediaSource liveSource = PlaybackResolver.maybeBuildLiveMediaSource(dataSource, info); 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 e204b8372..de0d5be73 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 @@ -46,11 +46,10 @@ import java.nio.charset.StandardCharsets; import java.util.Objects; /** - * This interface is just a shorthand for {@link Resolver} with {@link StreamInfo} as source and - * {@link MediaSource} as product. It contains many static methods that can be used by classes + * This interface contains many static methods that can be used by classes * implementing this interface, and nothing else. */ -public interface PlaybackResolver extends Resolver { +public interface PlaybackResolver { String TAG = PlaybackResolver.class.getSimpleName(); diff --git a/app/src/main/java/org/schabi/newpipe/player/resolver/Resolver.java b/app/src/main/java/org/schabi/newpipe/player/resolver/Resolver.java deleted file mode 100644 index a3e1db5b4..000000000 --- a/app/src/main/java/org/schabi/newpipe/player/resolver/Resolver.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.schabi.newpipe.player.resolver; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; - -public interface Resolver { - @Nullable - Product resolve(@NonNull Source source); -} 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 670c13934..496506454 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 @@ -39,8 +39,6 @@ public class VideoPlaybackResolver implements PlaybackResolver { private final Context context; @NonNull private final PlayerDataSource dataSource; - @NonNull - private final QualityResolver qualityResolver; private SourceType streamSourceType; @Nullable @@ -54,17 +52,23 @@ public class VideoPlaybackResolver implements PlaybackResolver { VIDEO_WITH_AUDIO_OR_AUDIO_ONLY } - public VideoPlaybackResolver(@NonNull final Context context, - @NonNull final PlayerDataSource dataSource, - @NonNull final QualityResolver qualityResolver) { - this.context = context; - this.dataSource = dataSource; - this.qualityResolver = qualityResolver; + /** + * Depending on the player we select different video streams. + */ + public enum SelectedPlayer { + MAIN, + POPUP + } + + public VideoPlaybackResolver(@NonNull final Context context, + @NonNull final PlayerDataSource dataSource) { + this.context = context; + this.dataSource = dataSource; } - @Override @Nullable - public MediaSource resolve(@NonNull final StreamInfo info) { + public MediaSource resolve(@NonNull final StreamInfo info, + @NonNull final SelectedPlayer selectedPlayer) { final MediaSource liveSource = PlaybackResolver.maybeBuildLiveMediaSource(dataSource, info); if (liveSource != null) { streamSourceType = SourceType.LIVE_STREAM; @@ -80,14 +84,42 @@ public class VideoPlaybackResolver implements PlaybackResolver { final List audioStreamsList = getFilteredAudioStreams(context, info.getAudioStreams()); - final int videoIndex; + int videoIndex = -999; if (videoStreamsList.isEmpty()) { videoIndex = -1; } else if (playbackQuality == null) { - videoIndex = qualityResolver.getDefaultResolutionIndex(videoStreamsList); + switch (selectedPlayer) { + case MAIN -> { + videoIndex = ListHelper.getDefaultResolutionIndex( + context, + videoStreamsList + ); + } + case POPUP -> { + videoIndex = ListHelper.getPopupDefaultResolutionIndex( + context, + videoStreamsList + ); + } + } + } else { - videoIndex = qualityResolver.getOverrideResolutionIndex(videoStreamsList, - getPlaybackQuality()); + switch (selectedPlayer) { + case MAIN -> { + videoIndex = ListHelper.getResolutionIndex( + context, + videoStreamsList, + getPlaybackQuality() + ); + } + case POPUP -> { + videoIndex = ListHelper.getPopupResolutionIndex( + context, + videoStreamsList, + getPlaybackQuality() + ); + } + } } final int audioIndex = @@ -195,10 +227,4 @@ public class VideoPlaybackResolver implements PlaybackResolver { public void setAudioTrack(@Nullable final String audioLanguage) { this.audioTrack = audioLanguage; } - - public interface QualityResolver { - int getDefaultResolutionIndex(List sortedVideos); - - int getOverrideResolutionIndex(List sortedVideos, String playbackQuality); - } } From 0148d65cab7f8eb0580cae36da0aae2f719c68c3 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 13:47:30 +0100 Subject: [PATCH 02/14] Inline getDefaultResolutionWithDefaultFormat --- .../resolver/VideoPlaybackResolver.java | 12 ++--- .../org/schabi/newpipe/util/ListHelper.java | 44 +++++++------------ 2 files changed, 22 insertions(+), 34 deletions(-) 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 496506454..4febccf18 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 @@ -106,17 +106,17 @@ public class VideoPlaybackResolver implements PlaybackResolver { } else { switch (selectedPlayer) { case MAIN -> { - videoIndex = ListHelper.getResolutionIndex( + videoIndex = ListHelper.getDefaultResolutionWithDefaultFormat( context, - videoStreamsList, - getPlaybackQuality() + getPlaybackQuality(), + videoStreamsList ); } case POPUP -> { - videoIndex = ListHelper.getPopupResolutionIndex( + videoIndex = ListHelper.getDefaultResolutionWithDefaultFormat( context, - videoStreamsList, - getPlaybackQuality() + getPlaybackQuality(), + videoStreamsList ); } } 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 71071d997..3017b153b 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -83,18 +83,6 @@ public final class ListHelper { return getDefaultResolutionWithDefaultFormat(context, defaultResolution, videoStreams); } - /** - * @param context Android app context - * @param videoStreams list of the video streams to check - * @param defaultResolution the default resolution to look for - * @return index of the video stream with the default index - * @see #getDefaultResolutionIndex(String, String, MediaFormat, List) - */ - public static int getResolutionIndex(final Context context, - final List videoStreams, - final String defaultResolution) { - return getDefaultResolutionWithDefaultFormat(context, defaultResolution, videoStreams); - } /** * @param context Android app context @@ -109,19 +97,6 @@ public final class ListHelper { return getDefaultResolutionWithDefaultFormat(context, defaultResolution, videoStreams); } - /** - * @param context Android app context - * @param videoStreams list of the video streams to check - * @param defaultResolution the default resolution to look for - * @return index of the video stream with the default index - * @see #getDefaultResolutionIndex(String, String, MediaFormat, List) - */ - public static int getPopupResolutionIndex(final Context context, - final List videoStreams, - final String defaultResolution) { - return getDefaultResolutionWithDefaultFormat(context, defaultResolution, videoStreams); - } - public static int getDefaultAudioFormat(final Context context, final List audioStreams) { return getAudioIndexByHighestRank(audioStreams, @@ -634,7 +609,7 @@ public final class ListHelper { * @param videoStreams the list of video streams to check * @return the index of the preferred video stream */ - private static int getDefaultResolutionWithDefaultFormat(@NonNull final Context context, + public static int getDefaultResolutionWithDefaultFormat(@NonNull final Context context, final String defaultResolution, final List videoStreams) { final MediaFormat defaultFormat = getDefaultFormat(context, @@ -680,6 +655,14 @@ public final class ListHelper { return format; } + /** #Comparator for two resolution strings. + * + * See {@link #sortStreamList} for ordering. + * + * @param r1 first + * @param r2 second + * @return comparison int + */ private static int compareVideoStreamResolution(@NonNull final String r1, @NonNull final String r2) { try { @@ -696,12 +679,17 @@ public final class ListHelper { } } + /** Does the application have a maximum resolution set? + * + * @param context App context + * @return whether a max resolution is set + */ static boolean isLimitingDataUsage(@NonNull final Context context) { return getResolutionLimit(context) != null; } /** - * The maximum resolution allowed. + * The maximum resolution allowed by application settings. * * @param context App context * @return maximum resolution allowed or null if there is no maximum @@ -720,7 +708,7 @@ public final class ListHelper { } /** - * The current network is metered (like mobile data)? + * Is the current network metered (like mobile data)? * * @param context App context * @return {@code true} if connected to a metered network From be4e0cb3dc555512a7bcbc075baf7a469f8bc94f Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 13:50:20 +0100 Subject: [PATCH 03/14] Remove now unnecessary switch --- .../resolver/VideoPlaybackResolver.java | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) 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 4febccf18..14c1f9f1e 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 @@ -102,24 +102,12 @@ public class VideoPlaybackResolver implements PlaybackResolver { ); } } - } else { - switch (selectedPlayer) { - case MAIN -> { - videoIndex = ListHelper.getDefaultResolutionWithDefaultFormat( - context, - getPlaybackQuality(), - videoStreamsList - ); - } - case POPUP -> { - videoIndex = ListHelper.getDefaultResolutionWithDefaultFormat( - context, - getPlaybackQuality(), - videoStreamsList - ); - } - } + videoIndex = ListHelper.getDefaultResolutionWithDefaultFormat( + context, + getPlaybackQuality(), + videoStreamsList + ); } final int audioIndex = From e5ac6824e8117fb4c2b9185da9891b160e7b8e3b Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 14:20:07 +0100 Subject: [PATCH 04/14] Remove null check on getDefaultSharedPreferences The function never returns `null`. --- app/src/main/java/org/schabi/newpipe/util/ListHelper.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 3017b153b..0acd476fe 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -371,9 +371,8 @@ public final class ListHelper { PreferenceManager.getDefaultSharedPreferences(context); // Load the preferred resolution otherwise the best available - String resolution = preferences != null - ? preferences.getString(context.getString(key), context.getString(value)) - : context.getString(R.string.best_resolution_key); + String resolution = + preferences.getString(context.getString(key), context.getString(value)); final String maxResolution = getResolutionLimit(context); if (maxResolution != null From 71d88d0bc0c6023edf5ca5e2ed11bb755f667a21 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 15:12:43 +0100 Subject: [PATCH 05/14] ListHelper: rename functions & variables and add documentation This should make the purpose of these functions more clear. --- .../org/schabi/newpipe/util/ListHelper.java | 65 ++++++++++++------- .../newpipe/util/SecondaryStreamHelper.java | 4 +- 2 files changed, 45 insertions(+), 24 deletions(-) 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 0acd476fe..978048a84 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -78,7 +78,7 @@ public final class ListHelper { */ public static int getDefaultResolutionIndex(final Context context, final List videoStreams) { - final String defaultResolution = computeDefaultResolution(context, + final String defaultResolution = getPreferredResolutionOrCurrentLimit(context, R.string.default_resolution_key, R.string.default_resolution_value); return getDefaultResolutionWithDefaultFormat(context, defaultResolution, videoStreams); } @@ -92,7 +92,7 @@ public final class ListHelper { */ public static int getPopupDefaultResolutionIndex(final Context context, final List videoStreams) { - final String defaultResolution = computeDefaultResolution(context, + final String defaultResolution = getPreferredResolutionOrCurrentLimit(context, R.string.default_popup_resolution_key, R.string.default_popup_resolution_value); return getDefaultResolutionWithDefaultFormat(context, defaultResolution, videoStreams); } @@ -365,22 +365,42 @@ public final class ListHelper { .collect(Collectors.toList()); } - private static String computeDefaultResolution(@NonNull final Context context, final int key, - final int value) { + /** Lookup the preferred resolution and the current resolution limit. + * + * @param context App context + * @param defaultResolutionKey The defaultResolution preference key + * @param defaultResolutionDefaultValue Default resolution if key does not exist + * @return The smaller resolution of either the preference or the current limit. + */ + private static String getPreferredResolutionOrCurrentLimit( + @NonNull final Context context, + final int defaultResolutionKey, + final int defaultResolutionDefaultValue + ) { final SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context); // Load the preferred resolution otherwise the best available - String resolution = - preferences.getString(context.getString(key), context.getString(value)); + final String preferredResolution = preferences.getString( + context.getString(defaultResolutionKey), + context.getString(defaultResolutionDefaultValue) + ); - final String maxResolution = getResolutionLimit(context); - if (maxResolution != null - && (resolution.equals(context.getString(R.string.best_resolution_key)) - || compareVideoStreamResolution(maxResolution, resolution) < 1)) { - resolution = maxResolution; + // clamp to the currently maximum allowed resolution + final String result; + final String resolutionLimit = getCurrentResolutionLimit(context); + if (resolutionLimit != null + && ( + // if the preference is best_resolution + preferredResolution.equals(context.getString(R.string.best_resolution_key)) + // or the preference is higher than the current max allowed resolution + || compareVideoStreamResolution(resolutionLimit, preferredResolution) < 1 + )) { + result = resolutionLimit; + } else { + result = preferredResolution; } - return resolution; + return result; } /** @@ -601,7 +621,7 @@ public final class ListHelper { /** * Fetches the desired resolution or returns the default if it is not found. - * The resolution will be reduced if video chocking is active. + * The resolution will be reduced if video choking is active. * * @param context Android app context * @param defaultResolution the default resolution @@ -683,27 +703,28 @@ public final class ListHelper { * @param context App context * @return whether a max resolution is set */ - static boolean isLimitingDataUsage(@NonNull final Context context) { - return getResolutionLimit(context) != null; + static boolean isCurrentlyLimitingDataUsage(@NonNull final Context context) { + return getCurrentResolutionLimit(context) != null; } /** - * The maximum resolution allowed by application settings. + * The maximum current resolution allowed by application settings. + * Takes into account whether we are on a metered network. * * @param context App context - * @return maximum resolution allowed or null if there is no maximum + * @return current maximum resolution allowed or null if there is no maximum */ - private static String getResolutionLimit(@NonNull final Context context) { - String resolutionLimit = null; + private static String getCurrentResolutionLimit(@NonNull final Context context) { + String currentResolutionLimit = null; if (isMeteredNetwork(context)) { final SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context); final String defValue = context.getString(R.string.limit_data_usage_none_key); final String value = preferences.getString( context.getString(R.string.limit_mobile_data_usage_key), defValue); - resolutionLimit = defValue.equals(value) ? null : value; + currentResolutionLimit = defValue.equals(value) ? null : value; } - return resolutionLimit; + return currentResolutionLimit; } /** @@ -734,7 +755,7 @@ public final class ListHelper { final @NonNull Context context) { final MediaFormat defaultFormat = getDefaultFormat(context, R.string.default_audio_format_key, R.string.default_audio_format_value); - return getAudioFormatComparator(defaultFormat, isLimitingDataUsage(context)); + return getAudioFormatComparator(defaultFormat, isCurrentlyLimitingDataUsage(context)); } /** diff --git a/app/src/main/java/org/schabi/newpipe/util/SecondaryStreamHelper.java b/app/src/main/java/org/schabi/newpipe/util/SecondaryStreamHelper.java index 75d9a3892..c3677055e 100644 --- a/app/src/main/java/org/schabi/newpipe/util/SecondaryStreamHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/SecondaryStreamHelper.java @@ -31,7 +31,7 @@ public class SecondaryStreamHelper { * Finds an audio stream compatible with the provided video-only stream, so that the two streams * can be combined in a single file by the downloader. If there are multiple available audio * streams, chooses either the highest or the lowest quality one based on - * {@link ListHelper#isLimitingDataUsage(Context)}. + * {@link ListHelper#isCurrentlyLimitingDataUsage(Context)}. * * @param context Android context * @param audioStreams list of audio streams @@ -56,7 +56,7 @@ public class SecondaryStreamHelper { } final boolean m4v = mediaFormat == MediaFormat.MPEG_4; - final boolean isLimitingDataUsage = ListHelper.isLimitingDataUsage(context); + final boolean isLimitingDataUsage = ListHelper.isCurrentlyLimitingDataUsage(context); Comparator comparator = ListHelper.getAudioFormatComparator( m4v ? MediaFormat.M4A : MediaFormat.WEBMA, isLimitingDataUsage); From 64c68cf2f12d36b617ed864b18bcc97d3dddbfe8 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 17:37:17 +0100 Subject: [PATCH 06/14] VideoPlaybackResolver: implicit -1 videoIndex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All functions that set videoIndex return `-1` if the list is empty, so we don’t have to check manually for that case. I’m somewhat more questioning why `StreamInfoTag.of` allows the index to be out-of-bounds in the constructor, that should be an error. --- .../resolver/VideoPlaybackResolver.java | 4 +--- .../org/schabi/newpipe/util/ListHelper.java | 22 +++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) 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 14c1f9f1e..9c66764e9 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 @@ -85,9 +85,7 @@ public class VideoPlaybackResolver implements PlaybackResolver { getFilteredAudioStreams(context, info.getAudioStreams()); int videoIndex = -999; - if (videoStreamsList.isEmpty()) { - videoIndex = -1; - } else if (playbackQuality == null) { + if (playbackQuality == null) { switch (selectedPlayer) { case MAIN -> { videoIndex = ListHelper.getDefaultResolutionIndex( 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 978048a84..365db9b95 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -73,7 +73,7 @@ public final class ListHelper { /** * @param context Android app context * @param videoStreams list of the video streams to check - * @return index of the video stream with the default index + * @return index of the video stream with the default index, -1 if `videoStreams` is empty * @see #getDefaultResolutionIndex(String, String, MediaFormat, List) */ public static int getDefaultResolutionIndex(final Context context, @@ -87,7 +87,7 @@ public final class ListHelper { /** * @param context Android app context * @param videoStreams list of the video streams to check - * @return index of the video stream with the default index + * @return index of the video stream with the default index, -1 if `videoStreams` is empty * @see #getDefaultResolutionIndex(String, String, MediaFormat, List) */ public static int getPopupDefaultResolutionIndex(final Context context, @@ -408,17 +408,19 @@ public final class ListHelper { * on the parameters defaultResolution and defaultFormat. * * @param defaultResolution the default resolution to look for + * (a resolution string or `bestResolutionKey`). * @param bestResolutionKey key of the best resolution * @param defaultFormat the default format to look for * @param videoStreams a mutable list of the video streams to check (it will be sorted in * place) - * @return index of the default resolution&format in the sorted videoStreams + * @return index of the default resolution&format in the sorted videoStreams, + * -1 if `videoStreams` is empty */ static int getDefaultResolutionIndex(final String defaultResolution, final String bestResolutionKey, final MediaFormat defaultFormat, - @Nullable final List videoStreams) { - if (videoStreams == null || videoStreams.isEmpty()) { + @NonNull final List videoStreams) { + if (videoStreams.isEmpty()) { return -1; } @@ -626,11 +628,13 @@ public final class ListHelper { * @param context Android app context * @param defaultResolution the default resolution * @param videoStreams the list of video streams to check - * @return the index of the preferred video stream + * @return the index of the preferred video stream, -1 if `videoStreams` is empty */ - public static int getDefaultResolutionWithDefaultFormat(@NonNull final Context context, - final String defaultResolution, - final List videoStreams) { + public static int getDefaultResolutionWithDefaultFormat( + @NonNull final Context context, + final String defaultResolution, + @NonNull final List videoStreams + ) { final MediaFormat defaultFormat = getDefaultFormat(context, R.string.default_video_format_key, R.string.default_video_format_value); return getDefaultResolutionIndex(defaultResolution, From e0cbddfe5476faea758d34834d3bb37c0018b863 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 18:33:48 +0100 Subject: [PATCH 07/14] getAudioIndexByHighestRank: simplify & make list NonNull --- .../org/schabi/newpipe/util/ListHelper.java | 19 ++++++++----------- .../newpipe/util/StreamItemAdapter.java | 3 ++- .../schabi/newpipe/util/ListHelperTest.java | 2 -- 3 files changed, 10 insertions(+), 14 deletions(-) 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 365db9b95..15385c0bf 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -98,7 +98,7 @@ public final class ListHelper { } public static int getDefaultAudioFormat(final Context context, - final List audioStreams) { + @NonNull final List audioStreams) { return getAudioIndexByHighestRank(audioStreams, getAudioTrackComparator(context).thenComparing(getAudioFormatComparator(context))); } @@ -117,7 +117,7 @@ public final class ListHelper { } public static int getAudioFormatIndex(final Context context, - final List audioStreams, + @NonNull final List audioStreams, @Nullable final String trackId) { if (trackId != null) { for (int i = 0; i < audioStreams.size(); i++) { @@ -263,6 +263,7 @@ public final class ListHelper { * @param audioStreams the list of audio streams * @return the sorted, filtered list */ + @NonNull public static List getFilteredAudioStreams( @NonNull final Context context, @Nullable final List audioStreams) { @@ -535,16 +536,12 @@ public final class ListHelper { * @param comparator The comparator used for determining the max/best/highest ranked value * @return Index of audio stream that produces the highest ranked result or -1 if not found */ - static int getAudioIndexByHighestRank(@Nullable final List audioStreams, + static int getAudioIndexByHighestRank(@NonNull final List audioStreams, final Comparator comparator) { - if (audioStreams == null || audioStreams.isEmpty()) { - return -1; - } - - final AudioStream highestRankedAudioStream = audioStreams.stream() - .max(comparator).orElse(null); - - return audioStreams.indexOf(highestRankedAudioStream); + return audioStreams.stream() + .max(comparator) + .map(audioStreams::indexOf) + .orElse(-1); } /** diff --git a/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java b/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java index 2eeb14b1b..260a54db8 100644 --- a/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java +++ b/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java @@ -230,7 +230,7 @@ public class StreamItemAdapter extends BaseA new StreamInfoWrapper<>(Collections.emptyList(), null); private static final int SIZE_UNSET = -2; - private final List streamsList; + @NonNull private final List streamsList; private final long[] streamSizes; private final MediaFormat[] streamFormats; private final String unknownSize; @@ -432,6 +432,7 @@ public class StreamItemAdapter extends BaseA return (StreamInfoWrapper) EMPTY; } + @NonNull public List getStreamsList() { return streamsList; } diff --git a/app/src/test/java/org/schabi/newpipe/util/ListHelperTest.java b/app/src/test/java/org/schabi/newpipe/util/ListHelperTest.java index c7c36eadc..62151657c 100644 --- a/app/src/test/java/org/schabi/newpipe/util/ListHelperTest.java +++ b/app/src/test/java/org/schabi/newpipe/util/ListHelperTest.java @@ -279,7 +279,6 @@ public class ListHelperTest { @Test public void getHighestQualityAudioNull() { final Comparator cmp = ListHelper.getAudioFormatComparator(null, false); - assertEquals(-1, ListHelper.getAudioIndexByHighestRank(null, cmp)); assertEquals(-1, ListHelper.getAudioIndexByHighestRank(new ArrayList<>(), cmp)); } @@ -356,7 +355,6 @@ public class ListHelperTest { @Test public void getLowestQualityAudioNull() { final Comparator cmp = ListHelper.getAudioFormatComparator(null, false); - assertEquals(-1, ListHelper.getAudioIndexByHighestRank(null, cmp)); assertEquals(-1, ListHelper.getAudioIndexByHighestRank(new ArrayList<>(), cmp)); } From 743e16a8a7d285d1e08ec01c67cc762870329659 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 19:05:11 +0100 Subject: [PATCH 08/14] 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; From 24ac6d57f34713bcaa60161bcebfc1942c528f0f Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 21:11:48 +0100 Subject: [PATCH 09/14] getFilteredAudioStreams: make trackId more obvious and document bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why would you serialize objects to avoid a null check, that’s just weird. --- .../org/schabi/newpipe/util/ListHelper.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) 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 917edd73a..1ebca9040 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -266,6 +266,13 @@ public final class ListHelper { * Filter the list of audio streams and return a list with the preferred stream for * each audio track. Streams are sorted with the preferred language in the first position. * + * The following formats cannot be played and are thus skipped: + * + *
    + *
  • {@literal DeliveryMethod.TORRENT} + *
  • both {@literal DeliveryMethod.HLS} AND {@literal MediaFormat.OPUS}
    • + *
+ * * @param context the context to search for the track to give preference * @param audioStreams the list of audio streams * @return the sorted, filtered list @@ -283,13 +290,20 @@ public final class ListHelper { final Comparator cmp = getAudioFormatComparator(context); for (final AudioStream stream : audioStreams) { + + // XXX: Why are these skipped exactly? if (stream.getDeliveryMethod() == DeliveryMethod.TORRENT || (stream.getDeliveryMethod() == DeliveryMethod.HLS - && stream.getFormat() == MediaFormat.OPUS)) { + && stream.getFormat() == MediaFormat.OPUS)) { continue; } - final String trackId = Objects.toString(stream.getAudioTrackId(), ""); + // TODO: since media.ccc.de does not have a trackId, + // only the first audio is ever returned here … + String trackId = stream.getAudioTrackId(); + if (trackId == null) { + trackId = ""; + } final AudioStream presentStream = collectedStreams.get(trackId); if (presentStream == null || cmp.compare(stream, presentStream) > 0) { From 982d23674b02917a63f88fda17b37473c78fa29c Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 23:57:42 +0100 Subject: [PATCH 10/14] getFilteredAudioStreams: make list nonNull --- .../newpipe/player/resolver/AudioPlaybackResolver.java | 8 +++++++- .../newpipe/player/resolver/VideoPlaybackResolver.java | 8 +++++++- app/src/main/java/org/schabi/newpipe/util/ListHelper.java | 5 +---- 3 files changed, 15 insertions(+), 6 deletions(-) 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 0540d3c6b..6730fc741 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 @@ -20,7 +20,9 @@ import org.schabi.newpipe.player.mediaitem.MediaItemTag; import org.schabi.newpipe.player.mediaitem.StreamInfoTag; import org.schabi.newpipe.util.ListHelper; +import java.util.Collections; import java.util.List; +import java.util.Objects; public class AudioPlaybackResolver implements PlaybackResolver { private static final String TAG = AudioPlaybackResolver.class.getSimpleName(); @@ -53,7 +55,11 @@ public class AudioPlaybackResolver implements PlaybackResolver { } final List audioStreams = - getFilteredAudioStreams(context, info.getAudioStreams()); + getFilteredAudioStreams( + context, + // TODO: getAudioStreams should be @NonNull + Objects.requireNonNullElse(info.getAudioStreams(), Collections.emptyList()) + ); final Stream stream; final MediaItemTag tag; 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 a8b1d79d2..64b14d40c 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 @@ -24,7 +24,9 @@ import org.schabi.newpipe.player.mediaitem.StreamInfoTag; import org.schabi.newpipe.util.ListHelper; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Optional; import static com.google.android.exoplayer2.C.TIME_UNSET; @@ -82,7 +84,11 @@ public class VideoPlaybackResolver implements PlaybackResolver { getPlayableStreams(info.getVideoStreams(), info.getServiceId()), getPlayableStreams(info.getVideoOnlyStreams(), info.getServiceId()), false, true); final List audioStreamsList = - getFilteredAudioStreams(context, info.getAudioStreams()); + getFilteredAudioStreams( + context, + // TODO: getAudioStreams should be @NonNull + Objects.requireNonNullElse(info.getAudioStreams(), Collections.emptyList()) + ); int videoIndex = -999; if (playbackQuality == null) { 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 1ebca9040..69d2f2959 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -280,10 +280,7 @@ public final class ListHelper { @NonNull public static List getFilteredAudioStreams( @NonNull final Context context, - @Nullable final List audioStreams) { - if (audioStreams == null) { - return Collections.emptyList(); - } + @NonNull final List audioStreams) { final HashMap collectedStreams = new HashMap<>(); From 2f529919ab2d99de97dba8d3c98f87e495dd9b57 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Jan 2024 23:58:25 +0100 Subject: [PATCH 11/14] getFilteredAudioStreams: reflow & more docs --- .../resolver/VideoPlaybackResolver.java | 3 +-- .../org/schabi/newpipe/util/ListHelper.java | 22 +++++++++++-------- 2 files changed, 14 insertions(+), 11 deletions(-) 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 64b14d40c..5c2b89027 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 @@ -77,8 +77,6 @@ public class VideoPlaybackResolver implements PlaybackResolver { return liveSource; } - final List mediaSources = new ArrayList<>(); - // Create video stream source final List videoStreamsList = ListHelper.getSortedStreamVideosList(context, getPlayableStreams(info.getVideoStreams(), info.getServiceId()), @@ -128,6 +126,7 @@ public class VideoPlaybackResolver implements PlaybackResolver { } final MediaItemTag tag = new StreamInfoTag(info, video, audio, null); + final List mediaSources = new ArrayList<>(); if (video != null) { try { final VideoStream stream = video.getSelectedVideoStream(); 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 69d2f2959..523c68d8b 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -284,14 +284,14 @@ public final class ListHelper { final HashMap collectedStreams = new HashMap<>(); - final Comparator cmp = getAudioFormatComparator(context); - for (final AudioStream stream : audioStreams) { - // XXX: Why are these skipped exactly? - if (stream.getDeliveryMethod() == DeliveryMethod.TORRENT - || (stream.getDeliveryMethod() == DeliveryMethod.HLS - && stream.getFormat() == MediaFormat.OPUS)) { + if (// we can’t play torrents + stream.getDeliveryMethod() == DeliveryMethod.TORRENT + // This format is not supported by ExoPlayer when returned as HLS streams, + // so we can't play streams using this format and this delivery method. + || (stream.getDeliveryMethod() == DeliveryMethod.HLS + && stream.getFormat() == MediaFormat.OPUS)) { continue; } @@ -303,7 +303,8 @@ public final class ListHelper { } final AudioStream presentStream = collectedStreams.get(trackId); - if (presentStream == null || cmp.compare(stream, presentStream) > 0) { + if (presentStream == null + || getAudioFormatComparator(context).compare(stream, presentStream) > 0) { collectedStreams.put(trackId, stream); } } @@ -903,8 +904,11 @@ public final class ListHelper { @NonNull final Context context) { final Locale appLoc = Localization.getAppLocale(context); - return Comparator.comparing(AudioStream::getAudioLocale, Comparator.nullsLast( - Comparator.comparing(locale -> locale.getDisplayName(appLoc)))) + return Comparator.comparing( + AudioStream::getAudioLocale, + Comparator.nullsLast( + Comparator.comparing(locale -> locale.getDisplayName(appLoc)) + )) .thenComparing(AudioStream::getAudioTrackType); } } From 189c35e89fcf5392377e7c4d5d5209916b93f75b Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 7 Jan 2024 01:09:45 +0100 Subject: [PATCH 12/14] PlaybackResolver: annotate the uses of ItagResolver --- .../org/schabi/newpipe/player/resolver/PlaybackResolver.java | 3 +++ 1 file changed, 3 insertions(+) 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 615bd616a..461979e70 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 @@ -415,6 +415,7 @@ public interface PlaybackResolver { // (which is the last segment of the stream) try { + // We know that itagItem has to be set, because it’s youtube-specific final ItagItem itagItem = Objects.requireNonNull(stream.getItagItem()); final String manifestString = YoutubePostLiveStreamDvrDashManifestCreator .fromPostLiveStreamDvrStreamingUrl(stream.getContent(), @@ -448,6 +449,7 @@ public interface PlaybackResolver { try { final String manifestString = YoutubeProgressiveDashManifestCreator .fromProgressiveStreamingUrl(stream.getContent(), + // We know that itagItem has to be set, because it’s youtube-specific Objects.requireNonNull(stream.getItagItem()), streamInfo.getDuration()); return buildYoutubeManualDashMediaSource(dataSource, @@ -475,6 +477,7 @@ public interface PlaybackResolver { try { final String manifestString = YoutubeOtfDashManifestCreator .fromOtfStreamingUrl(stream.getContent(), + // We know that itagItem has to be set, because it’s youtube-specific Objects.requireNonNull(stream.getItagItem()), streamInfo.getDuration()); return buildYoutubeManualDashMediaSource(dataSource, From e242ad42a8624fb3703d91f6ee423b7ff376df0d Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 7 Jan 2024 19:35:18 +0100 Subject: [PATCH 13/14] getFilteredAudioStreams: Use language and tracktype for grouping Instead of the `trackId`, which is only ever set for the `youtube` backend, we use the audio stream language and tracktype. These are `null` for youtube videos without language selector, leading to a single audio stream being selected. For media.ccc.de, the language on audio tracks is always set correctly, meaning for a video with German and English tracks we get two groups, which are then sorted according to the preferences of users. I verified that youtube still works by playing https://www.youtube.com/watch?v=8bDRVP9xSfc and selecting the different audio streams, going to background etc. and also tried with a video without multiple audio tracks. For media.ccc.de, it will select the right audio track as well. Unfortunately, media.ccc.de returns videos with multiple audio track muxed into the video itself, which is still buggy because the wrong track is selected by default. This is gonna be fixed/worked around in the next commit. --- .../org/schabi/newpipe/util/ListHelper.java | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) 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 523c68d8b..2b8f982ce 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -6,6 +6,7 @@ import android.content.Context; import android.content.SharedPreferences; import android.content.res.Resources; import android.net.ConnectivityManager; +import android.util.Pair; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -282,41 +283,40 @@ public final class ListHelper { @NonNull final Context context, @NonNull final List audioStreams) { - final HashMap collectedStreams = new HashMap<>(); - - for (final AudioStream stream : audioStreams) { - - if (// we can’t play torrents - stream.getDeliveryMethod() == DeliveryMethod.TORRENT - // This format is not supported by ExoPlayer when returned as HLS streams, - // so we can't play streams using this format and this delivery method. - || (stream.getDeliveryMethod() == DeliveryMethod.HLS - && stream.getFormat() == MediaFormat.OPUS)) { - continue; - } - - // TODO: since media.ccc.de does not have a trackId, - // only the first audio is ever returned here … - String trackId = stream.getAudioTrackId(); - if (trackId == null) { - trackId = ""; - } - - final AudioStream presentStream = collectedStreams.get(trackId); - if (presentStream == null - || getAudioFormatComparator(context).compare(stream, presentStream) > 0) { - collectedStreams.put(trackId, stream); - } - } - - // Filter unknown audio tracks if there are multiple tracks - if (collectedStreams.size() > 1) { - collectedStreams.remove(""); - } - - // Sort collected streams by name - return collectedStreams.values().stream().sorted(getAudioTrackNameComparator(context)) + final List result = audioStreams + .stream() + // remove torrents we can’t play + .filter(stream -> + !( + // we can’t play torrents + stream.getDeliveryMethod() == DeliveryMethod.TORRENT + // This format is not supported by ExoPlayer when returned as HLS streams, + // so we can't play streams using this format and this delivery method. + || (stream.getDeliveryMethod() == DeliveryMethod.HLS + && stream.getFormat() == MediaFormat.OPUS)) + ) + .collect(Collectors.groupingBy( + stream -> + // Streams grouped by their locale+audiotype + // (e.g. en+original, fr+dubbed) + new Pair( + stream.getAudioLocale(), + stream.getAudioTrackType() + ), + // from each list of grouped streams, + // we select the one that has the most fitting audio type & quality + Collectors.maxBy(getAudioFormatComparator(context)) + )) + .entrySet() + .stream() + // get streams and remove bins that don’t contain streams + .flatMap(e -> e.getValue().stream()) + // sort the preferred audio stream last + .sorted(getAudioTrackComparator(context)) .collect(Collectors.toList()); + + return result; + } /** From d134219da5eb7d0adcbedc67229f015486e1bf4f Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 7 Jan 2024 19:57:21 +0100 Subject: [PATCH 14/14] resolver: document the `audioTrack` setters better --- .../player/resolver/AudioPlaybackResolver.java | 8 ++++++-- .../player/resolver/VideoPlaybackResolver.java | 14 +++++++------- .../java/org/schabi/newpipe/util/ListHelper.java | 3 ++- 3 files changed, 15 insertions(+), 10 deletions(-) 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 6730fc741..6b2e3d27c 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 @@ -96,7 +96,11 @@ public class AudioPlaybackResolver implements PlaybackResolver { } } - public void setAudioTrack(@Nullable final String audioLanguage) { - this.audioTrack = audioLanguage; + /** Set audio track to be used the next time {@link #resolve(StreamInfo)} is called. + * + * @param audioTrack the {@link AudioStream} audioTrackId that should be selected on resolve + */ + public void setAudioTrack(@Nullable final String audioTrack) { + this.audioTrack = audioTrack; } } 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 5c2b89027..312bac7e9 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 @@ -218,12 +218,12 @@ public class VideoPlaybackResolver implements PlaybackResolver { this.playbackQuality = playbackQuality; } - @Nullable - public String getAudioTrack() { - return audioTrack; - } - - public void setAudioTrack(@Nullable final String audioLanguage) { - this.audioTrack = audioLanguage; + /** Set audio track to be used the next time {@link #resolve(StreamInfo, SelectedPlayer)} + * is called. + * + * @param audioTrack the {@link AudioStream} audioTrackId that should be selected on resolve + */ + public void setAudioTrack(@Nullable final String audioTrack) { + this.audioTrack = audioTrack; } } 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 2b8f982ce..7ac5adb57 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -121,12 +121,13 @@ public final class ListHelper { * . * @param context * @param audioStreams - * @param trackId ??? + * @param trackId Try to find this #AudioStream.getAudioTrackId in the audioStreams & select it * @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) { + // if we were given a trackId, try to select that before going to the defaults. if (trackId != null) { for (int i = 0; i < audioStreams.size(); i++) { final AudioStream s = audioStreams.get(i);