From 9d750edf5f30d1e3afc3275bf1988b4dd46a89a3 Mon Sep 17 00:00:00 2001 From: Siddhesh Naik Date: Sat, 7 Dec 2024 02:08:06 +0530 Subject: [PATCH] Addressed review comments --- .../org/schabi/newpipe/player/Player.java | 12 ++-- .../schabi/newpipe/player/PlayerService.kt | 56 ++++++++----------- .../mediabrowser/MediaBrowserConnector.kt | 10 ++-- 3 files changed, 36 insertions(+), 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 index 95692cbed..b19df82fa 100644 --- 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.player.notification.NotificationConstants.ACTIO 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.databinding.PlayerBinding; 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.player.ui.VideoPlayerUi; 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 final class Player implements PlaybackListener, Listener { == 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 index 82155ee66..4ccda0c17 100644 --- 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 index 659a44868..322875335 100644 --- 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, ) }