Merge pull request #11867 from Profpatsch/player-holder-refactor

PlayerHolder refactor

app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java

@@ -95,7 +95,8 @@ import org.schabi.newpipe.player.Player;
 import org.schabi.newpipe.player.PlayerService;
 import org.schabi.newpipe.player.PlayerType;
 import org.schabi.newpipe.player.event.OnKeyDownListener;
-import org.schabi.newpipe.player.event.PlayerServiceExtendedEventListener;
+import org.schabi.newpipe.player.event.PlayerHolderLifecycleEventListener;
+import org.schabi.newpipe.player.event.PlayerServiceEventListener;
 import org.schabi.newpipe.player.helper.PlayerHelper;
 import org.schabi.newpipe.player.helper.PlayerHolder;
 import org.schabi.newpipe.player.playqueue.PlayQueue;
@@ -136,7 +137,8 @@ import io.reactivex.rxjava3.schedulers.Schedulers;
 public final class VideoDetailFragment
         extends BaseStateFragment<StreamInfo>
         implements BackPressable,
-        PlayerServiceExtendedEventListener,
+        PlayerServiceEventListener,
+        PlayerHolderLifecycleEventListener,
         OnKeyDownListener {
     public static final String KEY_SWITCHING_PLAYERS = "switching_players";
 
@@ -234,10 +236,9 @@ public final class VideoDetailFragment
     // Service management
     //////////////////////////////////////////////////////////////////////////*/
     @Override
-    public void onServiceConnected(final Player connectedPlayer,
+    public void onServiceConnected(final PlayerService connectedPlayerService,
-                                   final PlayerService connectedPlayerService,
                                    final boolean playAfterConnect) {
-        player = connectedPlayer;
+        player = connectedPlayerService.getPlayer();
         playerService = connectedPlayerService;
 
         // It will do nothing if the player is not in fullscreen mode
@@ -393,7 +394,7 @@ public final class VideoDetailFragment
         if (activity.isFinishing() && isPlayerAvailable() && player.videoPlayerSelected()) {
             playerHolder.stopService();
         } else {
-            playerHolder.setListener(null);
+            playerHolder.unsetListeners();
         }
 
         PreferenceManager.getDefaultSharedPreferences(activity)
@@ -658,10 +659,10 @@ public final class VideoDetailFragment
         });
 
         setupBottomPlayer();
-        if (!playerHolder.isBound()) {
+        if (playerHolder.isNotBoundYet()) {
             setHeightThumbnail();
         } else {
-            playerHolder.startService(false, this);
+            playerHolder.startService(false, this, this);
         }
     }
 
@@ -1052,7 +1053,7 @@ public final class VideoDetailFragment
 
         // See UI changes while remote playQueue changes
         if (!isPlayerAvailable()) {
-            playerHolder.startService(false, this);
+            playerHolder.startService(false, this, this);
         } else {
             // FIXME Workaround #7427
             player.setRecovery();
@@ -1115,7 +1116,7 @@ public final class VideoDetailFragment
     private void openNormalBackgroundPlayer(final boolean append) {
         // See UI changes while remote playQueue changes
         if (!isPlayerAvailable()) {
-            playerHolder.startService(false, this);
+            playerHolder.startService(false, this, this);
         }
 
         final PlayQueue queue = setupPlayQueueForIntent(append);
@@ -1129,7 +1130,7 @@ public final class VideoDetailFragment
 
     private void openMainPlayer() {
         if (!isPlayerServiceAvailable()) {
-            playerHolder.startService(autoPlayEnabled, this);
+            playerHolder.startService(autoPlayEnabled, this, this);
             return;
         }
         if (currentInfo == null) {
@@ -1384,9 +1385,11 @@ public final class VideoDetailFragment
                             bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED);
                         }
                         // Rebound to the service if it was closed via notification or mini player
-                        if (!playerHolder.isBound()) {
+                        if (playerHolder.isNotBoundYet()) {
                             playerHolder.startService(
-                                    false, VideoDetailFragment.this);
+                                    false,
+                                    VideoDetailFragment.this,
+                                    VideoDetailFragment.this);
                         }
                         break;
                 }

app/src/main/java/org/schabi/newpipe/player/PlayQueueActivity.java

@@ -217,11 +217,16 @@ 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) {
+                if (binder instanceof PlayerService.LocalBinder localBinder) {
-                    player = ((PlayerService.LocalBinder) service).getPlayer();
+                    final @Nullable PlayerService s = localBinder.getService();
+                    if (s == null) {
+                        player = null;
+                    } else {
+                        player = s.getPlayer();
+                    }
                 }
 
                 if (player == null || player.getPlayQueue() == null || player.exoPlayerIsNull()) {

app/src/main/java/org/schabi/newpipe/player/PlayerService.java

@@ -28,6 +28,8 @@ 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;
@@ -36,7 +38,9 @@ import java.lang.ref.WeakReference;
 
 
 /**
- * One service for all players.
+ * 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();
@@ -46,6 +50,9 @@ public final class PlayerService extends Service {
 
     private final IBinder mBinder = new PlayerService.LocalBinder(this);
 
+    public Player getPlayer() {
+        return player;
+    }
 
     /*//////////////////////////////////////////////////////////////////////////
     // Service's LifeCycle
@@ -167,6 +174,10 @@ public final class PlayerService extends Service {
         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> playerService;
 
@@ -174,12 +185,12 @@ public final class PlayerService extends Service {
             this.playerService = new WeakReference<>(playerService);
         }
 
-        public PlayerService getService() {
+        /**
+         * Get the PlayerService object itself.
+         * @return this
+         * */
+        public @Nullable PlayerService getService() {
             return playerService.get();
         }
-
-        public Player getPlayer() {
-            return playerService.get().player;
-        }
     }
 }

app/src/main/java/org/schabi/newpipe/player/event/PlayerEventListener.java

@@ -5,6 +5,7 @@ import com.google.android.exoplayer2.PlaybackParameters;
 import org.schabi.newpipe.extractor.stream.StreamInfo;
 import org.schabi.newpipe.player.playqueue.PlayQueue;
 
+/** Player-specific events like queue or progress updates. */
 public interface PlayerEventListener {
     void onQueueUpdate(PlayQueue queue);
     void onPlaybackUpdate(int state, int repeatMode, boolean shuffled,

app/src/main/java/org/schabi/newpipe/player/event/PlayerHolderLifecycleEventListener.java

@@ -0,0 +1,10 @@
+package org.schabi.newpipe.player.event;
+
+import org.schabi.newpipe.player.PlayerService;
+
+/** Gets signalled if our PlayerHolder (dis)connects from the PlayerService. */
+public interface PlayerHolderLifecycleEventListener {
+    void onServiceConnected(PlayerService playerService,
+                            boolean playAfterConnect);
+    void onServiceDisconnected();
+}

app/src/main/java/org/schabi/newpipe/player/event/PlayerServiceEventListener.java

@@ -2,6 +2,9 @@ package org.schabi.newpipe.player.event;
 
 import com.google.android.exoplayer2.PlaybackException;
 
+/** {@link org.schabi.newpipe.player.event.PlayerEventListener} that also gets called for
+ * application-specific events like screen rotation or UI changes.
+ */
 public interface PlayerServiceEventListener extends PlayerEventListener {
     void onViewCreated();
 

app/src/main/java/org/schabi/newpipe/player/event/PlayerServiceExtendedEventListener.java

@@ -1,11 +0,0 @@
-package org.schabi.newpipe.player.event;
-
-import org.schabi.newpipe.player.PlayerService;
-import org.schabi.newpipe.player.Player;
-
-public interface PlayerServiceExtendedEventListener extends PlayerServiceEventListener {
-    void onServiceConnected(Player player,
-                            PlayerService playerService,
-                            boolean playAfterConnect);
-    void onServiceDisconnected();
-}

app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java

@@ -7,6 +7,7 @@ import android.content.ServiceConnection;
 import android.os.IBinder;
 import android.util.Log;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.core.content.ContextCompat;
 
@@ -20,9 +21,13 @@ import org.schabi.newpipe.player.Player;
 import org.schabi.newpipe.player.PlayerService;
 import org.schabi.newpipe.player.PlayerType;
 import org.schabi.newpipe.player.event.PlayerServiceEventListener;
-import org.schabi.newpipe.player.event.PlayerServiceExtendedEventListener;
+import org.schabi.newpipe.player.event.PlayerHolderLifecycleEventListener;
 import org.schabi.newpipe.player.playqueue.PlayQueue;
 
+/**
+ * Singleton that manages a `PlayerService`
+ * and can be used to control the player instance through the service.
+ */
 public final class PlayerHolder {
 
     private PlayerHolder() {
@@ -39,10 +44,12 @@ public final class PlayerHolder {
     private static final boolean DEBUG = MainActivity.DEBUG;
     private static final String TAG = PlayerHolder.class.getSimpleName();
 
-    @Nullable private PlayerServiceExtendedEventListener listener;
+    @Nullable private PlayerServiceEventListener listener;
+    @Nullable private PlayerHolderLifecycleEventListener holderListener;
 
     private final PlayerServiceConnection serviceConnection = new PlayerServiceConnection();
     private boolean bound;
+
     @Nullable private PlayerService playerService;
     @Nullable private Player player;
 
@@ -80,8 +87,8 @@ public final class PlayerHolder {
         return player != null && player.getPlayQueue() != null;
     }
 
-    public boolean isBound() {
+    public boolean isNotBoundYet() {
-        return bound;
+        return !bound;
     }
 
     public int getQueueSize() {
@@ -99,30 +106,48 @@ public final class PlayerHolder {
         return player.getPlayQueue().getIndex();
     }
 
-    public void setListener(@Nullable final PlayerServiceExtendedEventListener newListener) {
+    public void unsetListeners() {
-        listener = newListener;
+        listener = null;
-
+        holderListener = null;
-        if (listener == null) {
-            return;
     }
 
+    public void setListener(@NonNull final PlayerServiceEventListener newListener,
+                            @NonNull final PlayerHolderLifecycleEventListener newHolderListener) {
+        listener = newListener;
+        holderListener = newHolderListener;
+
         // Force reload data from service
         if (player != null) {
-            listener.onServiceConnected(player, playerService, false);
+            holderListener.onServiceConnected(playerService, false);
-            startPlayerListener();
+            player.setFragmentListener(internalListener);
         }
     }
 
-    // helper to handle context in common place as using the same
+    /**
-    // context to bind/unbind a service is crucial
+     * Helper to handle context in common place as using the same
+     * context to bind/unbind a service is crucial.
+     *
+     * @return the common context
+     * */
     private Context getCommonContext() {
         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 the service is started, start playing immediately
+     * @param newListener set this listener
+     * @param newHolderListener set this listener
+     * */
     public void startService(final boolean playAfterConnect,
-                             final PlayerServiceExtendedEventListener newListener) {
+                             final PlayerServiceEventListener newListener,
+                             final PlayerHolderLifecycleEventListener newHolderListener
+    ) {
         final Context context = getCommonContext();
-        setListener(newListener);
+        setListener(newListener, newHolderListener);
         if (bound) {
             return;
         }
@@ -131,8 +156,18 @@ public final class PlayerHolder {
         // bound twice. Prevent it with unbinding first
         unbind(context);
         ContextCompat.startForegroundService(context, new Intent(context, PlayerService.class));
-        serviceConnection.doPlayAfterConnect(playAfterConnect);
+        serviceConnection.playAfterConnect = playAfterConnect;
-        bind(context);
+
+        if (DEBUG) {
+            Log.d(TAG, "bind() called");
+        }
+
+        final Intent serviceIntent = new Intent(context, PlayerService.class);
+        bound = context.bindService(serviceIntent, serviceConnection,
+                Context.BIND_AUTO_CREATE);
+        if (!bound) {
+            context.unbindService(serviceConnection);
+        }
     }
 
     public void stopService() {
@@ -141,14 +176,35 @@ public final class PlayerHolder {
         context.stopService(new Intent(context, PlayerService.class));
     }
 
+    /**
+     * Call {@link Context#unbindService(ServiceConnection)} on our service
+     * (does not necesarily stop the service right away).
+     * Remove all our listeners and deinitialize them.
+     * @param context shared context
+     * */
+    private void unbind(final Context context) {
+        if (DEBUG) {
+            Log.d(TAG, "unbind() called");
+        }
+
+        if (bound) {
+            context.unbindService(serviceConnection);
+            bound = false;
+            if (player != null) {
+                player.removeFragmentListener(internalListener);
+            }
+            playerService = null;
+            player = null;
+            if (holderListener != null) {
+                holderListener.onServiceDisconnected();
+            }
+        }
+    }
+
     class PlayerServiceConnection implements ServiceConnection {
 
         private boolean playAfterConnect = false;
 
-        public void doPlayAfterConnect(final boolean playAfterConnection) {
-            this.playAfterConnect = playAfterConnection;
-        }
-
         @Override
         public void onServiceDisconnected(final ComponentName compName) {
             if (DEBUG) {
@@ -167,56 +223,22 @@ public final class PlayerHolder {
             final PlayerService.LocalBinder localBinder = (PlayerService.LocalBinder) service;
 
             playerService = localBinder.getService();
-            player = localBinder.getPlayer();
+            player = playerService != null ? playerService.getPlayer() : null;
-            if (listener != null) {
-                listener.onServiceConnected(player, playerService, playAfterConnect);
-            }
-            startPlayerListener();
-        }
-    }
 
-    private void bind(final Context context) {
+            if (holderListener != null) {
-        if (DEBUG) {
+                holderListener.onServiceConnected(playerService, playAfterConnect);
-            Log.d(TAG, "bind() called");
             }
-
-        final Intent serviceIntent = new Intent(context, PlayerService.class);
-        bound = context.bindService(serviceIntent, serviceConnection,
-                Context.BIND_AUTO_CREATE);
-        if (!bound) {
-            context.unbindService(serviceConnection);
-        }
-    }
-
-    private void unbind(final Context context) {
-        if (DEBUG) {
-            Log.d(TAG, "unbind() called");
-        }
-
-        if (bound) {
-            context.unbindService(serviceConnection);
-            bound = false;
-            stopPlayerListener();
-            playerService = null;
-            player = null;
-            if (listener != null) {
-                listener.onServiceDisconnected();
-            }
-        }
-    }
-
-    private void startPlayerListener() {
             if (player != null) {
                 player.setFragmentListener(internalListener);
             }
         }
-
-    private void stopPlayerListener() {
-        if (player != null) {
-            player.removeFragmentListener(internalListener);
-        }
     }
 
+    /**
+     * Delegate all {@link PlayerServiceEventListener} events to our current `listener` object.
+     * Only difference is that if {@link PlayerServiceEventListener#onServiceStopped()} is called,
+     * it also calls {@link PlayerHolder#unbind(Context)}.
+     * */
     private final PlayerServiceEventListener internalListener =
             new PlayerServiceEventListener() {
                 @Override