From 545c4f078f443ca174fa58c16fe13210a39cf767 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 13 May 2025 15:12:47 +0200 Subject: [PATCH 01/13] PlayerUIList: restrict superclasses a little --- .../java/org/schabi/newpipe/player/Player.java | 16 ++++++++++++---- .../org/schabi/newpipe/player/ui/PlayerUiList.kt | 12 ++++++------ 2 files changed, 18 insertions(+), 10 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 4d1accf26..111a98551 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -492,15 +492,15 @@ public final class Player implements PlaybackListener, Listener { 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; } } @@ -606,7 +606,7 @@ public final class Player implements PlaybackListener, Listener { databaseUpdateDisposable.clear(); progressUpdateDisposable.set(null); - UIs.destroyAll(Object.class); // destroy every UI: obviously every UI extends Object + UIs.destroyAllOfType(null); } public void setRecovery() { @@ -1995,6 +1995,10 @@ public final class Player implements PlaybackListener, 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; @@ -2009,6 +2013,10 @@ public final class Player implements PlaybackListener, 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/ui/PlayerUiList.kt b/app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt index e258d5ac1..24a46d1ef 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) { * [Optional] otherwise */ @Deprecated("use get", ReplaceWith("get(playerUiType)")) - fun getOpt(playerUiType: Class): Optional = + fun getOpt(playerUiType: Class): Optional = Optional.ofNullable(get(playerUiType)) /** From 945fbd884b064f6d3f61dc4905ebe5bcfdd4154e Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 13 May 2025 15:15:33 +0200 Subject: [PATCH 02/13] PlayerService: Convert to kotlin (mechanical) --- .../{PlayerService.java => PlayerService.kt} | 352 +++++++++--------- 1 file changed, 180 insertions(+), 172 deletions(-) rename app/src/main/java/org/schabi/newpipe/player/{PlayerService.java => PlayerService.kt} (50%) diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt similarity index 50% rename from app/src/main/java/org/schabi/newpipe/player/PlayerService.java rename to app/src/main/java/org/schabi/newpipe/player/PlayerService.kt index f465bbe79..cebdf339c 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -16,103 +16,101 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ +package org.schabi.newpipe.player -package org.schabi.newpipe.player; - -import static org.schabi.newpipe.util.Localization.assureCorrectAppLanguage; - -import android.content.Context; -import android.content.Intent; -import android.os.Binder; -import android.os.Bundle; -import android.os.IBinder; -import android.support.v4.media.MediaBrowserCompat; -import android.support.v4.media.session.MediaSessionCompat; -import android.util.Log; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.core.app.ServiceCompat; -import androidx.media.MediaBrowserServiceCompat; - -import com.google.android.exoplayer2.ext.mediasession.MediaSessionConnector; - -import org.schabi.newpipe.ktx.BundleKt; -import org.schabi.newpipe.player.mediabrowser.MediaBrowserImpl; -import org.schabi.newpipe.player.mediabrowser.MediaBrowserPlaybackPreparer; -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; -import java.util.List; -import java.util.function.Consumer; - +import android.content.Context +import android.content.Intent +import android.os.Binder +import android.os.Bundle +import android.os.IBinder +import android.support.v4.media.MediaBrowserCompat +import android.support.v4.media.session.MediaSessionCompat +import android.util.Log +import androidx.core.app.ServiceCompat +import androidx.media.MediaBrowserServiceCompat +import com.google.android.exoplayer2.ext.mediasession.MediaSessionConnector +import org.schabi.newpipe.ktx.toDebugString +import org.schabi.newpipe.player.mediabrowser.MediaBrowserImpl +import org.schabi.newpipe.player.mediabrowser.MediaBrowserPlaybackPreparer +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.BiConsumer +import java.util.function.Consumer /** * One service for all players. */ -public final class PlayerService extends MediaBrowserServiceCompat { - private static final String TAG = PlayerService.class.getSimpleName(); - private static final boolean DEBUG = Player.DEBUG; - - public static final String SHOULD_START_FOREGROUND_EXTRA = "should_start_foreground_extra"; - public static final String BIND_PLAYER_HOLDER_ACTION = "bind_player_holder_action"; - +class PlayerService : MediaBrowserServiceCompat() { // These objects are used to cleanly separate the Service implementation (in this file) and the // media browser and playback preparer implementations. At the moment the playback preparer is // only used in conjunction with the media browser. - private MediaBrowserImpl mediaBrowserImpl; - private MediaBrowserPlaybackPreparer mediaBrowserPlaybackPreparer; + private var mediaBrowserImpl: MediaBrowserImpl? = null + private var mediaBrowserPlaybackPreparer: MediaBrowserPlaybackPreparer? = null // these are instantiated in onCreate() as per // https://developer.android.com/training/cars/media#browser_workflow - private MediaSessionCompat mediaSession; - private MediaSessionConnector sessionConnector; - - @Nullable - private Player player; - - private final IBinder mBinder = new PlayerService.LocalBinder(this); + private var mediaSession: MediaSessionCompat? = null + private var sessionConnector: MediaSessionConnector? = null /** - * The parameter taken by this {@link Consumer} can be null to indicate the player is being + * @return the current active player instance. May be null, since the player service can outlive + * the player e.g. to respond to Android Auto media browser queries. + */ + var player: Player? = null + private set + + private val mBinder: IBinder = LocalBinder(this) + + /** + * The parameter taken by this [Consumer] can be null to indicate the player is being * stopped. */ - @Nullable - private Consumer onPlayerStartedOrStopped = null; - + private var onPlayerStartedOrStopped: Consumer? = null //region Service lifecycle - @Override - public void onCreate() { - super.onCreate(); + override fun onCreate() { + super.onCreate() if (DEBUG) { - Log.d(TAG, "onCreate() called"); + Log.d(TAG, "onCreate() called") } - assureCorrectAppLanguage(this); - ThemeHelper.setTheme(this); + Localization.assureCorrectAppLanguage(this) + ThemeHelper.setTheme(this) - mediaBrowserImpl = new MediaBrowserImpl(this, this::notifyChildrenChanged); + mediaBrowserImpl = MediaBrowserImpl( + this, + Consumer { parentId: String? -> + this.notifyChildrenChanged( + parentId!! + ) + } + ) // see https://developer.android.com/training/cars/media#browser_workflow - mediaSession = new MediaSessionCompat(this, "MediaSessionPlayerServ"); - setSessionToken(mediaSession.getSessionToken()); - sessionConnector = new MediaSessionConnector(mediaSession); - sessionConnector.setMetadataDeduplicationEnabled(true); + mediaSession = MediaSessionCompat(this, "MediaSessionPlayerServ") + setSessionToken(mediaSession!!.getSessionToken()) + sessionConnector = MediaSessionConnector(mediaSession!!) + sessionConnector!!.setMetadataDeduplicationEnabled(true) - mediaBrowserPlaybackPreparer = new MediaBrowserPlaybackPreparer( - this, - sessionConnector::setCustomErrorMessage, - () -> sessionConnector.setCustomErrorMessage(null), - (playWhenReady) -> { - if (player != null) { - player.onPrepare(); - } + mediaBrowserPlaybackPreparer = MediaBrowserPlaybackPreparer( + this, + BiConsumer { message: String?, code: Int? -> + sessionConnector!!.setCustomErrorMessage( + message, + code!! + ) + }, + Runnable { sessionConnector!!.setCustomErrorMessage(null) }, + Consumer { playWhenReady: Boolean? -> + if (player != null) { + player!!.onPrepare() } - ); - sessionConnector.setPlaybackPreparer(mediaBrowserPlaybackPreparer); + } + ) + sessionConnector!!.setPlaybackPreparer(mediaBrowserPlaybackPreparer) // Note: you might be tempted to create the player instance and call startForeground here, // but be aware that the Android system might start the service just to perform media @@ -123,22 +121,26 @@ public final class PlayerService extends MediaBrowserServiceCompat { // useless notification. } - @Override - public int onStartCommand(final Intent intent, final int flags, final int startId) { + override fun onStartCommand(intent: Intent, flags: Int, startId: Int): Int { if (DEBUG) { - Log.d(TAG, "onStartCommand() called with: intent = [" + intent - + "], extras = [" + BundleKt.toDebugString(intent.getExtras()) - + "], flags = [" + flags + "], startId = [" + startId + "]"); + Log.d( + TAG, + ( + "onStartCommand() called with: intent = [" + intent + + "], extras = [" + intent.getExtras().toDebugString() + + "], flags = [" + flags + "], startId = [" + startId + "]" + ) + ) } // All internal NewPipe intents used to interact with the player, that are sent to the // PlayerService using startForegroundService(), will have SHOULD_START_FOREGROUND_EXTRA, // to ensure startForeground() is called (otherwise Android will force-crash the app). if (intent.getBooleanExtra(SHOULD_START_FOREGROUND_EXTRA, false)) { - final boolean playerWasNull = (player == null); + val playerWasNull = (player == null) if (playerWasNull) { // make sure the player exists, in case the service was resumed - player = new Player(this, mediaSession, sessionConnector); + player = Player(this, mediaSession!!, sessionConnector!!) } // Be sure that the player notification is set and the service is started in foreground, @@ -148,107 +150,112 @@ public final class PlayerService extends MediaBrowserServiceCompat { // 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. - player.UIs().getOpt(NotificationPlayerUi.class) - .ifPresent(NotificationPlayerUi::createNotificationAndStartForeground); + player!!.UIs().getOpt(NotificationPlayerUi::class.java) + .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) if (playerWasNull && onPlayerStartedOrStopped != null) { // notify that a new player was created (but do it after creating the foreground // notification just to make sure we don't incur, due to slowness, in // "Context.startForegroundService() did not then call Service.startForeground()") - onPlayerStartedOrStopped.accept(player); + onPlayerStartedOrStopped!!.accept(player) } } - if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction()) - && (player == null || player.getPlayQueue() == null)) { + 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 */ - destroyPlayerAndStopService(); - return START_NOT_STICKY; + destroyPlayerAndStopService() + return START_NOT_STICKY } if (player != null) { - player.handleIntent(intent); - player.UIs().getOpt(MediaSessionPlayerUi.class) - .ifPresent(ui -> ui.handleMediaButtonIntent(intent)); + player!!.handleIntent(intent) + player!!.UIs().getOpt(MediaSessionPlayerUi::class.java) + .ifPresent( + Consumer { ui: MediaSessionPlayerUi? -> + ui!!.handleMediaButtonIntent( + intent + ) + } + ) } - return START_NOT_STICKY; + return START_NOT_STICKY } - public void stopForImmediateReusing() { + fun stopForImmediateReusing() { if (DEBUG) { - Log.d(TAG, "stopForImmediateReusing() called"); + Log.d(TAG, "stopForImmediateReusing() called") } - if (player != null && !player.exoPlayerIsNull()) { + 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(); + player!!.smoothStopForImmediateReusing() } } - @Override - public void onTaskRemoved(final Intent rootIntent) { - super.onTaskRemoved(rootIntent); - if (player != null && !player.videoPlayerSelected()) { - return; + override fun onTaskRemoved(rootIntent: Intent?) { + super.onTaskRemoved(rootIntent) + if (player != null && !player!!.videoPlayerSelected()) { + return } - onDestroy(); + onDestroy() // Unload from memory completely - Runtime.getRuntime().halt(0); + Runtime.getRuntime().halt(0) } - @Override - public void onDestroy() { + override fun onDestroy() { if (DEBUG) { - Log.d(TAG, "destroy() called"); + Log.d(TAG, "destroy() called") } - super.onDestroy(); + super.onDestroy() - cleanup(); + cleanup() - mediaBrowserPlaybackPreparer.dispose(); - mediaSession.release(); - mediaBrowserImpl.dispose(); + mediaBrowserPlaybackPreparer!!.dispose() + mediaSession!!.release() + mediaBrowserImpl!!.dispose() } - private void cleanup() { + private fun cleanup() { if (player != null) { if (onPlayerStartedOrStopped != null) { // notify that the player is being destroyed - onPlayerStartedOrStopped.accept(null); + onPlayerStartedOrStopped!!.accept(null) } - player.destroy(); - player = null; + player!!.destroy() + player = null } // Should already be handled by MediaSessionPlayerUi, but just to be sure. - mediaSession.setActive(false); + mediaSession!!.setActive(false) // Should already be handled by NotificationUtil.cancelNotificationAndStopForeground() in // NotificationPlayerUi, but let's make sure that the foreground service is stopped. - ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE); + ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE) } /** * Destroys the player and allows the player instance to be garbage collected. Sets the media * session to inactive. Stops the foreground service and removes the player notification - * associated with it. Tries to stop the {@link PlayerService} completely, but this step will + * associated with it. Tries to stop the [PlayerService] completely, but this step will * have no effect in case some service connection still uses the service (e.g. the Android Auto * system accesses the media browser even when no player is running). */ - public void destroyPlayerAndStopService() { + fun destroyPlayerAndStopService() { if (DEBUG) { - Log.d(TAG, "destroyPlayerAndStopService() called"); + Log.d(TAG, "destroyPlayerAndStopService() called") } - cleanup(); + cleanup() // This only really stops the service if there are no other service connections (see docs): // for example the (Android Auto) media browser binder will block stopService(). @@ -256,95 +263,96 @@ public final class PlayerService extends MediaBrowserServiceCompat { // If we were to call stopSelf(), then the service would be surely stopped (regardless of // other service connections), but this would be a waste of resources since the service // would be immediately restarted by those same connections to perform the queries. - stopService(new Intent(this, PlayerService.class)); + stopService(Intent(this, PlayerService::class.java)) } - @Override - protected void attachBaseContext(final Context base) { - super.attachBaseContext(AudioServiceLeakFix.preventLeakOf(base)); + override fun attachBaseContext(base: Context?) { + super.attachBaseContext(AudioServiceLeakFix.preventLeakOf(base)) } + //endregion - //region Bind - @Override - public IBinder onBind(final Intent intent) { + override fun onBind(intent: Intent): IBinder? { if (DEBUG) { - Log.d(TAG, "onBind() called with: intent = [" + intent - + "], extras = [" + BundleKt.toDebugString(intent.getExtras()) + "]"); + Log.d( + TAG, + ( + "onBind() called with: intent = [" + intent + + "], extras = [" + intent.getExtras().toDebugString() + "]" + ) + ) } - if (BIND_PLAYER_HOLDER_ACTION.equals(intent.getAction())) { + if (BIND_PLAYER_HOLDER_ACTION == intent.getAction()) { // Note that this binder might be reused multiple times while the service is alive, even // after unbind() has been called: https://stackoverflow.com/a/8794930 . - return mBinder; - - } else if (MediaBrowserServiceCompat.SERVICE_INTERFACE.equals(intent.getAction())) { + return mBinder + } else if (SERVICE_INTERFACE == intent.getAction()) { // MediaBrowserService also uses its own binder, so for actions related to the media // browser service, pass the onBind to the superclass. - return super.onBind(intent); - + return super.onBind(intent) } else { // This is an unknown request, avoid returning any binder to not leak objects. - return null; + return null } } - public static class LocalBinder extends Binder { - private final WeakReference playerService; + class LocalBinder internal constructor(playerService: PlayerService?) : Binder() { + private val playerService: WeakReference - LocalBinder(final PlayerService playerService) { - this.playerService = new WeakReference<>(playerService); + init { + this.playerService = WeakReference(playerService) } - public PlayerService getService() { - return playerService.get(); - } - } - - /** - * @return the current active player instance. May be null, since the player service can outlive - * the player e.g. to respond to Android Auto media browser queries. - */ - @Nullable - public Player getPlayer() { - return player; + val service: PlayerService? + get() = playerService.get() } /** * Sets the listener that will be called when the player is started or stopped. If a - * {@code null} listener is passed, then the current listener will be unset. The parameter taken - * by the {@link Consumer} can be null to indicate that the player is stopping. + * `null` listener is passed, then the current listener will be unset. The parameter taken + * by the [Consumer] can be null to indicate that the player is stopping. * @param listener the listener to set or unset */ - public void setPlayerListener(@Nullable final Consumer listener) { - this.onPlayerStartedOrStopped = listener; + fun setPlayerListener(listener: Consumer?) { + this.onPlayerStartedOrStopped = listener if (listener != null) { // if there is no player, then `null` will be sent here, to ensure the state is synced - listener.accept(player); + listener.accept(player) } } - //endregion + //endregion //region Media browser - @Override - public BrowserRoot onGetRoot(@NonNull final String clientPackageName, - final int clientUid, - @Nullable final Bundle rootHints) { + override fun onGetRoot( + clientPackageName: String, + clientUid: Int, + rootHints: Bundle? + ): BrowserRoot { // TODO check if the accessing package has permission to view data - return mediaBrowserImpl.onGetRoot(clientPackageName, clientUid, rootHints); + return mediaBrowserImpl!!.onGetRoot(clientPackageName, clientUid, rootHints) } - @Override - public void onLoadChildren(@NonNull final String parentId, - @NonNull final Result> result) { - mediaBrowserImpl.onLoadChildren(parentId, result); + override fun onLoadChildren( + parentId: String, + result: Result> + ) { + mediaBrowserImpl!!.onLoadChildren(parentId, result) } - @Override - public void onSearch(@NonNull final String query, - final Bundle extras, - @NonNull final Result> result) { - mediaBrowserImpl.onSearch(query, result); + override fun onSearch( + query: String, + extras: Bundle?, + result: Result> + ) { + mediaBrowserImpl!!.onSearch(query, result) + } //endregion + + companion object { + private val TAG: String = PlayerService::class.java.getSimpleName() + private val DEBUG = Player.DEBUG + + const val SHOULD_START_FOREGROUND_EXTRA: String = "should_start_foreground_extra" + const val BIND_PLAYER_HOLDER_ACTION: String = "bind_player_holder_action" } - //endregion } From b5dd49ecd3afed3383abd95c2f193b751a220a6b Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 13 May 2025 15:39:37 +0200 Subject: [PATCH 03/13] PlayerService: simplify nullable calls, getters --- .../schabi/newpipe/player/PlayerService.kt | 35 ++++++++----------- 1 file changed, 15 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 cebdf339c..8fe42ebff 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -91,7 +91,7 @@ class PlayerService : MediaBrowserServiceCompat() { // see https://developer.android.com/training/cars/media#browser_workflow mediaSession = MediaSessionCompat(this, "MediaSessionPlayerServ") - setSessionToken(mediaSession!!.getSessionToken()) + setSessionToken(mediaSession!!.sessionToken) sessionConnector = MediaSessionConnector(mediaSession!!) sessionConnector!!.setMetadataDeduplicationEnabled(true) @@ -127,7 +127,7 @@ class PlayerService : MediaBrowserServiceCompat() { TAG, ( "onStartCommand() called with: intent = [" + intent + - "], extras = [" + intent.getExtras().toDebugString() + + "], extras = [" + intent.extras.toDebugString() + "], flags = [" + flags + "], startId = [" + startId + "]" ) ) @@ -150,8 +150,8 @@ class PlayerService : MediaBrowserServiceCompat() { // 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. - player!!.UIs().getOpt(NotificationPlayerUi::class.java) - .ifPresent(Consumer { obj: NotificationPlayerUi? -> obj!!.createNotificationAndStartForeground() }) + player!!.UIs().get(NotificationPlayerUi::class.java) + ?.createNotificationAndStartForeground() if (playerWasNull && onPlayerStartedOrStopped != null) { // notify that a new player was created (but do it after creating the foreground @@ -161,8 +161,8 @@ class PlayerService : MediaBrowserServiceCompat() { } } - if (Intent.ACTION_MEDIA_BUTTON == intent.getAction() && - (player == null || player!!.getPlayQueue() == null) + if (Intent.ACTION_MEDIA_BUTTON == intent.action && + (player == null || player!!.playQueue == null) ) { /* No need to process media button's actions if the player is not working, otherwise @@ -174,16 +174,11 @@ class PlayerService : MediaBrowserServiceCompat() { return START_NOT_STICKY } - if (player != null) { - player!!.handleIntent(intent) - player!!.UIs().getOpt(MediaSessionPlayerUi::class.java) - .ifPresent( - Consumer { ui: MediaSessionPlayerUi? -> - ui!!.handleMediaButtonIntent( - intent - ) - } - ) + val p = player + if (p != null) { + p.handleIntent(intent) + p.UIs().get(MediaSessionPlayerUi::class.java) + ?.handleMediaButtonIntent(intent) } return START_NOT_STICKY @@ -278,16 +273,16 @@ class PlayerService : MediaBrowserServiceCompat() { TAG, ( "onBind() called with: intent = [" + intent + - "], extras = [" + intent.getExtras().toDebugString() + "]" + "], extras = [" + intent.extras.toDebugString() + "]" ) ) } - if (BIND_PLAYER_HOLDER_ACTION == intent.getAction()) { + if (BIND_PLAYER_HOLDER_ACTION == intent.action) { // Note that this binder might be reused multiple times while the service is alive, even // after unbind() has been called: https://stackoverflow.com/a/8794930 . return mBinder - } else if (SERVICE_INTERFACE == intent.getAction()) { + } else if (SERVICE_INTERFACE == intent.action) { // MediaBrowserService also uses its own binder, so for actions related to the media // browser service, pass the onBind to the superclass. return super.onBind(intent) @@ -297,7 +292,7 @@ class PlayerService : MediaBrowserServiceCompat() { } } - class LocalBinder internal constructor(playerService: PlayerService?) : Binder() { + class LocalBinder internal constructor(playerService: PlayerService) : Binder() { private val playerService: WeakReference init { From 4d6e1a4ecfe9976c04341dbab84282709eb92ea5 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:46:03 +0100 Subject: [PATCH 04/13] 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 c516b6b4d..de745a97e 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 @@ -188,21 +188,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; @@ -815,7 +815,7 @@ public final class VideoDetailFragment } - protected void prepareAndLoadInfo() { + private void prepareAndLoadInfo() { scrollToTop(); startLoading(false); } @@ -1337,10 +1337,10 @@ public final class VideoDetailFragment 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 c37db85b973aba2f0bc297ba121aed1a6ae4c26c Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:51:11 +0100 Subject: [PATCH 05/13] 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 de745a97e..04a373190 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 @@ -438,18 +438,15 @@ public final class VideoDetailFragment @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 + "]"); } } @@ -1138,7 +1135,7 @@ public final class VideoDetailFragment } private void openMainPlayer() { - if (!isPlayerServiceAvailable()) { + if (noPlayerServiceAvailable()) { playerHolder.startService(autoPlayEnabled, this); return; } @@ -1163,7 +1160,7 @@ public final class VideoDetailFragment */ private void hideMainPlayerOnLoadingNewStream() { final var root = getRoot(); - if (!isPlayerServiceAvailable() || root.isEmpty() || !player.videoPlayerSelected()) { + if (noPlayerServiceAvailable() || root.isEmpty() || !player.videoPlayerSelected()) { return; } @@ -1347,13 +1344,13 @@ public final class VideoDetailFragment 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)); } @@ -1361,7 +1358,7 @@ public final class VideoDetailFragment @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); @@ -1776,16 +1773,14 @@ public final class VideoDetailFragment 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); + } } } @@ -2444,8 +2439,8 @@ public final class VideoDetailFragment return player != null; } - boolean isPlayerServiceAvailable() { - return playerService != null; + boolean noPlayerServiceAvailable() { + return playerService == null; } boolean isPlayerAndPlayerServiceAvailable() { From 0b32738d42f7b5058cd1abf9ac62bc4c7715ee97 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 26 Dec 2024 16:53:50 +0100 Subject: [PATCH 06/13] 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 04a373190..17ae16325 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 @@ -819,18 +819,10 @@ public final class VideoDetailFragment @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(); @@ -839,7 +831,7 @@ public final class VideoDetailFragment currentWorker.dispose(); } - runWorker(forceLoad, addToBackStack); + runWorker(forceLoad, addToBackStack != null ? addToBackStack : stack.isEmpty()); } private void runWorker(final boolean forceLoad, final boolean addToBackStack) { From 26050d808e6cfb2329567f76739439e2877c1e73 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 1 Jan 2025 14:10:45 +0100 Subject: [PATCH 07/13] 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. --- .../java/org/schabi/newpipe/player/ui/VideoPlayerUi.java | 5 ++++- 1 file changed, 4 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 7157d6af2..e96873de5 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.getTimeString; 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 abstract class VideoPlayerUi extends PlayerUi implements SeekBar.OnSeekBa } /** - * 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,8 @@ public abstract class VideoPlayerUi extends PlayerUi implements SeekBar.OnSeekBa player.toggleShuffleModeEnabled(); } + // TODO: don’t reference internal exoplayer2 resources + @SuppressLint("PrivateResource") @Override public void onRepeatModeChanged(@RepeatMode final int repeatMode) { super.onRepeatModeChanged(repeatMode); From 06cf5111885204b39686d7601c4815f92b5c7c70 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 1 Jan 2025 14:28:37 +0100 Subject: [PATCH 08/13] PlayerHolder: improve interface docstrings --- .../org/schabi/newpipe/player/helper/PlayerHolder.java | 8 ++++++++ 1 file changed, 8 insertions(+) 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 331ea71c0..22540a138 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 @@ -120,6 +120,14 @@ public final class PlayerHolder { return App.getInstance(); } + /** + * 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 this holder’s service was already started, + * start playing immediately + * @param newListener set this listener + * */ public void startService(final boolean playAfterConnect, final PlayerServiceExtendedEventListener newListener) { if (DEBUG) { From f5a4af2d6788b93e6698853afe6d1dc9c445cda9 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 13 May 2025 15:51:21 +0200 Subject: [PATCH 09/13] Player: destroy -> saveAndShutdown --- .../main/java/org/schabi/newpipe/player/Player.java | 10 ++++++++-- .../java/org/schabi/newpipe/player/PlayerService.kt | 2 +- 2 files changed, 9 insertions(+), 3 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 111a98551..d3e3ff1df 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -591,9 +591,15 @@ public final class Player implements PlaybackListener, Listener { } } - 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"); + Log.d(TAG, "saveAndShutdown() called"); } saveStreamProgressState(); 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 8fe42ebff..10ae01e17 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -226,7 +226,7 @@ class PlayerService : MediaBrowserServiceCompat() { // notify that the player is being destroyed onPlayerStartedOrStopped!!.accept(null) } - player!!.destroy() + player!!.saveAndShutdown() player = null } From be373dca8d63a45801c6ba914f23596e873fed9d Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 13 May 2025 15:55:13 +0200 Subject: [PATCH 10/13] PlayerUIList: make UI list private --- app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 24a46d1ef..b3caca205 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 @@ -4,7 +4,7 @@ import org.schabi.newpipe.util.GuardedByMutex import java.util.Optional class PlayerUiList(vararg initialPlayerUis: PlayerUi) { - var playerUis = GuardedByMutex(mutableListOf()) + private var playerUis = GuardedByMutex(mutableListOf()) /** * Creates a [PlayerUiList] starting with the provided player uis. The provided player uis From 36115c316438a2ce9891b2ff76cee2a05d9d7ad2 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 13 May 2025 17:00:46 +0200 Subject: [PATCH 11/13] PlayerService: remove !! where possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s a bit unwieldy in places, but should improve the safety of the code in the face of possible race conditions. --- .../schabi/newpipe/player/PlayerService.kt | 73 +++++++++---------- 1 file changed, 36 insertions(+), 37 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 10ae01e17..f0102e4b9 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -82,35 +82,35 @@ class PlayerService : MediaBrowserServiceCompat() { mediaBrowserImpl = MediaBrowserImpl( this, - Consumer { parentId: String? -> + Consumer { parentId: String -> this.notifyChildrenChanged( - parentId!! + parentId ) } ) // see https://developer.android.com/training/cars/media#browser_workflow - mediaSession = MediaSessionCompat(this, "MediaSessionPlayerServ") - setSessionToken(mediaSession!!.sessionToken) - sessionConnector = MediaSessionConnector(mediaSession!!) - sessionConnector!!.setMetadataDeduplicationEnabled(true) + val session = MediaSessionCompat(this, "MediaSessionPlayerServ") + mediaSession = session + setSessionToken(session.sessionToken) + val connector = MediaSessionConnector(session) + sessionConnector = connector + connector.setMetadataDeduplicationEnabled(true) mediaBrowserPlaybackPreparer = MediaBrowserPlaybackPreparer( this, - BiConsumer { message: String?, code: Int? -> - sessionConnector!!.setCustomErrorMessage( + BiConsumer { message: String, code: Int -> + connector.setCustomErrorMessage( message, - code!! + code ) }, - Runnable { sessionConnector!!.setCustomErrorMessage(null) }, + Runnable { connector.setCustomErrorMessage(null) }, Consumer { playWhenReady: Boolean? -> - if (player != null) { - player!!.onPrepare() - } + player?.onPrepare() } ) - sessionConnector!!.setPlaybackPreparer(mediaBrowserPlaybackPreparer) + connector.setPlaybackPreparer(mediaBrowserPlaybackPreparer) // Note: you might be tempted to create the player instance and call startForeground here, // but be aware that the Android system might start the service just to perform media @@ -153,16 +153,18 @@ class PlayerService : MediaBrowserServiceCompat() { player!!.UIs().get(NotificationPlayerUi::class.java) ?.createNotificationAndStartForeground() - if (playerWasNull && onPlayerStartedOrStopped != null) { + val startedOrStopped = onPlayerStartedOrStopped + if (playerWasNull && startedOrStopped != null) { // notify that a new player was created (but do it after creating the foreground // notification just to make sure we don't incur, due to slowness, in // "Context.startForegroundService() did not then call Service.startForeground()") - onPlayerStartedOrStopped!!.accept(player) + startedOrStopped.accept(player) } } + val p = player if (Intent.ACTION_MEDIA_BUTTON == intent.action && - (player == null || player!!.playQueue == null) + (p == null || p.playQueue == null) ) { /* No need to process media button's actions if the player is not working, otherwise @@ -174,7 +176,6 @@ class PlayerService : MediaBrowserServiceCompat() { return START_NOT_STICKY } - val p = player if (p != null) { p.handleIntent(intent) p.UIs().get(MediaSessionPlayerUi::class.java) @@ -189,17 +190,19 @@ class PlayerService : MediaBrowserServiceCompat() { Log.d(TAG, "stopForImmediateReusing() called") } - if (player != null && !player!!.exoPlayerIsNull()) { + val p = player + if (p != null && !p.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() + p.smoothStopForImmediateReusing() } } override fun onTaskRemoved(rootIntent: Intent?) { super.onTaskRemoved(rootIntent) - if (player != null && !player!!.videoPlayerSelected()) { + val p = player + if (p != null && !p.videoPlayerSelected()) { return } onDestroy() @@ -215,23 +218,22 @@ class PlayerService : MediaBrowserServiceCompat() { cleanup() - mediaBrowserPlaybackPreparer!!.dispose() - mediaSession!!.release() - mediaBrowserImpl!!.dispose() + mediaBrowserPlaybackPreparer?.dispose() + mediaSession?.release() + mediaBrowserImpl?.dispose() } private fun cleanup() { - if (player != null) { - if (onPlayerStartedOrStopped != null) { - // notify that the player is being destroyed - onPlayerStartedOrStopped!!.accept(null) - } - player!!.saveAndShutdown() + val p = player + if (p != null) { + // notify that the player is being destroyed + onPlayerStartedOrStopped?.accept(null) + p.saveAndShutdown() player = null } // Should already be handled by MediaSessionPlayerUi, but just to be sure. - mediaSession!!.setActive(false) + mediaSession?.setActive(false) // Should already be handled by NotificationUtil.cancelNotificationAndStopForeground() in // NotificationPlayerUi, but let's make sure that the foreground service is stopped. @@ -311,10 +313,7 @@ class PlayerService : MediaBrowserServiceCompat() { */ fun setPlayerListener(listener: Consumer?) { this.onPlayerStartedOrStopped = listener - if (listener != null) { - // if there is no player, then `null` will be sent here, to ensure the state is synced - listener.accept(player) - } + listener?.accept(player) } //endregion @@ -332,7 +331,7 @@ class PlayerService : MediaBrowserServiceCompat() { parentId: String, result: Result> ) { - mediaBrowserImpl!!.onLoadChildren(parentId, result) + mediaBrowserImpl?.onLoadChildren(parentId, result) } override fun onSearch( @@ -340,7 +339,7 @@ class PlayerService : MediaBrowserServiceCompat() { extras: Bundle?, result: Result> ) { - mediaBrowserImpl!!.onSearch(query, result) + mediaBrowserImpl?.onSearch(query, result) } //endregion companion object { From cf8fe95abf195578763b720fcbe3e513cd8633f2 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 13 May 2025 17:11:52 +0200 Subject: [PATCH 12/13] PlayerService: runtime-assert that we get passed a service We directly call the `getService` function after receiving the argument, so resolving the WeakPointer should never return `null` in our case. Of course there could be a race condition in theory, but I feel like if that happens we have bigger problems? --- .../schabi/newpipe/player/PlayQueueActivity.java | 13 ++++++++++--- .../schabi/newpipe/player/helper/PlayerHolder.java | 12 +++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayQueueActivity.java b/app/src/main/java/org/schabi/newpipe/player/PlayQueueActivity.java index 49aff657a..9d680da4d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayQueueActivity.java +++ b/app/src/main/java/org/schabi/newpipe/player/PlayQueueActivity.java @@ -220,11 +220,18 @@ public final class PlayQueueActivity extends AppCompatActivity } @Override - public void onServiceConnected(final ComponentName name, final IBinder service) { + public void onServiceConnected(final ComponentName name, final IBinder binder) { Log.d(TAG, "Player service is connected"); - if (service instanceof PlayerService.LocalBinder) { - player = ((PlayerService.LocalBinder) service).getService().getPlayer(); + if (binder instanceof PlayerService.LocalBinder) { + @Nullable final PlayerService s = + ((PlayerService.LocalBinder) binder).getService(); + if (s == null) { + throw new IllegalArgumentException( + "PlayerService.LocalBinder.getService() must never be" + + "null after the service connects"); + } + player = s.getPlayer(); } if (player == null || player.getPlayQueue() == null || player.exoPlayerIsNull()) { 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 22540a138..5452068d9 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 @@ -188,9 +188,15 @@ public final class PlayerHolder { } final PlayerService.LocalBinder localBinder = (PlayerService.LocalBinder) service; - playerService = localBinder.getService(); + @Nullable final PlayerService s = localBinder.getService(); + if (s == null) { + throw new IllegalArgumentException( + "PlayerService.LocalBinder.getService() must never be" + + "null after the service connects"); + } + playerService = s; if (listener != null) { - listener.onServiceConnected(playerService); + listener.onServiceConnected(s); getPlayer().ifPresent(p -> listener.onPlayerConnected(p, playAfterConnect)); } startPlayerListener(); @@ -198,7 +204,7 @@ public final class PlayerHolder { // notify the main activity that binding the service has completed, so that it can // open the bottom mini-player - NavigationHelper.sendPlayerStartedEvent(localBinder.getService()); + NavigationHelper.sendPlayerStartedEvent(s); } } From 73fef268fc6a11163f174418b830d9017ff747ab Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 5 Jun 2025 13:51:19 +0200 Subject: [PATCH 13/13] PlayerService/PlayerUIList: some small improvements --- .../java/org/schabi/newpipe/player/PlayerService.kt | 10 +++------- .../java/org/schabi/newpipe/player/ui/PlayerUiList.kt | 2 +- 2 files changed, 4 insertions(+), 8 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 f0102e4b9..c335611b0 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -295,11 +295,7 @@ class PlayerService : MediaBrowserServiceCompat() { } class LocalBinder internal constructor(playerService: PlayerService) : Binder() { - private val playerService: WeakReference - - init { - this.playerService = WeakReference(playerService) - } + private val playerService = WeakReference(playerService) val service: PlayerService? get() = playerService.get() @@ -322,9 +318,9 @@ class PlayerService : MediaBrowserServiceCompat() { clientPackageName: String, clientUid: Int, rootHints: Bundle? - ): BrowserRoot { + ): BrowserRoot? { // TODO check if the accessing package has permission to view data - return mediaBrowserImpl!!.onGetRoot(clientPackageName, clientUid, rootHints) + return mediaBrowserImpl?.onGetRoot(clientPackageName, clientUid, rootHints) } override fun onLoadChildren( 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 b3caca205..190da81e6 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 @@ -4,7 +4,7 @@ import org.schabi.newpipe.util.GuardedByMutex import java.util.Optional class PlayerUiList(vararg initialPlayerUis: PlayerUi) { - private var playerUis = GuardedByMutex(mutableListOf()) + private val playerUis = GuardedByMutex(mutableListOf()) /** * Creates a [PlayerUiList] starting with the provided player uis. The provided player uis