From 5ab3a4a9e0fe636590277f43965b425c23f26cb0 Mon Sep 17 00:00:00 2001 From: AudricV <74829229+AudricV@users.noreply.github.com> Date: Wed, 28 Jun 2023 21:00:40 +0200 Subject: [PATCH] Refactor Player.useVideoSource logic and improve its comments - don't check for isAudioOnly == !videoEnabled, as this prevents enabling again video and text tracks renderers in some cases; - when reloading play queue manager if that's needed, disable or enable video and text tracks renderers, as they may need to be enabled again in some cases like starting a video in main player, opening play queue, switching to background player on it and switching back to main player; - disable or enable video renderers also for streams with AUDIO_STREAM StreamType, as doing so doesn't raise any issue and simplifies code; - reword and move some comments to make them easier to understand. --- .../org/schabi/newpipe/player/Player.java | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) 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 89bdd9d69..1a8283590 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -2065,43 +2065,36 @@ public final class Player implements PlaybackListener, Listener { } public void useVideoSource(final boolean videoEnabled) { - if (playQueue == null || isAudioOnly == !videoEnabled || audioPlayerSelected()) { + if (playQueue == null || audioPlayerSelected()) { return; } isAudioOnly = !videoEnabled; - // The current metadata may be null sometimes (for e.g. when using an unstable connection - // in livestreams) so we will be not able to execute the block below. - // Reload the play queue manager in this case, which is the behavior when we don't know the - // index of the video renderer or playQueueManagerReloadingNeeded returns true. getCurrentStreamInfo().ifPresentOrElse(info -> { - // In the case we don't know the source type, fallback to the one with video with audio - // or audio-only source. + // In case we don't know the source type, fall back to either video-with-audio, or + // audio-only source type final SourceType sourceType = videoResolver.getStreamSourceType() .orElse(SourceType.VIDEO_WITH_AUDIO_OR_AUDIO_ONLY); if (playQueueManagerReloadingNeeded(sourceType, info, getVideoRendererIndex())) { reloadPlayQueueManager(); - } else { - if (StreamTypeUtil.isAudio(info.getStreamType())) { - // Nothing to do more than setting the recovery position - setRecovery(); - return; - } - - final var parametersBuilder = trackSelector.buildUponParameters(); - - // Enable/disable the video track and the ability to select subtitles - parametersBuilder.setTrackTypeDisabled(C.TRACK_TYPE_TEXT, !videoEnabled); - parametersBuilder.setTrackTypeDisabled(C.TRACK_TYPE_VIDEO, !videoEnabled); - - trackSelector.setParameters(parametersBuilder); } setRecovery(); + + // Disable or enable video and subtitles renderers depending of the videoEnabled value + trackSelector.setParameters(trackSelector.buildUponParameters() + .setTrackTypeDisabled(C.TRACK_TYPE_TEXT, !videoEnabled) + .setTrackTypeDisabled(C.TRACK_TYPE_VIDEO, !videoEnabled)); }, () -> { - // This is executed when the current stream info is not available. + /* + The current metadata may be null sometimes (for e.g. when using an unstable connection + in livestreams) so we will be not able to execute the block below + + Reload the play queue manager in this case, which is the behavior when we don't know the + index of the video renderer or playQueueManagerReloadingNeeded returns true + */ reloadPlayQueueManager(); setRecovery(); });