From aaa3e20c5aa16766fff1b3f306e785670eed52a9 Mon Sep 17 00:00:00 2001 From: evermind Date: Tue, 22 Jun 2021 17:24:14 +0200 Subject: [PATCH 1/5] service.onDestroy() should only be called from the system and not manually instead use service.stopService() which inturn calls stopSelf() and triggers hopefully onDestroy() to be called. Eventually we have to make sure that all ServiceConnections are closed to successfully stop the service now! Cleanup within stopService() and not only onDestroy() So we make sure that all listeners can react to onServiceStopped() and close their ServiceConnections. Afterwards the android framework is ready to stop the Service. --- .../java/org/schabi/newpipe/player/MainPlayer.java | 10 +++++++++- .../main/java/org/schabi/newpipe/player/Player.java | 6 +++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java b/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java index 945bc9a04..7a04ec22e 100644 --- a/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java @@ -178,7 +178,10 @@ public final class MainPlayer extends Service { if (DEBUG) { Log.d(TAG, "destroy() called"); } + cleanup(); + } + private void cleanup() { if (player != null) { // Exit from fullscreen when user closes the player via notification if (player.isFullscreen()) { @@ -191,9 +194,14 @@ public final class MainPlayer extends Service { player.stopActivityBinding(); player.removePopupFromView(); player.destroy(); - } + player = null; + } + } + + public void stopService() { NotificationUtil.getInstance().cancelNotificationAndStopForeground(this); + cleanup(); stopSelf(); } 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 0a0ad619f..146134aee 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -857,7 +857,7 @@ public final class Player implements Log.d(TAG, "onPlaybackShutdown() called"); } // destroys the service, which in turn will destroy the player - service.onDestroy(); + service.stopService(); } public void smoothStopPlayer() { @@ -1097,7 +1097,7 @@ public final class Player implements pause(); break; case ACTION_CLOSE: - service.onDestroy(); + service.stopService(); break; case ACTION_PLAY_PAUSE: playPause(); @@ -1498,7 +1498,7 @@ public final class Player implements Objects.requireNonNull(windowManager) .removeView(closeOverlayBinding.getRoot()); closeOverlayBinding = null; - service.onDestroy(); + service.stopService(); } }).start(); } From 22a4a4b2dfa9f9a2984d38151ec95ec802dcb4a8 Mon Sep 17 00:00:00 2001 From: evermind Date: Wed, 23 Jun 2021 07:11:41 +0200 Subject: [PATCH 2/5] move null checks for player and playerService to helper methods - code is easier to read - duplication of code reduced --- .../fragments/detail/VideoDetailFragment.java | 81 +++++++++++-------- 1 file changed, 48 insertions(+), 33 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 170718fb2..248b4de92 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 @@ -359,7 +359,7 @@ public final class VideoDetailFragment // Stop the service when user leaves the app with double back press // if video player is selected. Otherwise unbind - if (activity.isFinishing() && player != null && player.videoPlayerSelected()) { + if (activity.isFinishing() && isPlayerAvailable() && player.videoPlayerSelected()) { PlayerHolder.stopService(App.getApp()); } else { PlayerHolder.removeListener(); @@ -512,7 +512,7 @@ public final class VideoDetailFragment openVideoPlayer(); } - setOverlayPlayPauseImage(player != null && player.isPlaying()); + setOverlayPlayPauseImage(isPlayerAvailable() && player.isPlaying()); break; case R.id.overlay_close_button: bottomSheetBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); @@ -721,7 +721,7 @@ public final class VideoDetailFragment @Override public boolean onKeyDown(final int keyCode) { - return player != null && player.onKeyDown(keyCode); + return isPlayerAvailable() && player.onKeyDown(keyCode); } @Override @@ -731,7 +731,7 @@ public final class VideoDetailFragment } // If we are in fullscreen mode just exit from it via first back press - if (player != null && player.isFullscreen()) { + if (isPlayerAvailable() && player.isFullscreen()) { if (!DeviceUtils.isTablet(activity)) { player.pause(); } @@ -741,7 +741,7 @@ public final class VideoDetailFragment } // If we have something in history of played items we replay it here - if (player != null + if (isPlayerAvailable() && player.getPlayQueue() != null && player.videoPlayerSelected() && player.getPlayQueue().previous()) { @@ -778,7 +778,7 @@ public final class VideoDetailFragment final PlayQueueItem playQueueItem = item.getPlayQueue().getItem(); // Update title, url, uploader from the last item in the stack (it's current now) - final boolean isPlayerStopped = player == null || player.isStopped(); + final boolean isPlayerStopped = !isPlayerAvailable() || player.isStopped(); if (playQueueItem != null && isPlayerStopped) { updateOverlayData(playQueueItem.getTitle(), playQueueItem.getUploader(), playQueueItem.getThumbnailUrl()); @@ -806,7 +806,7 @@ public final class VideoDetailFragment @Nullable final String newUrl, @NonNull final String newTitle, @Nullable final PlayQueue newQueue) { - if (player != null && newQueue != null && playQueue != null + if (isPlayerAvailable() && newQueue != null && playQueue != null && !Objects.equals(newQueue.getItem(), playQueue.getItem())) { // Preloading can be disabled since playback is surely being replaced. player.disablePreloadingOfCurrentTrack(); @@ -982,7 +982,7 @@ public final class VideoDetailFragment .replace(R.id.relatedItemsLayout, RelatedItemsFragment.getInstance(info)) .commitAllowingStateLoss(); binding.relatedItemsLayout.setVisibility( - player != null && player.isFullscreen() ? View.GONE : View.VISIBLE); + isPlayerAvailable() && player.isFullscreen() ? View.GONE : View.VISIBLE); } } @@ -1069,7 +1069,7 @@ public final class VideoDetailFragment // If a user watched video inside fullscreen mode and than chose another player // return to non-fullscreen mode - if (player != null && player.isFullscreen()) { + if (isPlayerAvailable() && player.isFullscreen()) { player.toggleFullscreen(); } @@ -1087,13 +1087,13 @@ public final class VideoDetailFragment } // See UI changes while remote playQueue changes - if (player == null) { + if (!isPlayerAvailable()) { PlayerHolder.startService(App.getApp(), false, this); } // If a user watched video inside fullscreen mode and than chose another player // return to non-fullscreen mode - if (player != null && player.isFullscreen()) { + if (isPlayerAvailable() && player.isFullscreen()) { player.toggleFullscreen(); } @@ -1117,7 +1117,7 @@ public final class VideoDetailFragment private void openNormalBackgroundPlayer(final boolean append) { // See UI changes while remote playQueue changes - if (player == null) { + if (!isPlayerAvailable()) { PlayerHolder.startService(App.getApp(), false, this); } @@ -1131,7 +1131,7 @@ public final class VideoDetailFragment } private void openMainPlayer() { - if (playerService == null) { + if (!isPlayerServiceAvailable()) { PlayerHolder.startService(App.getApp(), autoPlayEnabled, this); return; } @@ -1154,7 +1154,7 @@ public final class VideoDetailFragment } private void hideMainPlayer() { - if (playerService == null + if (!isPlayerServiceAvailable() || playerService.getView() == null || !player.videoPlayerSelected()) { return; @@ -1211,13 +1211,13 @@ public final class VideoDetailFragment private boolean isAutoplayEnabled() { return autoPlayEnabled && !isExternalPlayerEnabled() - && (player == null || player.videoPlayerSelected()) + && (!isPlayerAvailable() || player.videoPlayerSelected()) && bottomSheetState != BottomSheetBehavior.STATE_HIDDEN && PlayerHelper.isAutoplayAllowedByUser(requireContext()); } private void addVideoPlayerView() { - if (player == null || getView() == null) { + if (!isPlayerAvailable() || getView() == null) { return; } @@ -1277,7 +1277,7 @@ public final class VideoDetailFragment final boolean isPortrait = metrics.heightPixels > metrics.widthPixels; requireView().getViewTreeObserver().removeOnPreDrawListener(preDrawListener); - if (player != null && player.isFullscreen()) { + if (isPlayerAvailable() && player.isFullscreen()) { final int height = (isInMultiWindow() ? requireView() : activity.getWindow().getDecorView()).getHeight(); @@ -1300,7 +1300,7 @@ public final class VideoDetailFragment new FrameLayout.LayoutParams( RelativeLayout.LayoutParams.MATCH_PARENT, newHeight)); binding.detailThumbnailImageView.setMinimumHeight(newHeight); - if (player != null) { + if (isPlayerAvailable()) { final int maxHeight = (int) (metrics.heightPixels * MAX_PLAYER_HEIGHT); player.getSurfaceView() .setHeights(newHeight, player.isFullscreen() ? newHeight : maxHeight); @@ -1389,11 +1389,11 @@ public final class VideoDetailFragment //////////////////////////////////////////////////////////////////////////*/ private void restoreDefaultOrientation() { - if (player == null || !player.videoPlayerSelected() || activity == null) { + if (!isPlayerAvailable() || !player.videoPlayerSelected() || activity == null) { return; } - if (player != null && player.isFullscreen()) { + if (isPlayerAvailable() && player.isFullscreen()) { player.toggleFullscreen(); } // This will show systemUI and pause the player. @@ -1435,7 +1435,7 @@ public final class VideoDetailFragment if (binding.relatedItemsLayout != null) { if (showRelatedItems) { binding.relatedItemsLayout.setVisibility( - player != null && player.isFullscreen() ? View.GONE : View.INVISIBLE); + isPlayerAvailable() && player.isFullscreen() ? View.GONE : View.INVISIBLE); } else { binding.relatedItemsLayout.setVisibility(View.GONE); } @@ -1549,7 +1549,7 @@ public final class VideoDetailFragment showMetaInfoInTextView(info.getMetaInfo(), binding.detailMetaInfoTextView, binding.detailMetaInfoSeparator, disposables); - if (player == null || player.isStopped()) { + if (!isPlayerAvailable() || player.isStopped()) { updateOverlayData(info.getName(), info.getUploaderName(), info.getThumbnailUrl()); } @@ -1812,7 +1812,7 @@ public final class VideoDetailFragment if (error.type == ExoPlaybackException.TYPE_SOURCE || error.type == ExoPlaybackException.TYPE_UNEXPECTED) { // Properly exit from fullscreen - if (playerService != null && player.isFullscreen()) { + if (isPlayerAndPlayerServiceAvailable() && player.isFullscreen()) { player.toggleFullscreen(); } hideMainPlayer(); @@ -1832,7 +1832,9 @@ public final class VideoDetailFragment @Override public void onFullscreenStateChanged(final boolean fullscreen) { setupBrightness(); - if (playerService.getView() == null || player.getParentActivity() == null) { + if (!isPlayerAndPlayerServiceAvailable() + || playerService.getView() == null + || player.getParentActivity() == null) { return; } @@ -1955,7 +1957,7 @@ public final class VideoDetailFragment activity.getWindow().getDecorView().setSystemUiVisibility(visibility); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP - && (isInMultiWindow() || (player != null && player.isFullscreen()))) { + && (isInMultiWindow() || (isPlayerAvailable() && player.isFullscreen()))) { activity.getWindow().setStatusBarColor(Color.TRANSPARENT); activity.getWindow().setNavigationBarColor(Color.TRANSPARENT); } @@ -1964,7 +1966,7 @@ public final class VideoDetailFragment // Listener implementation public void hideSystemUiIfNeeded() { - if (player != null + if (isPlayerAvailable() && player.isFullscreen() && bottomSheetBehavior.getState() == BottomSheetBehavior.STATE_EXPANDED) { hideSystemUi(); @@ -1972,7 +1974,7 @@ public final class VideoDetailFragment } private boolean playerIsNotStopped() { - return player != null && !player.isStopped(); + return isPlayerAvailable() && !player.isStopped(); } private void restoreDefaultBrightness() { @@ -1993,7 +1995,7 @@ public final class VideoDetailFragment } final WindowManager.LayoutParams lp = activity.getWindow().getAttributes(); - if (player == null + if (!isPlayerAvailable() || !player.videoPlayerSelected() || !player.isFullscreen() || bottomSheetState != BottomSheetBehavior.STATE_EXPANDED) { @@ -2059,7 +2061,7 @@ public final class VideoDetailFragment } private void replaceQueueIfUserConfirms(final Runnable onAllow) { - @Nullable final PlayQueue activeQueue = player == null ? null : player.getPlayQueue(); + @Nullable final PlayQueue activeQueue = isPlayerAvailable() ? player.getPlayQueue() : null; // Player will have STATE_IDLE when a user pressed back button if (isClearingQueueConfirmationRequired(activity) @@ -2219,7 +2221,7 @@ public final class VideoDetailFragment hideSystemUiIfNeeded(); // Conditions when the player should be expanded to fullscreen if (isLandscape() - && player != null + && isPlayerAvailable() && player.isPlaying() && !player.isFullscreen() && !DeviceUtils.isTablet(activity) @@ -2236,17 +2238,17 @@ public final class VideoDetailFragment // Re-enable clicks setOverlayElementsClickable(true); - if (player != null) { + if (isPlayerAvailable()) { player.closeItemsList(); } setOverlayLook(binding.appBarLayout, behavior, 0); break; case BottomSheetBehavior.STATE_DRAGGING: case BottomSheetBehavior.STATE_SETTLING: - if (player != null && player.isFullscreen()) { + if (isPlayerAvailable() && player.isFullscreen()) { showSystemUi(); } - if (player != null && player.isControlsVisible()) { + if (isPlayerAvailable() && player.isControlsVisible()) { player.hideControls(0, 0); } break; @@ -2310,4 +2312,17 @@ public final class VideoDetailFragment binding.overlayPlayPauseButton.setClickable(enable); binding.overlayCloseButton.setClickable(enable); } + + // helpers to check the state of player and playerService + boolean isPlayerAvailable() { + return (player != null); + } + + boolean isPlayerServiceAvailable() { + return (playerService != null); + } + + boolean isPlayerAndPlayerServiceAvailable() { + return (player != null && playerService != null); + } } From e30a552b6c904969cb47df8f7db7d3a9669c6a42 Mon Sep 17 00:00:00 2001 From: evermind Date: Wed, 23 Jun 2021 16:07:04 +0200 Subject: [PATCH 3/5] remove duplicated code for toggle Fullscreen --- .../fragments/detail/VideoDetailFragment.java | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 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 248b4de92..e1232526d 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 @@ -1059,6 +1059,14 @@ public final class VideoDetailFragment // Play Utils //////////////////////////////////////////////////////////////////////////*/ + private void toggleFullscreenIfInFullscreenMode() { + // If a user watched video inside fullscreen mode and than chose another player + // return to non-fullscreen mode + if (isPlayerAvailable() && player.isFullscreen()) { + player.toggleFullscreen(); + } + } + private void openBackgroundPlayer(final boolean append) { final AudioStream audioStream = currentInfo.getAudioStreams() .get(ListHelper.getDefaultAudioFormat(activity, currentInfo.getAudioStreams())); @@ -1067,11 +1075,7 @@ public final class VideoDetailFragment .getDefaultSharedPreferences(activity) .getBoolean(activity.getString(R.string.use_external_audio_player_key), false); - // If a user watched video inside fullscreen mode and than chose another player - // return to non-fullscreen mode - if (isPlayerAvailable() && player.isFullscreen()) { - player.toggleFullscreen(); - } + toggleFullscreenIfInFullscreenMode(); if (!useExternalAudioPlayer) { openNormalBackgroundPlayer(append); @@ -1091,11 +1095,7 @@ public final class VideoDetailFragment PlayerHolder.startService(App.getApp(), false, this); } - // If a user watched video inside fullscreen mode and than chose another player - // return to non-fullscreen mode - if (isPlayerAvailable() && player.isFullscreen()) { - player.toggleFullscreen(); - } + toggleFullscreenIfInFullscreenMode(); final PlayQueue queue = setupPlayQueueForIntent(append); if (append) { @@ -1393,9 +1393,8 @@ public final class VideoDetailFragment return; } - if (isPlayerAvailable() && player.isFullscreen()) { - player.toggleFullscreen(); - } + toggleFullscreenIfInFullscreenMode(); + // This will show systemUI and pause the player. // User can tap on Play button and video will be in fullscreen mode again // Note for tablet: trying to avoid orientation changes since it's not easy @@ -1812,9 +1811,7 @@ public final class VideoDetailFragment if (error.type == ExoPlaybackException.TYPE_SOURCE || error.type == ExoPlaybackException.TYPE_UNEXPECTED) { // Properly exit from fullscreen - if (isPlayerAndPlayerServiceAvailable() && player.isFullscreen()) { - player.toggleFullscreen(); - } + toggleFullscreenIfInFullscreenMode(); hideMainPlayer(); } } From 435813355f727f7e1f3a15989593842bdcb1c2e1 Mon Sep 17 00:00:00 2001 From: evermind Date: Wed, 23 Jun 2021 16:53:01 +0200 Subject: [PATCH 4/5] use viewBinding correctly --- .../fragments/detail/VideoDetailFragment.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 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 e1232526d..9d8fd443f 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 @@ -304,7 +304,8 @@ public final class VideoDetailFragment @Override public View onCreateView(@NonNull final LayoutInflater inflater, final ViewGroup container, final Bundle savedInstanceState) { - return inflater.inflate(R.layout.fragment_video_detail, container, false); + binding = FragmentVideoDetailBinding.inflate(inflater, container, false); + return binding.getRoot(); } @Override @@ -355,7 +356,6 @@ public final class VideoDetailFragment @Override public void onDestroy() { super.onDestroy(); - binding = null; // Stop the service when user leaves the app with double back press // if video player is selected. Otherwise unbind @@ -388,6 +388,12 @@ public final class VideoDetailFragment } } + @Override + public void onDestroyView() { + super.onDestroyView(); + binding = null; + } + @Override public void onActivityResult(final int requestCode, final int resultCode, final Intent data) { super.onActivityResult(requestCode, resultCode, data); @@ -586,10 +592,9 @@ public final class VideoDetailFragment // Init //////////////////////////////////////////////////////////////////////////*/ - @Override + @Override // called from onViewCreated in {@link BaseFragment#onViewCreated} protected void initViews(final View rootView, final Bundle savedInstanceState) { super.initViews(rootView, savedInstanceState); - binding = FragmentVideoDetailBinding.bind(rootView); pageAdapter = new TabAdapter(getChildFragmentManager()); binding.viewPager.setAdapter(pageAdapter); From 48c2c156cb694b597700122887504e1db3cf06af Mon Sep 17 00:00:00 2001 From: evermind Date: Thu, 24 Jun 2021 10:00:56 +0200 Subject: [PATCH 5/5] convert PlayerHolder to Singleton, handle context within, bugfix ServiceConnection leak - bugfix: have ServiceConnection created only once! - select the context within the PlayerHolder to start, stop, bind or unbind the service -> we have to make sure the Service is started AND stopped within the same context -> so let PlayerHolder be the one to select the context - remove removeListener() and replace the call with setListener(null) - Compatibility: use ContextCompat.startForegroundService instead of startService() --- .../java/org/schabi/newpipe/MainActivity.java | 2 +- .../org/schabi/newpipe/RouterActivity.java | 2 +- .../fragments/detail/VideoDetailFragment.java | 25 ++-- .../fragments/list/BaseListFragment.java | 2 +- .../list/playlist/PlaylistFragment.java | 2 +- .../schabi/newpipe/local/feed/FeedFragment.kt | 2 +- .../history/StatisticsPlaylistFragment.java | 2 +- .../local/playlist/LocalPlaylistFragment.java | 2 +- .../newpipe/player/helper/PlayerHolder.java | 129 ++++++++++-------- .../schabi/newpipe/util/NavigationHelper.java | 4 +- .../newpipe/util/StreamDialogEntry.java | 2 +- 11 files changed, 98 insertions(+), 76 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/MainActivity.java b/app/src/main/java/org/schabi/newpipe/MainActivity.java index 9bd289376..da3a8cc7d 100644 --- a/app/src/main/java/org/schabi/newpipe/MainActivity.java +++ b/app/src/main/java/org/schabi/newpipe/MainActivity.java @@ -823,7 +823,7 @@ public class MainActivity extends AppCompatActivity { return; } - if (PlayerHolder.isPlayerOpen()) { + if (PlayerHolder.getInstance().isPlayerOpen()) { // if the player is already open, no need for a broadcast receiver openMiniPlayerIfMissing(); } else { diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 0c6165084..c8636c66c 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -453,7 +453,7 @@ public class RouterActivity extends AppCompatActivity { returnList.add(showInfo); returnList.add(videoPlayer); } else { - final MainPlayer.PlayerType playerType = PlayerHolder.getType(); + final MainPlayer.PlayerType playerType = PlayerHolder.getInstance().getType(); if (capabilities.contains(VIDEO) && PlayerHelper.isAutoplayAllowedByUser(context) && playerType == null || playerType == MainPlayer.PlayerType.VIDEO) { 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 9d8fd443f..18e5bb7bf 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 @@ -201,6 +201,7 @@ public final class VideoDetailFragment @Nullable private MainPlayer playerService; private Player player; + private PlayerHolder playerHolder = PlayerHolder.getInstance(); /*////////////////////////////////////////////////////////////////////////// // Service management @@ -360,9 +361,9 @@ public final class VideoDetailFragment // Stop the service when user leaves the app with double back press // if video player is selected. Otherwise unbind if (activity.isFinishing() && isPlayerAvailable() && player.videoPlayerSelected()) { - PlayerHolder.stopService(App.getApp()); + playerHolder.stopService(); } else { - PlayerHolder.removeListener(); + playerHolder.setListener(null); } PreferenceManager.getDefaultSharedPreferences(activity) @@ -660,10 +661,10 @@ public final class VideoDetailFragment }); setupBottomPlayer(); - if (!PlayerHolder.bound) { + if (!playerHolder.bound) { setHeightThumbnail(); } else { - PlayerHolder.startService(App.getApp(), false, this); + playerHolder.startService(false, this); } } @@ -1097,7 +1098,7 @@ public final class VideoDetailFragment // See UI changes while remote playQueue changes if (!isPlayerAvailable()) { - PlayerHolder.startService(App.getApp(), false, this); + playerHolder.startService(false, this); } toggleFullscreenIfInFullscreenMode(); @@ -1123,7 +1124,7 @@ public final class VideoDetailFragment private void openNormalBackgroundPlayer(final boolean append) { // See UI changes while remote playQueue changes if (!isPlayerAvailable()) { - PlayerHolder.startService(App.getApp(), false, this); + playerHolder.startService(false, this); } final PlayQueue queue = setupPlayQueueForIntent(append); @@ -1137,7 +1138,7 @@ public final class VideoDetailFragment private void openMainPlayer() { if (!isPlayerServiceAvailable()) { - PlayerHolder.startService(App.getApp(), autoPlayEnabled, this); + playerHolder.startService(autoPlayEnabled, this); return; } if (currentInfo == null) { @@ -1155,7 +1156,7 @@ public final class VideoDetailFragment final Intent playerIntent = NavigationHelper .getPlayerIntent(requireContext(), MainPlayer.class, queue, true, autoPlayEnabled); - activity.startService(playerIntent); + ContextCompat.startForegroundService(activity, playerIntent); } private void hideMainPlayer() { @@ -1373,9 +1374,9 @@ public final class VideoDetailFragment bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); } // Rebound to the service if it was closed via notification or mini player - if (!PlayerHolder.bound) { - PlayerHolder.startService( - App.getApp(), false, VideoDetailFragment.this); + if (!playerHolder.bound) { + playerHolder.startService( + false, VideoDetailFragment.this); } break; } @@ -2119,7 +2120,7 @@ public final class VideoDetailFragment if (currentWorker != null) { currentWorker.dispose(); } - PlayerHolder.stopService(App.getApp()); + playerHolder.stopService(); setInitialData(0, null, "", null); currentInfo = null; updateOverlayData(null, null, null); diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java index 45436ab6b..ae661cfa3 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java @@ -353,7 +353,7 @@ public abstract class BaseListFragment extends BaseStateFragment final List entries = new ArrayList<>(); - if (PlayerHolder.getType() != null) { + if (PlayerHolder.getInstance().getType() != null) { entries.add(StreamDialogEntry.enqueue); } if (item.getStreamType() == StreamType.AUDIO_STREAM) { diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java index de96905db..824aa2612 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java @@ -144,7 +144,7 @@ public class PlaylistFragment extends BaseListInfoFragment { final ArrayList entries = new ArrayList<>(); - if (PlayerHolder.getType() != null) { + if (PlayerHolder.getInstance().getType() != null) { entries.add(StreamDialogEntry.enqueue); } if (item.getStreamType() == StreamType.AUDIO_STREAM) { diff --git a/app/src/main/java/org/schabi/newpipe/local/feed/FeedFragment.kt b/app/src/main/java/org/schabi/newpipe/local/feed/FeedFragment.kt index 4c1bb0732..c235b22df 100644 --- a/app/src/main/java/org/schabi/newpipe/local/feed/FeedFragment.kt +++ b/app/src/main/java/org/schabi/newpipe/local/feed/FeedFragment.kt @@ -331,7 +331,7 @@ class FeedFragment : BaseStateFragment() { if (context == null || context.resources == null || activity == null) return val entries = ArrayList() - if (PlayerHolder.getType() != null) { + if (PlayerHolder.getInstance().getType() != null) { entries.add(StreamDialogEntry.enqueue) } if (item.streamType == StreamType.AUDIO_STREAM) { diff --git a/app/src/main/java/org/schabi/newpipe/local/history/StatisticsPlaylistFragment.java b/app/src/main/java/org/schabi/newpipe/local/history/StatisticsPlaylistFragment.java index aa871190f..166b9c04e 100644 --- a/app/src/main/java/org/schabi/newpipe/local/history/StatisticsPlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/local/history/StatisticsPlaylistFragment.java @@ -340,7 +340,7 @@ public class StatisticsPlaylistFragment final ArrayList entries = new ArrayList<>(); - if (PlayerHolder.getType() != null) { + if (PlayerHolder.getInstance().getType() != null) { entries.add(StreamDialogEntry.enqueue); } if (infoItem.getStreamType() == StreamType.AUDIO_STREAM) { diff --git a/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java b/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java index cefc63c0d..3edbff45a 100644 --- a/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java @@ -749,7 +749,7 @@ public class LocalPlaylistFragment extends BaseLocalListFragment entries = new ArrayList<>(); - if (PlayerHolder.getType() != null) { + if (PlayerHolder.getInstance().getType() != null) { entries.add(StreamDialogEntry.enqueue); } if (infoItem.getStreamType() == StreamType.AUDIO_STREAM) { 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 da1238c81..68de8ce9f 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 @@ -8,6 +8,7 @@ import android.os.IBinder; import android.util.Log; import androidx.annotation.Nullable; +import androidx.core.content.ContextCompat; import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.PlaybackParameters; @@ -22,18 +23,27 @@ import org.schabi.newpipe.player.event.PlayerServiceExtendedEventListener; import org.schabi.newpipe.player.playqueue.PlayQueue; public final class PlayerHolder { + private PlayerHolder() { } - private static final boolean DEBUG = MainActivity.DEBUG; - private static final String TAG = "PlayerHolder"; + private static PlayerHolder instance; + public static synchronized PlayerHolder getInstance() { + if (PlayerHolder.instance == null) { + PlayerHolder.instance = new PlayerHolder(); + } + return PlayerHolder.instance; + } - private static PlayerServiceExtendedEventListener listener; + private final boolean DEBUG = MainActivity.DEBUG; + private final String TAG = PlayerHolder.class.getSimpleName(); - private static ServiceConnection serviceConnection; - public static boolean bound; - private static MainPlayer playerService; - private static Player player; + private PlayerServiceExtendedEventListener listener; + + private final PlayerServiceConnection serviceConnection = new PlayerServiceConnection(); + public boolean bound; + private MainPlayer playerService; + private Player player; /** * Returns the current {@link MainPlayer.PlayerType} of the {@link MainPlayer} service, @@ -42,26 +52,31 @@ public final class PlayerHolder { * @return Current PlayerType */ @Nullable - public static MainPlayer.PlayerType getType() { + public MainPlayer.PlayerType getType() { if (player == null) { return null; } return player.getPlayerType(); } - public static boolean isPlaying() { + public boolean isPlaying() { if (player == null) { return false; } return player.isPlaying(); } - public static boolean isPlayerOpen() { + public boolean isPlayerOpen() { return player != null; } - public static void setListener(final PlayerServiceExtendedEventListener newListener) { + public void setListener(@Nullable final PlayerServiceExtendedEventListener newListener) { listener = newListener; + + if (listener == null) { + return; + } + // Force reload data from service if (player != null) { listener.onServiceConnected(player, playerService, false); @@ -69,14 +84,15 @@ public final class PlayerHolder { } } - public static void removeListener() { - listener = null; + // helper to handle context in common place as using the same + // context to bind/unbind a service is crucial + private Context getCommonContext() { + return App.getApp(); } - - public static void startService(final Context context, - final boolean playAfterConnect, - final PlayerServiceExtendedEventListener newListener) { + public void startService(final boolean playAfterConnect, + final PlayerServiceExtendedEventListener newListener) { + final Context context = getCommonContext(); setListener(newListener); if (bound) { return; @@ -85,58 +101,65 @@ public final class PlayerHolder { // and NullPointerExceptions inside the service because the service will be // bound twice. Prevent it with unbinding first unbind(context); - context.startService(new Intent(context, MainPlayer.class)); - serviceConnection = getServiceConnection(context, playAfterConnect); + ContextCompat.startForegroundService(context, new Intent(context, MainPlayer.class)); + serviceConnection.doPlayAfterConnect(playAfterConnect); bind(context); } - public static void stopService(final Context context) { + public void stopService() { + final Context context = getCommonContext(); unbind(context); context.stopService(new Intent(context, MainPlayer.class)); } - private static ServiceConnection getServiceConnection(final Context context, - final boolean playAfterConnect) { - return new ServiceConnection() { - @Override - public void onServiceDisconnected(final ComponentName compName) { - if (DEBUG) { - Log.d(TAG, "Player service is disconnected"); - } + class PlayerServiceConnection implements ServiceConnection { - unbind(context); + private boolean playAfterConnect = false; + + public void doPlayAfterConnect(final boolean playAfterConnection) { + this.playAfterConnect = playAfterConnection; + } + + @Override + public void onServiceDisconnected(final ComponentName compName) { + if (DEBUG) { + Log.d(TAG, "Player service is disconnected"); } - @Override - public void onServiceConnected(final ComponentName compName, final IBinder service) { - if (DEBUG) { - Log.d(TAG, "Player service is connected"); - } - final MainPlayer.LocalBinder localBinder = (MainPlayer.LocalBinder) service; + final Context context = getCommonContext(); + unbind(context); + } - playerService = localBinder.getService(); - player = localBinder.getPlayer(); - if (listener != null) { - listener.onServiceConnected(player, playerService, playAfterConnect); - } - startPlayerListener(); + @Override + public void onServiceConnected(final ComponentName compName, final IBinder service) { + if (DEBUG) { + Log.d(TAG, "Player service is connected"); } - }; - } + final MainPlayer.LocalBinder localBinder = (MainPlayer.LocalBinder) service; - private static void bind(final Context context) { + playerService = localBinder.getService(); + player = localBinder.getPlayer(); + if (listener != null) { + listener.onServiceConnected(player, playerService, playAfterConnect); + } + startPlayerListener(); + } + }; + + private void bind(final Context context) { if (DEBUG) { Log.d(TAG, "bind() called"); } final Intent serviceIntent = new Intent(context, MainPlayer.class); - bound = context.bindService(serviceIntent, serviceConnection, Context.BIND_AUTO_CREATE); + bound = context.bindService(serviceIntent, serviceConnection, + Context.BIND_AUTO_CREATE); if (!bound) { context.unbindService(serviceConnection); } } - private static void unbind(final Context context) { + private void unbind(final Context context) { if (DEBUG) { Log.d(TAG, "unbind() called"); } @@ -153,21 +176,19 @@ public final class PlayerHolder { } } - - private static void startPlayerListener() { + private void startPlayerListener() { if (player != null) { - player.setFragmentListener(INNER_LISTENER); + player.setFragmentListener(internalListener); } } - private static void stopPlayerListener() { + private void stopPlayerListener() { if (player != null) { - player.removeFragmentListener(INNER_LISTENER); + player.removeFragmentListener(internalListener); } } - - private static final PlayerServiceEventListener INNER_LISTENER = + private final PlayerServiceEventListener internalListener = new PlayerServiceEventListener() { @Override public void onFullscreenStateChanged(final boolean fullscreen) { @@ -242,7 +263,7 @@ public final class PlayerHolder { if (listener != null) { listener.onServiceStopped(); } - unbind(App.getApp()); + unbind(getCommonContext()); } }; } diff --git a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java index d6e1888e1..f44bd6f9f 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -349,13 +349,13 @@ public final class NavigationHelper { final boolean switchingPlayers) { final boolean autoPlay; - @Nullable final MainPlayer.PlayerType playerType = PlayerHolder.getType(); + @Nullable final MainPlayer.PlayerType playerType = PlayerHolder.getInstance().getType(); if (playerType == null) { // no player open autoPlay = PlayerHelper.isAutoplayAllowedByUser(context); } else if (switchingPlayers) { // switching player to main player - autoPlay = PlayerHolder.isPlaying(); // keep play/pause state + autoPlay = PlayerHolder.getInstance().isPlaying(); // keep play/pause state } else if (playerType == MainPlayer.PlayerType.VIDEO) { // opening new stream while already playing in main player autoPlay = PlayerHelper.isAutoplayAllowedByUser(context); diff --git a/app/src/main/java/org/schabi/newpipe/util/StreamDialogEntry.java b/app/src/main/java/org/schabi/newpipe/util/StreamDialogEntry.java index 50eeef7e7..8bfd428b8 100644 --- a/app/src/main/java/org/schabi/newpipe/util/StreamDialogEntry.java +++ b/app/src/main/java/org/schabi/newpipe/util/StreamDialogEntry.java @@ -39,7 +39,7 @@ public enum StreamDialogEntry { * Info: Add this entry within showStreamDialog. */ enqueue(R.string.enqueue_stream, (fragment, item) -> { - final MainPlayer.PlayerType type = PlayerHolder.getType(); + final MainPlayer.PlayerType type = PlayerHolder.getInstance().getType(); if (type == AUDIO) { NavigationHelper.enqueueOnBackgroundPlayer(fragment.getContext(),