Some refactorings after review comments

TeamNewPipe · Jul 9, 2022 · 755ab2b · 755ab2b
1 parent 1709ebf
commit 755ab2b
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 60 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
@@ -1311,11 +1311,12 @@ private void addVideoPlayerView() {
         setHeightThumbnail();
 
         // Prevent from re-adding a view multiple times
-        new Handler().post(() -> player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> {
-            playerUi.removeViewFromParent();
-            binding.playerPlaceholder.addView(playerUi.getBinding().getRoot());
-            playerUi.setupVideoSurfaceIfNeeded();
-        }));
+        new Handler(Looper.getMainLooper()).post(() ->
+                player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> {
+                    playerUi.removeViewFromParent();
+                    binding.playerPlaceholder.addView(playerUi.getBinding().getRoot());
+                    playerUi.setupVideoSurfaceIfNeeded();
+                }));
     }
 
     private void removeVideoPlayerView() {

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
@@ -168,7 +168,7 @@ public static Disposable showForPlayQueue(
 
         final List<StreamEntity> streamEntities = Stream.of(player.getPlayQueue())
                 .filter(Objects::nonNull)
-                .flatMap(playQueue -> Objects.requireNonNull(playQueue).getStreams().stream())
+                .flatMap(playQueue -> playQueue.getStreams().stream())
                 .map(StreamEntity::new)
                 .collect(Collectors.toList());
         if (streamEntities.isEmpty()) {

diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java
@@ -33,8 +33,6 @@
 
 /**
  * One service for all players.
- *
- * @author mauriciocolli
  */
 public final class PlayerService extends Service {
     private static final String TAG = PlayerService.class.getSimpleName();
@@ -72,14 +70,16 @@ public int onStartCommand(final Intent intent, final int flags, final int startI
                     + "], flags = [" + flags + "], startId = [" + startId + "]");
         }
 
-        if (!Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
-                || player.getPlayQueue() != null) {
-            // ^ no need to process media button's action if player is not working
+        if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
+                && player.getPlayQueue() == 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
+            return START_NOT_STICKY;
+        }
 
-            player.handleIntent(intent);
-            if (player.getMediaSessionManager() != null) {
-                player.getMediaSessionManager().handleMediaButtonIntent(intent);
-            }
+        player.handleIntent(intent);
+        if (player.getMediaSessionManager() != null) {
+            player.getMediaSessionManager().handleMediaButtonIntent(intent);
         }
 
         return START_NOT_STICKY;
@@ -97,11 +97,6 @@ public void stopForImmediateReusing() {
             // We can't just pause the player here because it will make transition
             // from one stream to a new stream not smooth
             player.smoothStopForImmediateReusing();
-
-            // Notification shows information about old stream but if a user selects
-            // a stream from backStack it's not actual anymore
-            // So we should hide the notification at all.
-            // When autoplay enabled such notification flashing is annoying so skip this case
         }
     }
 

diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java
@@ -75,6 +75,11 @@
 public final class MainPlayerUi extends VideoPlayerUi implements View.OnLayoutChangeListener {
     private static final String TAG = MainPlayerUi.class.getSimpleName();
 
+    // see the Javadoc of calculateMaxEndScreenThumbnailHeight for information
+    private static final int DETAIL_ROOT_MINIMUM_HEIGHT = 85; // dp
+    private static final int DETAIL_TITLE_TEXT_SIZE_TV = 16; // sp
+    private static final int DETAIL_TITLE_TEXT_SIZE_TABLET = 15; // sp
+
     private boolean isFullscreen = false;
     private boolean isVerticalVideo = false;
     private boolean fragmentIsVisible = false;
@@ -262,13 +267,8 @@ protected void setupElementsVisibility() {
         binding.topControls.setClickable(true);
         binding.topControls.setFocusable(true);
 
-        if (isFullscreen) {
-            binding.titleTextView.setVisibility(View.VISIBLE);
-            binding.channelTextView.setVisibility(View.VISIBLE);
-        } else {
-            binding.titleTextView.setVisibility(View.GONE);
-            binding.channelTextView.setVisibility(View.GONE);
-        }
+        binding.titleTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
+        binding.channelTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
     }
 
     @Override
@@ -452,13 +452,12 @@ public void hideSystemUIIfNeeded() {
      * The calculating follows these rules:
      * <ul>
      * <li>
-     *     Show at least stream title and content creator on TVs and tablets
-     *     when in landscape (always the case for TVs) and not in fullscreen mode.
-     *     This requires to have at least <code>85dp</code> free space for {@link R.id.detail_root}
-     *     and additional space for the stream title text size
-     *     ({@link R.id.detail_title_root_layout}).
-     *     The text size is <code>15sp</code> on tablets and <code>16sp</code> on TVs,
-     *     see {@link R.id.titleTextView}.
+     *     Show at least stream title and content creator on TVs and tablets when in landscape
+     *     (always the case for TVs) and not in fullscreen mode. This requires to have at least
+     *     {@link #DETAIL_ROOT_MINIMUM_HEIGHT} free space for {@link R.id.detail_root} and
+     *     additional space for the stream title text size ({@link R.id.detail_title_root_layout}).
+     *     The text size is {@link #DETAIL_TITLE_TEXT_SIZE_TABLET} on tablets and
+     *     {@link #DETAIL_TITLE_TEXT_SIZE_TV} on TVs, see {@link R.id.titleTextView}.
      * </li>
      * <li>
      *     Otherwise, the max thumbnail height is the screen height.
@@ -474,12 +473,12 @@ protected float calculateMaxEndScreenThumbnailHeight(@NonNull final Bitmap bitma
         final int screenHeight = context.getResources().getDisplayMetrics().heightPixels;
 
         if (DeviceUtils.isTv(context) && !isFullscreen()) {
-            final int videoInfoHeight =
-                    DeviceUtils.dpToPx(85, context) + DeviceUtils.spToPx(16, context);
+            final int videoInfoHeight = DeviceUtils.dpToPx(DETAIL_ROOT_MINIMUM_HEIGHT, context)
+                    + DeviceUtils.spToPx(DETAIL_TITLE_TEXT_SIZE_TV, context);
             return Math.min(bitmap.getHeight(), screenHeight - videoInfoHeight);
         } else if (DeviceUtils.isTablet(context) && isLandscape() && !isFullscreen()) {
-            final int videoInfoHeight =
-                    DeviceUtils.dpToPx(85, context) + DeviceUtils.spToPx(15, context);
+            final int videoInfoHeight = DeviceUtils.dpToPx(DETAIL_ROOT_MINIMUM_HEIGHT, context)
+                    + DeviceUtils.spToPx(DETAIL_TITLE_TEXT_SIZE_TABLET, context);
             return Math.min(bitmap.getHeight(), screenHeight - videoInfoHeight);
         } else { // fullscreen player: max height is the device height
             return Math.min(bitmap.getHeight(), screenHeight);
@@ -935,15 +934,9 @@ public void toggleFullscreen() {
         }
         fragmentListener.onFullscreenStateChanged(isFullscreen);
 
-        if (isFullscreen) {
-            binding.titleTextView.setVisibility(View.VISIBLE);
-            binding.channelTextView.setVisibility(View.VISIBLE);
-            binding.playerCloseButton.setVisibility(View.GONE);
-        } else {
-            binding.titleTextView.setVisibility(View.GONE);
-            binding.channelTextView.setVisibility(View.GONE);
-            binding.playerCloseButton.setVisibility(View.VISIBLE);
-        }
+        binding.titleTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
+        binding.channelTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
+        binding.playerCloseButton.setVisibility(isFullscreen ? View.GONE : View.VISIBLE);
         setupScreenRotationButton();
     }
 

diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java
@@ -1,7 +1,6 @@
 package org.schabi.newpipe.player.ui;
 
 import static com.google.android.exoplayer2.Player.REPEAT_MODE_ALL;
-import static com.google.android.exoplayer2.Player.REPEAT_MODE_OFF;
 import static com.google.android.exoplayer2.Player.REPEAT_MODE_ONE;
 import static org.schabi.newpipe.MainActivity.DEBUG;
 import static org.schabi.newpipe.ktx.ViewUtils.animate;
@@ -912,18 +911,12 @@ public void onShuffleClicked() {
     public void onRepeatModeChanged(@RepeatMode final int repeatMode) {
         super.onRepeatModeChanged(repeatMode);
 
-        switch (repeatMode) {
-            case REPEAT_MODE_OFF:
-                binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_off);
-                break;
-            case REPEAT_MODE_ONE:
-                binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_one);
-                break;
-            case REPEAT_MODE_ALL:
-                binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_all);
-                break;
-            default:
-                break; // unreachable
+        if (repeatMode == REPEAT_MODE_ALL) {
+            binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_all);
+        } else if (repeatMode == REPEAT_MODE_ONE) {
+            binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_one);
+        } else /* repeatMode == REPEAT_MODE_OFF */ {
+            binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_off);
         }
     }