From 749c7107667ad120d628d9a0cc8c522fd1c8e163 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 11:58:27 +0100 Subject: [PATCH 01/12] PlayerUIList: transform to kotlin And simplify the code a little --- .../newpipe/player/ui/PlayerUiList.java | 90 ------------------ .../schabi/newpipe/player/ui/PlayerUiList.kt | 92 +++++++++++++++++++ 2 files changed, 92 insertions(+), 90 deletions(-) delete mode 100644 app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.java create mode 100644 app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.java b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.java deleted file mode 100644 index 24fec3b8afc..00000000000 --- a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.java +++ /dev/null @@ -1,90 +0,0 @@ -package org.schabi.newpipe.player.ui; - -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; -import java.util.function.Consumer; - -public final class PlayerUiList { - final List playerUis = new ArrayList<>(); - - /** - * Creates a {@link PlayerUiList} starting with the provided player uis. The provided player uis - * will not be prepared like those passed to {@link #addAndPrepare(PlayerUi)}, because when - * the {@link PlayerUiList} constructor is called, the player is still not running and it - * wouldn't make sense to initialize uis then. Instead the player will initialize them by doing - * proper calls to {@link #call(Consumer)}. - * - * @param initialPlayerUis the player uis this list should start with; the order will be kept - */ - public PlayerUiList(final PlayerUi... initialPlayerUis) { - playerUis.addAll(List.of(initialPlayerUis)); - } - - /** - * Adds the provided player ui to the list and calls on it the initialization functions that - * apply based on the current player state. The preparation step needs to be done since when UIs - * are removed and re-added, the player will not call e.g. initPlayer again since the exoplayer - * is already initialized, but we need to notify the newly built UI that the player is ready - * nonetheless. - * @param playerUi the player ui to prepare and add to the list; its {@link - * PlayerUi#getPlayer()} will be used to query information about the player - * state - */ - public void addAndPrepare(final PlayerUi playerUi) { - if (playerUi.getPlayer().getFragmentListener().isPresent()) { - // make sure UIs know whether a service is connected or not - playerUi.onFragmentListenerSet(); - } - - if (!playerUi.getPlayer().exoPlayerIsNull()) { - playerUi.initPlayer(); - if (playerUi.getPlayer().getPlayQueue() != null) { - playerUi.initPlayback(); - } - } - - playerUis.add(playerUi); - } - - /** - * Destroys all matching player UIs and removes them from the list. - * @param playerUiType the class of the player UI to destroy; the {@link - * Class#isInstance(Object)} method will be used, so even subclasses will be - * destroyed and removed - * @param the class type parameter - */ - public void destroyAll(final Class playerUiType) { - playerUis.stream() - .filter(playerUiType::isInstance) - .forEach(playerUi -> { - playerUi.destroyPlayer(); - playerUi.destroy(); - }); - playerUis.removeIf(playerUiType::isInstance); - } - - /** - * @param playerUiType the class of the player UI to return; the {@link - * Class#isInstance(Object)} method will be used, so even subclasses could - * be returned - * @param the class type parameter - * @return the first player UI of the required type found in the list, or an empty {@link - * Optional} otherwise - */ - public Optional get(final Class playerUiType) { - return playerUis.stream() - .filter(playerUiType::isInstance) - .map(playerUiType::cast) - .findFirst(); - } - - /** - * Calls the provided consumer on all player UIs in the list, in order of addition. - * @param consumer the consumer to call with player UIs - */ - public void call(final Consumer consumer) { - //noinspection SimplifyStreamApiCallChains - playerUis.stream().forEachOrdered(consumer); - } -} diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt new file mode 100644 index 00000000000..46090285f7b --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt @@ -0,0 +1,92 @@ +package org.schabi.newpipe.player.ui + +import androidx.core.util.Consumer +import java.util.Optional + +class PlayerUiList(vararg initialPlayerUis: PlayerUi) { + val playerUis = mutableListOf() + + /** + * Creates a [PlayerUiList] starting with the provided player uis. The provided player uis + * will not be prepared like those passed to [.addAndPrepare], because when + * the [PlayerUiList] constructor is called, the player is still not running and it + * wouldn't make sense to initialize uis then. Instead the player will initialize them by doing + * proper calls to [.call]. + * + * @param initialPlayerUis the player uis this list should start with; the order will be kept + */ + init { + playerUis.addAll(listOf(*initialPlayerUis)) + } + + /** + * Adds the provided player ui to the list and calls on it the initialization functions that + * apply based on the current player state. The preparation step needs to be done since when UIs + * are removed and re-added, the player will not call e.g. initPlayer again since the exoplayer + * is already initialized, but we need to notify the newly built UI that the player is ready + * nonetheless. + * @param playerUi the player ui to prepare and add to the list; its [PlayerUi.getPlayer] + * will be used to query information about the player state + */ + fun addAndPrepare(playerUi: PlayerUi) { + if (playerUi.getPlayer().fragmentListener.isPresent) { + // make sure UIs know whether a service is connected or not + playerUi.onFragmentListenerSet() + } + + if (!playerUi.getPlayer().exoPlayerIsNull()) { + playerUi.initPlayer() + if (playerUi.getPlayer().playQueue != null) { + playerUi.initPlayback() + } + } + + playerUis.add(playerUi) + } + + /** + * Destroys all matching player UIs and removes them from the list. + * @param playerUiType the class of the player UI to destroy; + * the [Class.isInstance] method will be used, so even subclasses will be + * destroyed and removed + * @param T the class type parameter + * */ + fun destroyAll(playerUiType: Class) { + for (ui in playerUis) { + if (playerUiType.isInstance(ui)) { + ui.destroyPlayer() + ui.destroy() + playerUis.remove(ui) + } + } + } + + /** + * @param playerUiType the class of the player UI to return; + * the [Class.isInstance] method will be used, so even subclasses could be returned + * @param T the class type parameter + * @return the first player UI of the required type found in the list, or an empty + * [ ] otherwise + */ + fun get(playerUiType: Class): Optional { + for (ui in playerUis) { + if (playerUiType.isInstance(ui)) { + when (val r = playerUiType.cast(ui)) { + null -> continue + else -> return Optional.of(r) + } + } + } + return Optional.empty() + } + + /** + * Calls the provided consumer on all player UIs in the list, in order of addition. + * @param consumer the consumer to call with player UIs + */ + fun call(consumer: java.util.function.Consumer) { + for (ui in playerUis) { + consumer.accept(ui) + } + } +} From 23aa1f82ce011f44c8aafa927f3a8941bef526e6 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 12:05:10 +0100 Subject: [PATCH 02/12] PlayerUIList: rename get to getOpt and make get nullable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Kotlin, dealing with nulls works better so we don’t need optional. --- .../fragments/detail/VideoDetailFragment.java | 30 +++++++++---------- .../org/schabi/newpipe/player/Player.java | 7 +++-- .../schabi/newpipe/player/PlayerService.java | 6 ++-- .../mediasession/MediaSessionPlayerUi.java | 2 +- .../player/notification/NotificationUtil.java | 2 +- .../schabi/newpipe/player/ui/PlayerUiList.kt | 21 +++++++++---- 6 files changed, 40 insertions(+), 28 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 976d5bdbeb9..97246cc2a2e 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -244,7 +244,7 @@ public void onServiceConnected(final PlayerService connectedPlayerService, // It will do nothing if the player is not in fullscreen mode hideSystemUiIfNeeded(); - final Optional playerUi = player.UIs().get(MainPlayerUi.class); + final Optional playerUi = player.UIs().getOpt(MainPlayerUi.class); if (!player.videoPlayerSelected() && !playAfterConnect) { return; } @@ -520,7 +520,7 @@ private void setOnClickListeners() { binding.overlayPlayPauseButton.setOnClickListener(v -> { if (playerIsNotStopped()) { player.playPause(); - player.UIs().get(VideoPlayerUi.class).ifPresent(ui -> ui.hideControls(0, 0)); + player.UIs().getOpt(VideoPlayerUi.class).ifPresent(ui -> ui.hideControls(0, 0)); showSystemUi(); } else { autoPlayEnabled = true; // forcefully start playing @@ -679,7 +679,7 @@ protected void initListeners() { @Override public boolean onKeyDown(final int keyCode) { return isPlayerAvailable() - && player.UIs().get(VideoPlayerUi.class) + && player.UIs().getOpt(VideoPlayerUi.class) .map(playerUi -> playerUi.onKeyDown(keyCode)).orElse(false); } @@ -1019,7 +1019,7 @@ private void toggleFullscreenIfInFullscreenMode() { // If a user watched video inside fullscreen mode and than chose another player // return to non-fullscreen mode if (isPlayerAvailable()) { - player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> { + player.UIs().getOpt(MainPlayerUi.class).ifPresent(playerUi -> { if (playerUi.isFullscreen()) { playerUi.toggleFullscreen(); } @@ -1235,7 +1235,7 @@ private void tryAddVideoPlayerView() { // setup the surface view height, so that it fits the video correctly setHeightThumbnail(); - player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> { + player.UIs().getOpt(MainPlayerUi.class).ifPresent(playerUi -> { // sometimes binding would be null here, even though getView() != null above u.u if (binding != null) { // prevent from re-adding a view multiple times @@ -1251,7 +1251,7 @@ private void removeVideoPlayerView() { makeDefaultHeightForVideoPlaceholder(); if (player != null) { - player.UIs().get(VideoPlayerUi.class).ifPresent(VideoPlayerUi::removeViewFromParent); + player.UIs().getOpt(VideoPlayerUi.class).ifPresent(VideoPlayerUi::removeViewFromParent); } } @@ -1318,7 +1318,7 @@ private void setHeightThumbnail(final int newHeight, final DisplayMetrics metric binding.detailThumbnailImageView.setMinimumHeight(newHeight); if (isPlayerAvailable()) { final int maxHeight = (int) (metrics.heightPixels * MAX_PLAYER_HEIGHT); - player.UIs().get(VideoPlayerUi.class).ifPresent(ui -> + player.UIs().getOpt(VideoPlayerUi.class).ifPresent(ui -> ui.getBinding().surfaceView.setHeights(newHeight, ui.isFullscreen() ? newHeight : maxHeight)); } @@ -1851,7 +1851,7 @@ public void onServiceStopped() { public void onFullscreenStateChanged(final boolean fullscreen) { setupBrightness(); if (!isPlayerAndPlayerServiceAvailable() - || player.UIs().get(MainPlayerUi.class).isEmpty() + || player.UIs().getOpt(MainPlayerUi.class).isEmpty() || getRoot().map(View::getParent).isEmpty()) { return; } @@ -1880,7 +1880,7 @@ public void onScreenRotationButtonClicked() { final boolean isLandscape = DeviceUtils.isLandscape(requireContext()); if (DeviceUtils.isTablet(activity) && (!globalScreenOrientationLocked(activity) || isLandscape)) { - player.UIs().get(MainPlayerUi.class).ifPresent(MainPlayerUi::toggleFullscreen); + player.UIs().getOpt(MainPlayerUi.class).ifPresent(MainPlayerUi::toggleFullscreen); return; } @@ -1980,7 +1980,7 @@ public void hideSystemUiIfNeeded() { } private boolean isFullscreen() { - return isPlayerAvailable() && player.UIs().get(VideoPlayerUi.class) + return isPlayerAvailable() && player.UIs().getOpt(VideoPlayerUi.class) .map(VideoPlayerUi::isFullscreen).orElse(false); } @@ -2057,7 +2057,7 @@ private void checkLandscape() { setAutoPlay(true); } - player.UIs().get(MainPlayerUi.class).ifPresent(MainPlayerUi::checkLandscape); + player.UIs().getOpt(MainPlayerUi.class).ifPresent(MainPlayerUi::checkLandscape); // Let's give a user time to look at video information page if video is not playing if (globalScreenOrientationLocked(activity) && !player.isPlaying()) { player.play(); @@ -2322,7 +2322,7 @@ && isPlayerAvailable() && player.isPlaying() && !isFullscreen() && !DeviceUtils.isTablet(activity)) { - player.UIs().get(MainPlayerUi.class) + player.UIs().getOpt(MainPlayerUi.class) .ifPresent(MainPlayerUi::toggleFullscreen); } setOverlayLook(binding.appBarLayout, behavior, 1); @@ -2336,7 +2336,7 @@ && isPlayerAvailable() // Re-enable clicks setOverlayElementsClickable(true); if (isPlayerAvailable()) { - player.UIs().get(MainPlayerUi.class) + player.UIs().getOpt(MainPlayerUi.class) .ifPresent(MainPlayerUi::closeItemsList); } setOverlayLook(binding.appBarLayout, behavior, 0); @@ -2347,7 +2347,7 @@ && isPlayerAvailable() showSystemUi(); } if (isPlayerAvailable()) { - player.UIs().get(MainPlayerUi.class).ifPresent(ui -> { + player.UIs().getOpt(MainPlayerUi.class).ifPresent(ui -> { if (ui.isControlsVisible()) { ui.hideControls(0, 0); } @@ -2444,7 +2444,7 @@ boolean isPlayerAndPlayerServiceAvailable() { public Optional getRoot() { return Optional.ofNullable(player) - .flatMap(player1 -> player1.UIs().get(VideoPlayerUi.class)) + .flatMap(player1 -> player1.UIs().getOpt(VideoPlayerUi.class)) .map(playerUi -> playerUi.getBinding().getRoot()); } 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 c6319c9e812..e0b883053b9 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -462,14 +462,15 @@ public void handleIntent(@NonNull final Intent intent) { } private void initUIsForCurrentPlayerType() { - if ((UIs.get(MainPlayerUi.class).isPresent() && playerType == PlayerType.MAIN) - || (UIs.get(PopupPlayerUi.class).isPresent() && playerType == PlayerType.POPUP)) { + if ((UIs.getOpt(MainPlayerUi.class).isPresent() && playerType == PlayerType.MAIN) + || (UIs.getOpt(PopupPlayerUi.class).isPresent() + && playerType == PlayerType.POPUP)) { // correct UI already in place return; } // try to reuse binding if possible - final PlayerBinding binding = UIs.get(VideoPlayerUi.class).map(VideoPlayerUi::getBinding) + final PlayerBinding binding = UIs.getOpt(VideoPlayerUi.class).map(VideoPlayerUi::getBinding) .orElseGet(() -> { if (playerType == PlayerType.AUDIO) { return null; diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java index 0e8ff795e6a..f16c374853a 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java @@ -73,7 +73,7 @@ otherwise if nothing is played or initializing the player and its components (es loading stream metadata) takes a lot of time, the app would crash on Android 8+ as the service would never be put in the foreground while we said to the system we would do so */ - player.UIs().get(NotificationPlayerUi.class) + player.UIs().getOpt(NotificationPlayerUi.class) .ifPresent(NotificationPlayerUi::createNotificationAndStartForeground); } @@ -95,7 +95,7 @@ public int onStartCommand(final Intent intent, final int flags, final int startI do anything */ if (player != null) { - player.UIs().get(NotificationPlayerUi.class) + player.UIs().getOpt(NotificationPlayerUi.class) .ifPresent(NotificationPlayerUi::createNotificationAndStartForeground); } @@ -113,7 +113,7 @@ public int onStartCommand(final Intent intent, final int flags, final int startI if (player != null) { player.handleIntent(intent); - player.UIs().get(MediaSessionPlayerUi.class) + player.UIs().getOpt(MediaSessionPlayerUi.class) .ifPresent(ui -> ui.handleMediaButtonIntent(intent)); } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java index c673e688c47..c3b427e2326 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java @@ -145,7 +145,7 @@ private ForwardingPlayer getForwardingPlayer() { public void play() { player.play(); // hide the player controls even if the play command came from the media session - player.UIs().get(VideoPlayerUi.class).ifPresent(ui -> ui.hideControls(0, 0)); + player.UIs().getOpt(VideoPlayerUi.class).ifPresent(ui -> ui.hideControls(0, 0)); } @Override diff --git a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java index 30420b0c7da..5658693f24d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java +++ b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java @@ -102,7 +102,7 @@ private synchronized NotificationCompat.Builder createNotification() { mediaStyle.setShowActionsInCompactView(compactSlots); } player.UIs() - .get(MediaSessionPlayerUi.class) + .getOpt(MediaSessionPlayerUi.class) .flatMap(MediaSessionPlayerUi::getSessionToken) .ifPresent(mediaStyle::setMediaSession); diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt index 46090285f7b..460dfeb0e0f 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt +++ b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt @@ -65,21 +65,32 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { * @param playerUiType the class of the player UI to return; * the [Class.isInstance] method will be used, so even subclasses could be returned * @param T the class type parameter - * @return the first player UI of the required type found in the list, or an empty - * [ ] otherwise + * @return the first player UI of the required type found in the list, or null */ - fun get(playerUiType: Class): Optional { + fun get(playerUiType: Class): T? { for (ui in playerUis) { if (playerUiType.isInstance(ui)) { when (val r = playerUiType.cast(ui)) { + // try all UIs before returning null null -> continue - else -> return Optional.of(r) + else -> return r } } } - return Optional.empty() + return null } + /** + * @param playerUiType the class of the player UI to return; + * the [Class.isInstance] method will be used, so even subclasses could be returned + * @param T the class type parameter + * @return the first player UI of the required type found in the list, or an empty + * [ ] otherwise + */ + @Deprecated("use get", ReplaceWith("get(playerUiType)")) + fun getOpt(playerUiType: Class): Optional = + Optional.ofNullable(get(playerUiType)) + /** * Calls the provided consumer on all player UIs in the list, in order of addition. * @param consumer the consumer to call with player UIs From 25b35f6dab622cc67ec292960f790bdf93f0f3c7 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 15:04:45 +0100 Subject: [PATCH 03/12] PlayerUiList: guard list actions with mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new implementation would throw `ConcurrentModificationExceptions` when destroying the UIs. So let’s play it safe and put the list behind a mutex. Adds a helper class `GuardedByMutex` that can be wrapped around a property to force all use-sites to acquire the lock before doing anything with the data. --- .../schabi/newpipe/player/ui/PlayerUiList.kt | 59 +++++++++++++------ .../org/schabi/newpipe/util/GuardedByMutex.kt | 47 +++++++++++++++ 2 files changed, 87 insertions(+), 19 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt index 460dfeb0e0f..812f11ec46f 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt +++ b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt @@ -1,10 +1,10 @@ package org.schabi.newpipe.player.ui -import androidx.core.util.Consumer +import org.schabi.newpipe.util.GuardedByMutex import java.util.Optional class PlayerUiList(vararg initialPlayerUis: PlayerUi) { - val playerUis = mutableListOf() + var playerUis = GuardedByMutex(mutableListOf()) /** * Creates a [PlayerUiList] starting with the provided player uis. The provided player uis @@ -16,7 +16,9 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { * @param initialPlayerUis the player uis this list should start with; the order will be kept */ init { - playerUis.addAll(listOf(*initialPlayerUis)) + playerUis.runWithLockSync { + lockData.addAll(listOf(*initialPlayerUis)) + } } /** @@ -41,7 +43,9 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { } } - playerUis.add(playerUi) + playerUis.runWithLockSync { + lockData.add(playerUi) + } } /** @@ -52,12 +56,24 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { * @param T the class type parameter * */ fun destroyAll(playerUiType: Class) { - for (ui in playerUis) { - if (playerUiType.isInstance(ui)) { - ui.destroyPlayer() - ui.destroy() - playerUis.remove(ui) + val toDestroy = mutableListOf() + + // short blocking removal from class to prevent interfering from other threads + playerUis.runWithLockSync { + val new = mutableListOf() + for (ui in lockData) { + if (playerUiType.isInstance(ui)) { + toDestroy.add(ui) + } else { + new.add(ui) + } } + lockData = new + } + // then actually destroy the UIs + for (ui in toDestroy) { + ui.destroyPlayer() + ui.destroy() } } @@ -67,18 +83,19 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { * @param T the class type parameter * @return the first player UI of the required type found in the list, or null */ - fun get(playerUiType: Class): T? { - for (ui in playerUis) { - if (playerUiType.isInstance(ui)) { - when (val r = playerUiType.cast(ui)) { - // try all UIs before returning null - null -> continue - else -> return r + fun get(playerUiType: Class): T? = + playerUis.runWithLockSync { + for (ui in lockData) { + if (playerUiType.isInstance(ui)) { + when (val r = playerUiType.cast(ui)) { + // try all UIs before returning null + null -> continue + else -> return@runWithLockSync r + } } } + return@runWithLockSync null } - return null - } /** * @param playerUiType the class of the player UI to return; @@ -96,7 +113,11 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { * @param consumer the consumer to call with player UIs */ fun call(consumer: java.util.function.Consumer) { - for (ui in playerUis) { + // copy the list out of the mutex before calling the consumer which might block + val new = playerUis.runWithLockSync { + lockData.toMutableList() + } + for (ui in new) { consumer.accept(ui) } } diff --git a/app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt b/app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt new file mode 100644 index 00000000000..1777a8cc3c8 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt @@ -0,0 +1,47 @@ +package org.schabi.newpipe.util + +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock + +/** Guard the given data so that it can only be accessed by locking the mutex first. + * + * Inspired by [this blog post](https://jonnyzzz.com/blog/2017/03/01/guarded-by-lock/) + * */ +class GuardedByMutex( + private var data: T, + private val lock: Mutex = Mutex(locked = false), +) { + + /** Lock the mutex and access the data, blocking the current thread. + * @param action to run with locked mutex + * */ + fun runWithLockSync( + action: MutexData.() -> Y + ) = + runBlocking { + lock.withLock { + MutexData(data, { d -> data = d }).action() + } + } + + /** Lock the mutex and access the data, suspending the coroutine. + * @param action to run with locked mutex + * */ + suspend fun runWithLock(action: MutexData.() -> Y) = + lock.withLock { + MutexData(data, { d -> data = d }).action() + } +} + +/** The data inside a [GuardedByMutex], which can be accessed via [lockData]. + * [lockData] is a `var`, so you can `set` it as well. + * */ +class MutexData(data: T, val setFun: (T) -> Unit) { + /** The data inside this [GuardedByMutex] */ + var lockData: T = data + set(t) { + setFun(t) + field = t + } +} \ No newline at end of file From 487f08c8132f1b0c1cc7f1933f642b5d8fa117b3 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:12:35 +0100 Subject: [PATCH 04/12] PlayerService: Convert to kotlin (mechanical) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No adjustments done yet; only change is that I added an upper bound on `PlayerUi` to the `PlayerUiList` functions, so “everything” is now `null` in `destroyAll`. --- .../org/schabi/newpipe/player/Player.java | 14 +- .../schabi/newpipe/player/PlayerService.java | 196 ----------------- .../schabi/newpipe/player/PlayerService.kt | 198 ++++++++++++++++++ .../schabi/newpipe/player/ui/PlayerUiList.kt | 12 +- .../org/schabi/newpipe/util/GuardedByMutex.kt | 2 +- 5 files changed, 215 insertions(+), 207 deletions(-) delete mode 100644 app/src/main/java/org/schabi/newpipe/player/PlayerService.java create mode 100644 app/src/main/java/org/schabi/newpipe/player/PlayerService.kt 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 e0b883053b9..6d1a5f43408 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -481,15 +481,15 @@ private void initUIsForCurrentPlayerType() { switch (playerType) { case MAIN: - UIs.destroyAll(PopupPlayerUi.class); + UIs.destroyAllOfType(PopupPlayerUi.class); UIs.addAndPrepare(new MainPlayerUi(this, binding)); break; case POPUP: - UIs.destroyAll(MainPlayerUi.class); + UIs.destroyAllOfType(MainPlayerUi.class); UIs.addAndPrepare(new PopupPlayerUi(this, binding)); break; case AUDIO: - UIs.destroyAll(VideoPlayerUi.class); + UIs.destroyAllOfType(VideoPlayerUi.class); break; } } @@ -595,7 +595,7 @@ public void destroy() { databaseUpdateDisposable.clear(); progressUpdateDisposable.set(null); - UIs.destroyAll(Object.class); // destroy every UI: obviously every UI extends Object + UIs.destroyAllOfType(null); } public void setRecovery() { @@ -1971,6 +1971,9 @@ public void setFragmentListener(final PlayerServiceEventListener listener) { triggerProgressUpdate(); } + /** Remove the listener, if it was set. + * @param listener listener to remove + * */ public void removeFragmentListener(final PlayerServiceEventListener listener) { if (fragmentListener == listener) { fragmentListener = null; @@ -1985,6 +1988,9 @@ void setActivityListener(final PlayerEventListener listener) { triggerProgressUpdate(); } + /** Remove the listener, if it was set. + * @param listener listener to remove + * */ void removeActivityListener(final PlayerEventListener listener) { if (activityListener == listener) { activityListener = null; diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java deleted file mode 100644 index f16c374853a..00000000000 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java +++ /dev/null @@ -1,196 +0,0 @@ -/* - * Copyright 2017 Mauricio Colli - * Part of NewPipe - * - * License: GPL-3.0+ - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package org.schabi.newpipe.player; - -import static org.schabi.newpipe.util.Localization.assureCorrectAppLanguage; - -import android.app.Service; -import android.content.Context; -import android.content.Intent; -import android.os.Binder; -import android.os.IBinder; -import android.util.Log; - -import androidx.annotation.Nullable; - -import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi; -import org.schabi.newpipe.player.notification.NotificationPlayerUi; -import org.schabi.newpipe.util.ThemeHelper; - -import java.lang.ref.WeakReference; - - -/** - * One background service for our player. Even though the player has multiple UIs - * (e.g. the audio-only UI, the main UI, the pulldown-menu UI), - * this allows us to keep playing even when switching between the different UIs. - */ -public final class PlayerService extends Service { - private static final String TAG = PlayerService.class.getSimpleName(); - private static final boolean DEBUG = Player.DEBUG; - - private Player player; - - private final IBinder mBinder = new PlayerService.LocalBinder(this); - - public Player getPlayer() { - return player; - } - - /*////////////////////////////////////////////////////////////////////////// - // Service's LifeCycle - //////////////////////////////////////////////////////////////////////////*/ - - @Override - public void onCreate() { - if (DEBUG) { - Log.d(TAG, "onCreate() called"); - } - assureCorrectAppLanguage(this); - ThemeHelper.setTheme(this); - - player = new Player(this); - /* - Create the player notification and start immediately the service in foreground, - otherwise if nothing is played or initializing the player and its components (especially - loading stream metadata) takes a lot of time, the app would crash on Android 8+ as the - service would never be put in the foreground while we said to the system we would do so - */ - player.UIs().getOpt(NotificationPlayerUi.class) - .ifPresent(NotificationPlayerUi::createNotificationAndStartForeground); - } - - @Override - public int onStartCommand(final Intent intent, final int flags, final int startId) { - if (DEBUG) { - Log.d(TAG, "onStartCommand() called with: intent = [" + intent - + "], flags = [" + flags + "], startId = [" + startId + "]"); - } - - /* - Be sure that the player notification is set and the service is started in foreground, - otherwise, the app may crash on Android 8+ as the service would never be put in the - foreground while we said to the system we would do so - The service is always requested to be started in foreground, so always creating a - notification if there is no one already and starting the service in foreground should - not create any issues - If the service is already started in foreground, requesting it to be started shouldn't - do anything - */ - if (player != null) { - player.UIs().getOpt(NotificationPlayerUi.class) - .ifPresent(NotificationPlayerUi::createNotificationAndStartForeground); - } - - if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction()) - && (player == null || player.getPlayQueue() == null)) { - /* - No need to process media button's actions if the player is not working, otherwise - the player service would strangely start with nothing to play - Stop the service in this case, which will be removed from the foreground and its - notification cancelled in its destruction - */ - stopSelf(); - return START_NOT_STICKY; - } - - if (player != null) { - player.handleIntent(intent); - player.UIs().getOpt(MediaSessionPlayerUi.class) - .ifPresent(ui -> ui.handleMediaButtonIntent(intent)); - } - - return START_NOT_STICKY; - } - - public void stopForImmediateReusing() { - if (DEBUG) { - Log.d(TAG, "stopForImmediateReusing() called"); - } - - if (player != null && !player.exoPlayerIsNull()) { - // Releases wifi & cpu, disables keepScreenOn, etc. - // We can't just pause the player here because it will make transition - // from one stream to a new stream not smooth - player.smoothStopForImmediateReusing(); - } - } - - @Override - public void onTaskRemoved(final Intent rootIntent) { - super.onTaskRemoved(rootIntent); - if (player != null && !player.videoPlayerSelected()) { - return; - } - onDestroy(); - // Unload from memory completely - Runtime.getRuntime().halt(0); - } - - @Override - public void onDestroy() { - if (DEBUG) { - Log.d(TAG, "destroy() called"); - } - cleanup(); - } - - private void cleanup() { - if (player != null) { - player.destroy(); - player = null; - } - } - - public void stopService() { - cleanup(); - stopSelf(); - } - - @Override - protected void attachBaseContext(final Context base) { - super.attachBaseContext(AudioServiceLeakFix.preventLeakOf(base)); - } - - @Override - public IBinder onBind(final Intent intent) { - return mBinder; - } - - /** - * Allows us this {@link org.schabi.newpipe.player.PlayerService} over the Service boundary - * back to our {@link org.schabi.newpipe.player.helper.PlayerHolder}. - */ - public static class LocalBinder extends Binder { - private final WeakReference playerService; - - LocalBinder(final PlayerService playerService) { - this.playerService = new WeakReference<>(playerService); - } - - /** - * Get the PlayerService object itself. - * @return this - * */ - public @Nullable PlayerService getService() { - return playerService.get(); - } - } -} diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt new file mode 100644 index 00000000000..6fe7240918b --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -0,0 +1,198 @@ +/* + * Copyright 2017 Mauricio Colli + * Part of NewPipe + * + * License: GPL-3.0+ + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.schabi.newpipe.player + +import android.app.Service +import android.content.Context +import android.content.Intent +import android.os.Binder +import android.os.IBinder +import android.util.Log +import org.schabi.newpipe.player.PlayerService.LocalBinder +import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi +import org.schabi.newpipe.player.notification.NotificationPlayerUi +import org.schabi.newpipe.util.Localization +import org.schabi.newpipe.util.ThemeHelper +import java.lang.ref.WeakReference +import java.util.function.Consumer + +/** + * One background service for our player. Even though the player has multiple UIs + * (e.g. the audio-only UI, the main UI, the pulldown-menu UI), + * this allows us to keep playing even when switching between the different UIs. + */ +class PlayerService : Service() { + private var player: Player? = null + + private val mBinder: IBinder = LocalBinder(this) + + fun getPlayer(): Player? { + return player + } + + /*////////////////////////////////////////////////////////////////////////// + // Service's LifeCycle + ////////////////////////////////////////////////////////////////////////// */ + override fun onCreate() { + if (DEBUG) { + Log.d(TAG, "onCreate() called") + } + Localization.assureCorrectAppLanguage(this) + ThemeHelper.setTheme(this) + + player = Player(this) + /* + Create the player notification and start immediately the service in foreground, + otherwise if nothing is played or initializing the player and its components (especially + loading stream metadata) takes a lot of time, the app would crash on Android 8+ as the + service would never be put in the foreground while we said to the system we would do so + */ + player!!.UIs().getOpt(NotificationPlayerUi::class.java) + .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) + } + + override fun onStartCommand(intent: Intent, flags: Int, startId: Int): Int { + if (DEBUG) { + Log.d( + TAG, + ( + "onStartCommand() called with: intent = [" + intent + + "], flags = [" + flags + "], startId = [" + startId + "]" + ) + ) + } + + /* + Be sure that the player notification is set and the service is started in foreground, + otherwise, the app may crash on Android 8+ as the service would never be put in the + foreground while we said to the system we would do so + The service is always requested to be started in foreground, so always creating a + notification if there is no one already and starting the service in foreground should + not create any issues + If the service is already started in foreground, requesting it to be started shouldn't + do anything + */ + if (player != null) { + player!!.UIs().getOpt(NotificationPlayerUi::class.java) + .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) + } + + if (Intent.ACTION_MEDIA_BUTTON == intent.getAction() && + (player == null || player!!.getPlayQueue() == null) + ) { + /* + No need to process media button's actions if the player is not working, otherwise + the player service would strangely start with nothing to play + Stop the service in this case, which will be removed from the foreground and its + notification cancelled in its destruction + */ + stopSelf() + return START_NOT_STICKY + } + + if (player != null) { + player!!.handleIntent(intent) + player!!.UIs().getOpt(MediaSessionPlayerUi::class.java) + .ifPresent( + Consumer { ui: MediaSessionPlayerUi? -> + ui!!.handleMediaButtonIntent( + intent + ) + } + ) + } + + return START_NOT_STICKY + } + + fun stopForImmediateReusing() { + if (DEBUG) { + Log.d(TAG, "stopForImmediateReusing() called") + } + + if (player != null && !player!!.exoPlayerIsNull()) { + // Releases wifi & cpu, disables keepScreenOn, etc. + // We can't just pause the player here because it will make transition + // from one stream to a new stream not smooth + player!!.smoothStopForImmediateReusing() + } + } + + override fun onTaskRemoved(rootIntent: Intent?) { + super.onTaskRemoved(rootIntent) + if (player != null && !player!!.videoPlayerSelected()) { + return + } + onDestroy() + // Unload from memory completely + Runtime.getRuntime().halt(0) + } + + override fun onDestroy() { + if (DEBUG) { + Log.d(TAG, "destroy() called") + } + cleanup() + } + + private fun cleanup() { + if (player != null) { + player!!.destroy() + player = null + } + } + + fun stopService() { + cleanup() + stopSelf() + } + + override fun attachBaseContext(base: Context?) { + super.attachBaseContext(AudioServiceLeakFix.preventLeakOf(base)) + } + + override fun onBind(intent: Intent?): IBinder { + return mBinder + } + + /** + * Allows us this [PlayerService] over the Service boundary + * back to our [org.schabi.newpipe.player.helper.PlayerHolder]. + */ + class LocalBinder internal constructor(playerService: PlayerService?) : Binder() { + private val playerService: WeakReference + + init { + this.playerService = WeakReference(playerService) + } + + /** + * Get the PlayerService object itself. + * @return this + */ + fun getService(): PlayerService? { + return playerService.get() + } + } + + companion object { + private val TAG: String = PlayerService::class.java.getSimpleName() + private val DEBUG = Player.DEBUG + } +} diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt index 812f11ec46f..280e34ae79b 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt +++ b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt @@ -50,19 +50,19 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { /** * Destroys all matching player UIs and removes them from the list. - * @param playerUiType the class of the player UI to destroy; - * the [Class.isInstance] method will be used, so even subclasses will be + * @param playerUiType the class of the player UI to destroy, everything if `null`. + * The [Class.isInstance] method will be used, so even subclasses will be * destroyed and removed * @param T the class type parameter * */ - fun destroyAll(playerUiType: Class) { + fun destroyAllOfType(playerUiType: Class? = null) { val toDestroy = mutableListOf() // short blocking removal from class to prevent interfering from other threads playerUis.runWithLockSync { val new = mutableListOf() for (ui in lockData) { - if (playerUiType.isInstance(ui)) { + if (playerUiType == null || playerUiType.isInstance(ui)) { toDestroy.add(ui) } else { new.add(ui) @@ -83,7 +83,7 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { * @param T the class type parameter * @return the first player UI of the required type found in the list, or null */ - fun get(playerUiType: Class): T? = + fun get(playerUiType: Class): T? = playerUis.runWithLockSync { for (ui in lockData) { if (playerUiType.isInstance(ui)) { @@ -105,7 +105,7 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) { * [ ] otherwise */ @Deprecated("use get", ReplaceWith("get(playerUiType)")) - fun getOpt(playerUiType: Class): Optional = + fun getOpt(playerUiType: Class): Optional = Optional.ofNullable(get(playerUiType)) /** diff --git a/app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt b/app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt index 1777a8cc3c8..b3bd077f813 100644 --- a/app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt +++ b/app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt @@ -44,4 +44,4 @@ class MutexData(data: T, val setFun: (T) -> Unit) { setFun(t) field = t } -} \ No newline at end of file +} From 7c1d33de54aca972552ae26724dfdc7416b85c11 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:33:30 +0100 Subject: [PATCH 05/12] PlayerService: use `lateinit` for the player This removes a bunch of unncessary checks. We forgo setting player to `null` in onDestroy, because shutting it down and when the service stops should be enough to free it for garbage collection. --- .../org/schabi/newpipe/player/Player.java | 7 ++- .../schabi/newpipe/player/PlayerService.kt | 54 +++++++------------ 2 files changed, 26 insertions(+), 35 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 6d1a5f43408..97d8d01debe 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -580,7 +580,12 @@ private void destroyPlayer() { } } - public void destroy() { + + /** Shut down this player. + * Saves the stream progress, sets recovery. + * Then destroys the player in all UIs and destroys the UIs as well. + */ + public void saveAndShutdown() { if (DEBUG) { Log.d(TAG, "destroy() called"); } diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt index 6fe7240918b..130549392c5 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -38,14 +38,11 @@ import java.util.function.Consumer * this allows us to keep playing even when switching between the different UIs. */ class PlayerService : Service() { - private var player: Player? = null + lateinit var player: Player + private set private val mBinder: IBinder = LocalBinder(this) - fun getPlayer(): Player? { - return player - } - /*////////////////////////////////////////////////////////////////////////// // Service's LifeCycle ////////////////////////////////////////////////////////////////////////// */ @@ -63,7 +60,7 @@ class PlayerService : Service() { loading stream metadata) takes a lot of time, the app would crash on Android 8+ as the service would never be put in the foreground while we said to the system we would do so */ - player!!.UIs().getOpt(NotificationPlayerUi::class.java) + player.UIs().getOpt(NotificationPlayerUi::class.java) .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) } @@ -88,13 +85,11 @@ class PlayerService : Service() { If the service is already started in foreground, requesting it to be started shouldn't do anything */ - if (player != null) { - player!!.UIs().getOpt(NotificationPlayerUi::class.java) - .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) - } + player.UIs().getOpt(NotificationPlayerUi::class.java) + .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) if (Intent.ACTION_MEDIA_BUTTON == intent.getAction() && - (player == null || player!!.getPlayQueue() == null) + (player.getPlayQueue() == null) ) { /* No need to process media button's actions if the player is not working, otherwise @@ -106,17 +101,15 @@ class PlayerService : Service() { return START_NOT_STICKY } - if (player != null) { - player!!.handleIntent(intent) - player!!.UIs().getOpt(MediaSessionPlayerUi::class.java) - .ifPresent( - Consumer { ui: MediaSessionPlayerUi? -> - ui!!.handleMediaButtonIntent( - intent - ) - } - ) - } + player.handleIntent(intent) + player.UIs().getOpt(MediaSessionPlayerUi::class.java) + .ifPresent( + Consumer { ui: MediaSessionPlayerUi? -> + ui!!.handleMediaButtonIntent( + intent + ) + } + ) return START_NOT_STICKY } @@ -126,17 +119,17 @@ class PlayerService : Service() { Log.d(TAG, "stopForImmediateReusing() called") } - if (player != null && !player!!.exoPlayerIsNull()) { + if (!player.exoPlayerIsNull()) { // Releases wifi & cpu, disables keepScreenOn, etc. // We can't just pause the player here because it will make transition // from one stream to a new stream not smooth - player!!.smoothStopForImmediateReusing() + player.smoothStopForImmediateReusing() } } override fun onTaskRemoved(rootIntent: Intent?) { super.onTaskRemoved(rootIntent) - if (player != null && !player!!.videoPlayerSelected()) { + if (!player.videoPlayerSelected()) { return } onDestroy() @@ -148,18 +141,11 @@ class PlayerService : Service() { if (DEBUG) { Log.d(TAG, "destroy() called") } - cleanup() - } - - private fun cleanup() { - if (player != null) { - player!!.destroy() - player = null - } + player.saveAndShutdown() } fun stopService() { - cleanup() + player.saveAndShutdown() stopSelf() } From b9eeee8bf62f037d93b0e559fe6963f84e010694 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:41:12 +0100 Subject: [PATCH 06/12] PlayerService: simplify nullable calls --- .../schabi/newpipe/player/PlayerService.kt | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt index 130549392c5..1f4687ad078 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -30,7 +30,6 @@ import org.schabi.newpipe.player.notification.NotificationPlayerUi import org.schabi.newpipe.util.Localization import org.schabi.newpipe.util.ThemeHelper import java.lang.ref.WeakReference -import java.util.function.Consumer /** * One background service for our player. Even though the player has multiple UIs @@ -60,8 +59,8 @@ class PlayerService : Service() { loading stream metadata) takes a lot of time, the app would crash on Android 8+ as the service would never be put in the foreground while we said to the system we would do so */ - player.UIs().getOpt(NotificationPlayerUi::class.java) - .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) + player.UIs().get(NotificationPlayerUi::class.java) + ?.createNotificationAndStartForeground() } override fun onStartCommand(intent: Intent, flags: Int, startId: Int): Int { @@ -85,11 +84,11 @@ class PlayerService : Service() { If the service is already started in foreground, requesting it to be started shouldn't do anything */ - player.UIs().getOpt(NotificationPlayerUi::class.java) - .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) + player.UIs().get(NotificationPlayerUi::class.java) + ?.createNotificationAndStartForeground() - if (Intent.ACTION_MEDIA_BUTTON == intent.getAction() && - (player.getPlayQueue() == null) + if (Intent.ACTION_MEDIA_BUTTON == intent.action && + (player.playQueue == null) ) { /* No need to process media button's actions if the player is not working, otherwise @@ -102,14 +101,8 @@ class PlayerService : Service() { } player.handleIntent(intent) - player.UIs().getOpt(MediaSessionPlayerUi::class.java) - .ifPresent( - Consumer { ui: MediaSessionPlayerUi? -> - ui!!.handleMediaButtonIntent( - intent - ) - } - ) + player.UIs().get(MediaSessionPlayerUi::class.java) + ?.handleMediaButtonIntent(intent) return START_NOT_STICKY } @@ -162,11 +155,8 @@ class PlayerService : Service() { * back to our [org.schabi.newpipe.player.helper.PlayerHolder]. */ class LocalBinder internal constructor(playerService: PlayerService?) : Binder() { - private val playerService: WeakReference - - init { - this.playerService = WeakReference(playerService) - } + private val playerService: WeakReference = + WeakReference(playerService) /** * Get the PlayerService object itself. From 969f59ae22f3d48c7482438ddac815d1adfe59c2 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:46:03 +0100 Subject: [PATCH 07/12] VideoDetailFragment: apply visibility suggestions Because the class is final, protected does not make sense (Android Studio auto-suggestions) --- .../fragments/detail/VideoDetailFragment.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 97246cc2a2e..96716786b69 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -190,21 +190,21 @@ public final class VideoDetailFragment }; @State - protected int serviceId = Constants.NO_SERVICE_ID; + int serviceId = Constants.NO_SERVICE_ID; @State @NonNull - protected String title = ""; + String title = ""; @State @Nullable - protected String url = null; + String url = null; @Nullable - protected PlayQueue playQueue = null; + private PlayQueue playQueue = null; @State int bottomSheetState = BottomSheetBehavior.STATE_EXPANDED; @State int lastStableBottomSheetState = BottomSheetBehavior.STATE_EXPANDED; @State - protected boolean autoPlayEnabled = true; + boolean autoPlayEnabled = true; @Nullable private StreamInfo currentInfo = null; @@ -806,7 +806,7 @@ private void prepareAndHandleInfo(final StreamInfo info, final boolean scrollToT } - protected void prepareAndLoadInfo() { + private void prepareAndLoadInfo() { scrollToTop(); startLoading(false); } @@ -1328,10 +1328,10 @@ private void showContent() { binding.detailContentRootHiding.setVisibility(View.VISIBLE); } - protected void setInitialData(final int newServiceId, - @Nullable final String newUrl, - @NonNull final String newTitle, - @Nullable final PlayQueue newPlayQueue) { + private void setInitialData(final int newServiceId, + @Nullable final String newUrl, + @NonNull final String newTitle, + @Nullable final PlayQueue newPlayQueue) { this.serviceId = newServiceId; this.url = newUrl; this.title = newTitle; From 44cb912c273a8f8e037453c31297aa09e8e643f0 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:51:11 +0100 Subject: [PATCH 08/12] VideoDetailFragment: apply more IDE suggestions --- .../fragments/detail/VideoDetailFragment.java | 53 +++++++++---------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 96716786b69..42aa164a89d 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -429,18 +429,15 @@ public void onDestroyView() { @Override public void onActivityResult(final int requestCode, final int resultCode, final Intent data) { super.onActivityResult(requestCode, resultCode, data); - switch (requestCode) { - case ReCaptchaActivity.RECAPTCHA_REQUEST: - if (resultCode == Activity.RESULT_OK) { - NavigationHelper.openVideoDetailFragment(requireContext(), getFM(), - serviceId, url, title, null, false); - } else { - Log.e(TAG, "ReCaptcha failed"); - } - break; - default: - Log.e(TAG, "Request code from activity not supported [" + requestCode + "]"); - break; + if (requestCode == ReCaptchaActivity.RECAPTCHA_REQUEST) { + if (resultCode == Activity.RESULT_OK) { + NavigationHelper.openVideoDetailFragment(requireContext(), getFM(), + serviceId, url, title, null, false); + } else { + Log.e(TAG, "ReCaptcha failed"); + } + } else { + Log.e(TAG, "Request code from activity not supported [" + requestCode + "]"); } } @@ -1129,7 +1126,7 @@ private void openNormalBackgroundPlayer(final boolean append) { } private void openMainPlayer() { - if (!isPlayerServiceAvailable()) { + if (noPlayerServiceAvailable()) { playerHolder.startService(autoPlayEnabled, this, this); return; } @@ -1154,7 +1151,7 @@ private void openMainPlayer() { */ private void hideMainPlayerOnLoadingNewStream() { final var root = getRoot(); - if (!isPlayerServiceAvailable() || root.isEmpty() || !player.videoPlayerSelected()) { + if (noPlayerServiceAvailable() || root.isEmpty() || !player.videoPlayerSelected()) { return; } @@ -1338,13 +1335,13 @@ private void setInitialData(final int newServiceId, this.playQueue = newPlayQueue; } - private void setErrorImage(final int imageResource) { + private void setErrorImage() { if (binding == null || activity == null) { return; } binding.detailThumbnailImageView.setImageDrawable( - AppCompatResources.getDrawable(requireContext(), imageResource)); + AppCompatResources.getDrawable(requireContext(), R.drawable.not_available_monkey)); animate(binding.detailThumbnailImageView, false, 0, AnimationType.ALPHA, 0, () -> animate(binding.detailThumbnailImageView, true, 500)); } @@ -1352,7 +1349,7 @@ private void setErrorImage(final int imageResource) { @Override public void handleError() { super.handleError(); - setErrorImage(R.drawable.not_available_monkey); + setErrorImage(); if (binding.relatedItemsLayout != null) { // hide related streams for tablets binding.relatedItemsLayout.setVisibility(View.INVISIBLE); @@ -1769,16 +1766,14 @@ public void onPlaybackUpdate(final int state, final PlaybackParameters parameters) { setOverlayPlayPauseImage(player != null && player.isPlaying()); - switch (state) { - case Player.STATE_PLAYING: - if (binding.positionView.getAlpha() != 1.0f - && player.getPlayQueue() != null - && player.getPlayQueue().getItem() != null - && player.getPlayQueue().getItem().getUrl().equals(url)) { - animate(binding.positionView, true, 100); - animate(binding.detailPositionView, true, 100); - } - break; + if (state == Player.STATE_PLAYING) { + if (binding.positionView.getAlpha() != 1.0f + && player.getPlayQueue() != null + && player.getPlayQueue().getItem() != null + && player.getPlayQueue().getItem().getUrl().equals(url)) { + animate(binding.positionView, true, 100); + animate(binding.detailPositionView, true, 100); + } } } @@ -2434,8 +2429,8 @@ boolean isPlayerAvailable() { return player != null; } - boolean isPlayerServiceAvailable() { - return playerService != null; + boolean noPlayerServiceAvailable() { + return playerService == null; } boolean isPlayerAndPlayerServiceAvailable() { From b82d048b13e83c3adc71308032680df5141bf31d Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:53:50 +0100 Subject: [PATCH 09/12] VideoDetailFragment: remove duplicate code in startLoading --- .../fragments/detail/VideoDetailFragment.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 42aa164a89d..3d6ca44c68d 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -810,18 +810,10 @@ private void prepareAndLoadInfo() { @Override public void startLoading(final boolean forceLoad) { - super.startLoading(forceLoad); - - initTabs(); - currentInfo = null; - if (currentWorker != null) { - currentWorker.dispose(); - } - - runWorker(forceLoad, stack.isEmpty()); + startLoading(forceLoad, null); } - private void startLoading(final boolean forceLoad, final boolean addToBackStack) { + private void startLoading(final boolean forceLoad, final @Nullable Boolean addToBackStack) { super.startLoading(forceLoad); initTabs(); @@ -830,7 +822,7 @@ private void startLoading(final boolean forceLoad, final boolean addToBackStack) currentWorker.dispose(); } - runWorker(forceLoad, addToBackStack); + runWorker(forceLoad, addToBackStack != null ? addToBackStack : stack.isEmpty()); } private void runWorker(final boolean forceLoad, final boolean addToBackStack) { From e56307ae3cebd20f2b2356a800ac7cce2ab7f83c Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 1 Jan 2025 14:10:45 +0100 Subject: [PATCH 10/12] VideoPlayerUi: suppress warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `R.id` link from the comment cannot be resolved, so let’s not link it for now. We are using some exoplayer2 resources, let’s silence the warning. --- .../main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 ec9d6783b48..bde34560445 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 @@ -16,6 +16,7 @@ import static org.schabi.newpipe.player.helper.PlayerHelper.nextResizeModeAndSaveToPrefs; import static org.schabi.newpipe.player.helper.PlayerHelper.retrieveSeekDurationFromPreferences; +import android.annotation.SuppressLint; import android.content.Intent; import android.content.res.Resources; import android.graphics.Bitmap; @@ -761,7 +762,7 @@ public boolean isFullscreen() { } /** - * Update the play/pause button ({@link R.id.playPauseButton}) to reflect the action + * Update the play/pause button (`R.id.playPauseButton`) to reflect the action * that will be performed when the button is clicked.. * @param action the action that is performed when the play/pause button is clicked */ @@ -947,6 +948,7 @@ public void onShuffleClicked() { player.toggleShuffleModeEnabled(); } + @SuppressLint("PrivateResource") @Override public void onRepeatModeChanged(@RepeatMode final int repeatMode) { super.onRepeatModeChanged(repeatMode); From c9d273b1a6cdbc11a7962fb5abf40a7fdbf7cc1b Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 1 Jan 2025 14:21:52 +0100 Subject: [PATCH 11/12] PlayerHolder: inline setListener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s only used in this one place. --- .../newpipe/player/helper/PlayerHolder.java | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java index 707a44ea5a4..125b8fa31d2 100644 --- a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java +++ b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java @@ -7,7 +7,6 @@ import android.os.IBinder; import android.util.Log; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.core.content.ContextCompat; @@ -111,18 +110,6 @@ public void unsetListeners() { holderListener = null; } - public void setListener(@NonNull final PlayerServiceEventListener newListener, - @NonNull final PlayerHolderLifecycleEventListener newHolderListener) { - listener = newListener; - holderListener = newHolderListener; - - // Force reload data from service - if (player != null) { - holderListener.onServiceConnected(playerService, false); - player.setFragmentListener(internalListener); - } - } - /** * Helper to handle context in common place as using the same * context to bind/unbind a service is crucial. @@ -147,7 +134,14 @@ public void startService(final boolean playAfterConnect, final PlayerHolderLifecycleEventListener newHolderListener ) { final Context context = getCommonContext(); - setListener(newListener, newHolderListener); + listener = newListener; + holderListener = newHolderListener; + + // Force reload data from service + if (player != null) { + holderListener.onServiceConnected(playerService, false); + player.setFragmentListener(internalListener); + } if (bound) { return; } From 5a2047f89b129ec0b618932dec06cca812bfd7d6 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 1 Jan 2025 14:28:37 +0100 Subject: [PATCH 12/12] PlayerHolder: improve interface docstrings --- .../event/PlayerHolderLifecycleEventListener.java | 11 ++++++++++- .../schabi/newpipe/player/helper/PlayerHolder.java | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/event/PlayerHolderLifecycleEventListener.java b/app/src/main/java/org/schabi/newpipe/player/event/PlayerHolderLifecycleEventListener.java index e5eaa09c734..e8e8f730773 100644 --- a/app/src/main/java/org/schabi/newpipe/player/event/PlayerHolderLifecycleEventListener.java +++ b/app/src/main/java/org/schabi/newpipe/player/event/PlayerHolderLifecycleEventListener.java @@ -2,9 +2,18 @@ import org.schabi.newpipe.player.PlayerService; -/** Gets signalled if our PlayerHolder (dis)connects from the PlayerService. */ +/** Gets signalled if our PlayerHolder (dis)connects from the PlayerService. + * This is currently only implemented by the + * {@link org.schabi.newpipe.fragments.detail.VideoDetailFragment}. */ public interface PlayerHolderLifecycleEventListener { + + /** Our {@link org.schabi.newpipe.player.helper.PlayerHolder} connected to its service. + * @param playerService The service the holder connected to + * @param playAfterConnect */ void onServiceConnected(PlayerService playerService, boolean playAfterConnect); + + /** Our {@link org.schabi.newpipe.player.helper.PlayerHolder} was unbound and thus + * disconnected from the {@link org.schabi.newpipe.player.PlayerService}. */ void onServiceDisconnected(); } diff --git a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java index 125b8fa31d2..26ef6d213bf 100644 --- a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java +++ b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java @@ -125,7 +125,8 @@ private Context getCommonContext() { * Connect to (and if needed start) the {@link PlayerService} * and bind {@link PlayerServiceConnection} to it. * If the service is already started, only set the listener. - * @param playAfterConnect If the service is started, start playing immediately + * @param playAfterConnect If this holder’s service was already started, + * start playing immediately * @param newListener set this listener * @param newHolderListener set this listener * */