From 7509e6482d94ca6d0c35396a2d46005dcd349c98 Mon Sep 17 00:00:00 2001 From: Siddhesh Naik Date: Sat, 12 Oct 2024 21:04:45 +0530 Subject: [PATCH] Addressed review comments --- .../fragments/detail/VideoDetailFragment.java | 3 ++- .../newpipe/local/dialog/PlaylistDialog.java | 2 +- .../newpipe/player/PlayQueueActivity.java | 27 ++++++++++++------- .../schabi/newpipe/player/PlayerService.kt | 8 ++---- .../PlayerServiceExtendedEventListener.java | 4 ++- .../newpipe/player/helper/PlayerHolder.java | 2 +- .../mediabrowser/MediaBrowserConnector.kt | 9 +++++-- 7 files changed, 34 insertions(+), 21 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 95b54f65a..fe0c11ee1 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 @@ -229,6 +229,7 @@ public final class VideoDetailFragment private ContentObserver settingsContentObserver; @Nullable private PlayerService playerService; + @Nullable private Player player; private final PlayerHolder playerHolder = PlayerHolder.getInstance(); @@ -236,7 +237,7 @@ public final class VideoDetailFragment // Service management //////////////////////////////////////////////////////////////////////////*/ @Override - public void onServiceConnected(final Player connectedPlayer, + public void onServiceConnected(@Nullable final Player connectedPlayer, final PlayerService connectedPlayerService, final boolean playAfterConnect) { player = connectedPlayer; diff --git a/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistDialog.java b/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistDialog.java index 612c38181..da408bb50 100644 --- a/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistDialog.java +++ b/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistDialog.java @@ -163,7 +163,7 @@ public abstract class PlaylistDialog extends DialogFragment implements StateSave * @return the disposable that was created */ public static Disposable showForPlayQueue( - final Player player, + @NonNull final Player player, @NonNull final FragmentManager fragmentManager) { final List streamEntities = Stream.of(player.getPlayQueue()) 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 195baecbd..dc959afea 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayQueueActivity.java +++ b/app/src/main/java/org/schabi/newpipe/player/PlayQueueActivity.java @@ -61,6 +61,7 @@ public final class PlayQueueActivity extends AppCompatActivity private static final int MENU_ID_AUDIO_TRACK = 71; + @Nullable private Player player; private boolean serviceBound; @@ -137,30 +138,38 @@ public final class PlayQueueActivity extends AppCompatActivity NavigationHelper.openSettings(this); return true; case R.id.action_append_playlist: - PlaylistDialog.showForPlayQueue(player, getSupportFragmentManager()); + if (player != null) { + PlaylistDialog.showForPlayQueue(player, getSupportFragmentManager()); + } return true; case R.id.action_playback_speed: openPlaybackParameterDialog(); return true; case R.id.action_mute: - player.toggleMute(); + if (player != null) { + player.toggleMute(); + } return true; case R.id.action_system_audio: startActivity(new Intent(Settings.ACTION_SOUND_SETTINGS)); return true; case R.id.action_switch_main: - this.player.setRecovery(); - NavigationHelper.playOnMainPlayer(this, player.getPlayQueue(), true); + if (player != null) { + this.player.setRecovery(); + NavigationHelper.playOnMainPlayer(this, player.getPlayQueue(), true); + } return true; case R.id.action_switch_popup: - if (PermissionHelper.isPopupEnabledElseAsk(this)) { + if (PermissionHelper.isPopupEnabledElseAsk(this) && player != null) { this.player.setRecovery(); NavigationHelper.playOnPopupPlayer(this, player.getPlayQueue(), true); } return true; case R.id.action_switch_background: - this.player.setRecovery(); - NavigationHelper.playOnBackgroundPlayer(this, player.getPlayQueue(), true); + if (player != null) { + this.player.setRecovery(); + NavigationHelper.playOnBackgroundPlayer(this, player.getPlayQueue(), true); + } return true; } @@ -309,7 +318,7 @@ public final class PlayQueueActivity extends AppCompatActivity @Override public void onSwiped(final int index) { - if (index != -1) { + if (index != -1 && player != null) { player.getPlayQueue().remove(index); } } @@ -659,7 +668,7 @@ public final class PlayQueueActivity extends AppCompatActivity * @param itemId index of the selected item */ private void onAudioTrackClick(final int itemId) { - if (player.getCurrentMetadata() == null) { + if (player == null || player.getCurrentMetadata() == null) { return; } player.getCurrentMetadata().getMaybeAudioTrack().ifPresent(audioTrack -> { 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 112565aed..c9cb8f25e 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.kt @@ -46,8 +46,7 @@ class PlayerService : MediaBrowserServiceCompat() { 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() - .get(NotificationPlayerUi::class.java) + UIs()[NotificationPlayerUi::class.java] .ifPresent { it.createNotificationAndStartForeground() } } } @@ -198,10 +197,7 @@ class PlayerService : MediaBrowserServiceCompat() { ) : Binder() { private val playerService = WeakReference(playerService) - val service: PlayerService? - get() = playerService.get() - - fun getPlayer(): Player = service?.player ?: throw Error("Player service is null") + fun getPlayer(): Player? = playerService.get()?.player } companion object { diff --git a/app/src/main/java/org/schabi/newpipe/player/event/PlayerServiceExtendedEventListener.java b/app/src/main/java/org/schabi/newpipe/player/event/PlayerServiceExtendedEventListener.java index 8effe2f0e..15852088b 100644 --- a/app/src/main/java/org/schabi/newpipe/player/event/PlayerServiceExtendedEventListener.java +++ b/app/src/main/java/org/schabi/newpipe/player/event/PlayerServiceExtendedEventListener.java @@ -1,10 +1,12 @@ package org.schabi.newpipe.player.event; +import androidx.annotation.Nullable; + import org.schabi.newpipe.player.PlayerService; import org.schabi.newpipe.player.Player; public interface PlayerServiceExtendedEventListener extends PlayerServiceEventListener { - void onServiceConnected(Player player, + void onServiceConnected(@Nullable Player player, PlayerService playerService, boolean playAfterConnect); 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 b55a6547a..ba4fb3772 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 @@ -166,7 +166,7 @@ public final class PlayerHolder { } final PlayerService.LocalBinder localBinder = (PlayerService.LocalBinder) service; - playerService = localBinder.getService(); + playerService = localBinder.getPlayer().getService(); player = localBinder.getPlayer(); if (listener != null) { listener.onServiceConnected(player, playerService, playAfterConnect); 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 7e43305af..659a44868 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 @@ -59,7 +59,9 @@ import org.schabi.newpipe.util.NavigationHelper import org.schabi.newpipe.util.ServiceHelper import java.util.stream.Collectors -class MediaBrowserConnector(private val playerService: PlayerService) : PlaybackPreparer { +class MediaBrowserConnector( + private val playerService: PlayerService, +) : PlaybackPreparer { private val mediaSession = MediaSessionCompat(playerService, TAG) val sessionConnector = MediaSessionConnector(mediaSession).apply { setMetadataDeduplicationEnabled(true) @@ -627,7 +629,10 @@ class MediaBrowserConnector(private val playerService: PlayerService) : Playback private fun handleSearchError(throwable: Throwable) { Log.e(TAG, "Search error: $throwable") disposePrepareOrPlayCommands() - playbackError(R.string.content_not_supported, PlaybackStateCompat.ERROR_CODE_NOT_SUPPORTED) + sessionConnector.setCustomErrorMessage( + playerService.getString(R.string.search_no_results), + PlaybackStateCompat.ERROR_CODE_APP_ERROR, + ) } override fun onPrepareFromUri(