Merge pull request #8731 from Stypox/player-refactor-wrong-video-size

Fix surface view not resizing video correctly + other player fixes

Author: Stypox

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 static VideoDetailFragment getInstance(final int serviceId,
 
     public static VideoDetailFragment getInstanceInCollapsedState() {
         final VideoDetailFragment instance = new VideoDetailFragment();
-        instance.bottomSheetState = BottomSheetBehavior.STATE_COLLAPSED;
+        instance.updateBottomSheetState(BottomSheetBehavior.STATE_COLLAPSED);
         return instance;
     }
 
@@ -503,12 +505,18 @@ public void onClick(final View v) {
                 }
                 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();
@@ -1170,7 +1178,7 @@ public void openVideoPlayer(final boolean directlyFullscreenIfApplicable) {
             // 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();
         }
@@ -1220,7 +1228,7 @@ private void openMainPlayer() {
         }
 
         final PlayQueue queue = setupPlayQueueForIntent(false);
-        addVideoPlayerView();
+        tryAddVideoPlayerView();
 
         final Intent playerIntent = NavigationHelper.getPlayerIntent(requireContext(),
                 PlayerService.class, queue, true, autoPlayEnabled);
@@ -1301,21 +1309,33 @@ private boolean isAutoplayEnabled() {
                 && PlayerHelper.isAutoplayAllowedByUser(requireContext());
     }
 
-    private void addVideoPlayerView() {
-        if (!isPlayerAvailable() || getView() == null) {
-            return;
+    private void tryAddVideoPlayerView() {
+        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();
         }
-        setHeightThumbnail();
 
-        // 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();
-                    }
-                }));
+        // 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;
+            }
+
+            // 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 +1804,7 @@ private void showPlaybackProgress(final long progress, final long duration) {
 
     @Override
     public void onViewCreated() {
-        addVideoPlayerView();
+        tryAddVideoPlayerView();
     }
 
     @Override
@@ -1926,7 +1946,7 @@ public void onFullscreenStateChanged(final boolean fullscreen) {
         }
         scrollToTop();
 
-        addVideoPlayerView();
+        tryAddVideoPlayerView();
     }
 
     @Override
@@ -2272,7 +2292,9 @@ private void setupBottomPlayer() {
 
         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);
@@ -2288,7 +2310,7 @@ private void setupBottomPlayer() {
         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:
@@ -2429,4 +2451,12 @@ public Optional<View> getRoot() {
         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;
+        }
+    }
 }