Addressed review comments

TeamNewPipe · Dec 7, 2024 · 9d750ed · 9d750ed
1 parent c8ccc60
commit 9d750ed
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 42 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
@@ -45,7 +45,6 @@
 import static org.schabi.newpipe.util.ListHelper.getPopupResolutionIndex;
 import static org.schabi.newpipe.util.ListHelper.getResolutionIndex;
 import static org.schabi.newpipe.util.Localization.assureCorrectAppLanguage;
-import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 import android.content.BroadcastReceiver;
@@ -87,8 +86,8 @@
 import org.schabi.newpipe.error.ErrorInfo;
 import org.schabi.newpipe.error.ErrorUtil;
 import org.schabi.newpipe.error.UserAction;
-import org.schabi.newpipe.extractor.stream.AudioStream;
 import org.schabi.newpipe.extractor.Image;
+import org.schabi.newpipe.extractor.stream.AudioStream;
 import org.schabi.newpipe.extractor.stream.StreamInfo;
 import org.schabi.newpipe.extractor.stream.StreamType;
 import org.schabi.newpipe.extractor.stream.VideoStream;
@@ -119,9 +118,9 @@
 import org.schabi.newpipe.util.DependentPreferenceHelper;
 import org.schabi.newpipe.util.ListHelper;
 import org.schabi.newpipe.util.NavigationHelper;
-import org.schabi.newpipe.util.image.PicassoHelper;
 import org.schabi.newpipe.util.SerializedCache;
 import org.schabi.newpipe.util.StreamTypeUtil;
+import org.schabi.newpipe.util.image.PicassoHelper;
 
 import java.util.List;
 import java.util.Optional;
@@ -416,9 +415,12 @@ public void handleIntent(@NonNull final Intent intent) {
                     == com.google.android.exoplayer2.Player.STATE_IDLE) {
                 simpleExoPlayer.prepare();
             }
+            // Seeks to a specific index and position in the player if the queue index has changed.
             if (playQueue.getIndex() != newQueue.getIndex()) {
-                simpleExoPlayer.seekTo(newQueue.getIndex(),
-                        requireNonNull(newQueue.getItem()).getRecoveryPosition());
+                final PlayQueueItem queueItem = newQueue.getItem();
+                if (queueItem != null) {
+                    simpleExoPlayer.seekTo(newQueue.getIndex(), queueItem.getRecoveryPosition());
+                }
             }
             simpleExoPlayer.setPlayWhenReady(playWhenReady);
 

diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt
@@ -39,28 +39,18 @@ import java.lang.ref.WeakReference
  * One service for all players.
  */
 class PlayerService : MediaBrowserServiceCompat() {
-    private val player: Player by lazy {
-        Player(this).apply {
-            /*
-            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
-             */
-            UIs()[NotificationPlayerUi::class.java].ifPresent {
-                it.createNotificationAndStartForeground()
-            }
-        }
-    }
+    private var player: Player? = null
 
     private val mBinder: IBinder = LocalBinder(this)
-    private val compositeDisposableLoadChildren = CompositeDisposable()
-    private var mediaBrowserConnector: MediaBrowserConnector? = null
+    private val disposables = CompositeDisposable()
+    private var _mediaBrowserConnector: MediaBrowserConnector? = null
+    private val mediaBrowserConnector: MediaBrowserConnector
         get() {
-            if (field == null) {
-                field = MediaBrowserConnector(this)
+            return _mediaBrowserConnector ?: run {
+                val newMediaBrowserConnector = MediaBrowserConnector(this)
+                _mediaBrowserConnector = newMediaBrowserConnector
+                newMediaBrowserConnector
             }
-            return field
         }
 
     val sessionConnector: MediaSessionConnector?
@@ -78,13 +68,14 @@ class PlayerService : MediaBrowserServiceCompat() {
         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()[NotificationPlayerUi::class.java].ifPresent {
+        player!!.UIs()[NotificationPlayerUi::class.java].ifPresent {
             it.createNotificationAndStartForeground()
         }
     }
@@ -112,11 +103,11 @@ class PlayerService : MediaBrowserServiceCompat() {
         If the service is already started in foreground, requesting it to be started shouldn't
         do anything
          */
-        player.UIs()[NotificationPlayerUi::class.java].ifPresent {
+        player?.UIs()?.get(NotificationPlayerUi::class.java)?.ifPresent {
             it.createNotificationAndStartForeground()
         }
 
-        if (Intent.ACTION_MEDIA_BUTTON == intent.action && (player.playQueue == 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
             the player service would strangely start with nothing to play
@@ -127,8 +118,8 @@ class PlayerService : MediaBrowserServiceCompat() {
             return START_NOT_STICKY
         }
 
-        player.handleIntent(intent)
-        player.UIs()[MediaSessionPlayerUi::class.java].ifPresent {
+        player?.handleIntent(intent)
+        player?.UIs()?.get(MediaSessionPlayerUi::class.java)?.ifPresent {
             it.handleMediaButtonIntent(intent)
         }
 
@@ -140,17 +131,17 @@ class PlayerService : MediaBrowserServiceCompat() {
             Log.d(TAG, "stopForImmediateReusing() called")
         }
 
-        if (!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 fun onTaskRemoved(rootIntent: Intent) {
         super.onTaskRemoved(rootIntent)
-        if (!player.videoPlayerSelected()) {
+        if (player != null && !player!!.videoPlayerSelected()) {
             return
         }
         onDestroy()
@@ -166,14 +157,15 @@ class PlayerService : MediaBrowserServiceCompat() {
 
         cleanup()
 
-        mediaBrowserConnector?.release()
-        mediaBrowserConnector = null
+        mediaBrowserConnector.release()
+        _mediaBrowserConnector = null
 
-        compositeDisposableLoadChildren.clear()
+        disposables.clear()
     }
 
     private fun cleanup() {
-        player.destroy()
+        player?.destroy()
+        player = null
     }
 
     fun stopService() {
@@ -187,7 +179,7 @@ class PlayerService : MediaBrowserServiceCompat() {
 
     override fun onBind(intent: Intent): IBinder = mBinder
 
-    // MediaBrowserServiceCompat methods
+    // MediaBrowserServiceCompat methods (they defer function calls to mediaBrowserConnector)
     override fun onGetRoot(
         clientPackageName: String,
         clientUid: Int,
@@ -204,7 +196,7 @@ class PlayerService : MediaBrowserServiceCompat() {
                 it.onLoadChildren(parentId).subscribe { mediaItems ->
                     result.sendResult(mediaItems)
                 }
-            compositeDisposableLoadChildren.add(disposable)
+            disposables.add(disposable)
         }
     }
 

diff --git a/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserConnector.kt b/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserConnector.kt
@@ -125,20 +125,20 @@ class MediaBrowserConnector(
 
     private fun createPlaylistMediaItem(playlist: PlaylistLocalItem): MediaBrowserCompat.MediaItem {
         val builder = MediaDescriptionCompat.Builder()
-        val isRemote = playlist is PlaylistRemoteEntity
-        builder.setMediaId(createMediaIdForInfoItem(isRemote, playlist.uid))
+        builder
+            .setMediaId(createMediaIdForInfoItem(playlist is PlaylistRemoteEntity, playlist.uid))
             .setTitle(playlist.orderingName)
-            .setIconUri(Uri.parse(playlist.thumbnailUrl))
+            .setIconUri(playlist.thumbnailUrl?.let { Uri.parse(it) })
 
         val extras = Bundle()
         extras.putString(
             MediaConstants.DESCRIPTION_EXTRAS_KEY_CONTENT_STYLE_GROUP_TITLE,
-            playerService.resources.getString(R.string.tab_bookmarks)
+            playerService.resources.getString(R.string.tab_bookmarks),
         )
         builder.setExtras(extras)
         return MediaBrowserCompat.MediaItem(
             builder.build(),
-            MediaBrowserCompat.MediaItem.FLAG_BROWSABLE
+            MediaBrowserCompat.MediaItem.FLAG_BROWSABLE,
         )
     }