From 6805c75c9cf90f9448e266eda34866136f0c71c8 Mon Sep 17 00:00:00 2001 From: Stypox Date: Tue, 2 Aug 2022 14:46:11 +0200 Subject: [PATCH 1/4] Fix surface view not resizing video correctly Also fix yet another random null pointer exception that could happen when adding the video player view --- .../fragments/detail/VideoDetailFragment.java | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 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 3b1bdaede..594006ab5 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 @@ -1220,7 +1220,7 @@ public final class VideoDetailFragment } final PlayQueue queue = setupPlayQueueForIntent(false); - addVideoPlayerView(); + tryAddVideoPlayerView(); final Intent playerIntent = NavigationHelper.getPlayerIntent(requireContext(), PlayerService.class, queue, true, autoPlayEnabled); @@ -1301,21 +1301,27 @@ public final class VideoDetailFragment && PlayerHelper.isAutoplayAllowedByUser(requireContext()); } - private void addVideoPlayerView() { - if (!isPlayerAvailable() || getView() == null) { - return; - } - setHeightThumbnail(); + private void tryAddVideoPlayerView() { + // do all the null checks in the posted lambda, since the player, the binding and the view + // could be set or unset before the lambda gets executed on the next main thread cycle + new Handler(Looper.getMainLooper()).post(() -> { + if (!isPlayerAvailable() || getView() == null) { + return; + } - // Prevent from re-adding a view multiple times - new Handler(Looper.getMainLooper()).post(() -> - player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> { - if (binding != null) { - playerUi.removeViewFromParent(); - binding.playerPlaceholder.addView(playerUi.getBinding().getRoot()); - playerUi.setupVideoSurfaceIfNeeded(); - } - })); + // setup the surface view height, so that it fits the video correctly + setHeightThumbnail(); + + player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> { + // sometimes binding would be null here, even though getView() != null above u.u + if (binding != null) { + // prevent from re-adding a view multiple times + playerUi.removeViewFromParent(); + binding.playerPlaceholder.addView(playerUi.getBinding().getRoot()); + playerUi.setupVideoSurfaceIfNeeded(); + } + }); + }); } private void removeVideoPlayerView() { @@ -1784,7 +1790,7 @@ public final class VideoDetailFragment @Override public void onViewCreated() { - addVideoPlayerView(); + tryAddVideoPlayerView(); } @Override @@ -1926,7 +1932,7 @@ public final class VideoDetailFragment } scrollToTop(); - addVideoPlayerView(); + tryAddVideoPlayerView(); } @Override From 500acce178d1e9f21d811f7e9f3b3bfac6c4aeb0 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 24 Aug 2022 15:08:24 +0200 Subject: [PATCH 2/4] Fix regression in screen rotation animation --- .../newpipe/fragments/detail/VideoDetailFragment.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 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 594006ab5..9800b2b0a 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 @@ -1302,8 +1302,14 @@ public final class VideoDetailFragment } private void tryAddVideoPlayerView() { - // do all the null checks in the posted lambda, since the player, the binding and the view - // could be set or unset before the lambda gets executed on the next main thread cycle + if (isPlayerAvailable() && getView() != null) { + // Setup the surface view height, so that it fits the video correctly; this is done also + // here, and not only in the Handler, to avoid a choppy fullscreen rotation animation. + setHeightThumbnail(); + } + + // do all the null checks in the posted lambda, too, since the player, the binding and the + // view could be set or unset before the lambda gets executed on the next main thread cycle new Handler(Looper.getMainLooper()).post(() -> { if (!isPlayerAvailable() || getView() == null) { return; From ca0f56eea815bd2112f2e36655c4f753dd975a70 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 24 Aug 2022 16:03:15 +0200 Subject: [PATCH 3/4] Avoid setting invalid states to bottom sheet callback --- .../fragments/detail/VideoDetailFragment.java | 20 +++++++++++++++---- 1 file changed, 16 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 9800b2b0a..0ec1efe57 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 @@ -180,6 +180,8 @@ public final class VideoDetailFragment @State int bottomSheetState = BottomSheetBehavior.STATE_EXPANDED; @State + int lastStableBottomSheetState = BottomSheetBehavior.STATE_EXPANDED; + @State protected boolean autoPlayEnabled = true; @Nullable @@ -269,7 +271,7 @@ public final class VideoDetailFragment public static VideoDetailFragment getInstanceInCollapsedState() { final VideoDetailFragment instance = new VideoDetailFragment(); - instance.bottomSheetState = BottomSheetBehavior.STATE_COLLAPSED; + instance.updateBottomSheetState(BottomSheetBehavior.STATE_COLLAPSED); return instance; } @@ -1170,7 +1172,7 @@ public final class VideoDetailFragment // doesn't tell which state it was settling to, and thus the bottom sheet settles to // STATE_COLLAPSED. This can be solved by manually setting the state that will be // restored (i.e. bottomSheetState) to STATE_EXPANDED. - bottomSheetState = BottomSheetBehavior.STATE_EXPANDED; + updateBottomSheetState(BottomSheetBehavior.STATE_EXPANDED); // toggle landscape in order to open directly in fullscreen onScreenRotationButtonClicked(); } @@ -2284,7 +2286,9 @@ public final class VideoDetailFragment final FrameLayout bottomSheetLayout = activity.findViewById(R.id.fragment_player_holder); bottomSheetBehavior = BottomSheetBehavior.from(bottomSheetLayout); - bottomSheetBehavior.setState(bottomSheetState); + bottomSheetBehavior.setState(lastStableBottomSheetState); + updateBottomSheetState(lastStableBottomSheetState); + final int peekHeight = getResources().getDimensionPixelSize(R.dimen.mini_player_height); if (bottomSheetState != BottomSheetBehavior.STATE_HIDDEN) { manageSpaceAtTheBottom(false); @@ -2300,7 +2304,7 @@ public final class VideoDetailFragment bottomSheetCallback = new BottomSheetBehavior.BottomSheetCallback() { @Override public void onStateChanged(@NonNull final View bottomSheet, final int newState) { - bottomSheetState = newState; + updateBottomSheetState(newState); switch (newState) { case BottomSheetBehavior.STATE_HIDDEN: @@ -2441,4 +2445,12 @@ public final class VideoDetailFragment return player.UIs().get(VideoPlayerUi.class) .map(playerUi -> playerUi.getBinding().getRoot()); } + + private void updateBottomSheetState(final int newState) { + bottomSheetState = newState; + if (newState != BottomSheetBehavior.STATE_DRAGGING + && newState != BottomSheetBehavior.STATE_SETTLING) { + lastStableBottomSheetState = newState; + } + } } From f9994abb94453f97080bdd48200c6009b965a43c Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 24 Aug 2022 17:48:02 +0200 Subject: [PATCH 4/4] Prevent tapping on thumbnail if video details are not loaded --- .../fragments/detail/VideoDetailFragment.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 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 0ec1efe57..09e085791 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 @@ -505,12 +505,18 @@ public final class VideoDetailFragment } break; case R.id.detail_thumbnail_root_layout: - autoPlayEnabled = true; // forcefully start playing - // FIXME Workaround #7427 - if (isPlayerAvailable()) { - player.setRecovery(); + // make sure not to open any player if there is nothing currently loaded! + // FIXME removing this `if` causes the player service to start correctly, then stop, + // then restart badly without calling `startForeground()`, causing a crash when + // later closing the detail fragment + if (currentInfo != null) { + autoPlayEnabled = true; // forcefully start playing + // FIXME Workaround #7427 + if (isPlayerAvailable()) { + player.setRecovery(); + } + openVideoPlayerAutoFullscreen(); } - openVideoPlayerAutoFullscreen(); break; case R.id.detail_title_root_layout: toggleTitleAndSecondaryControls();