Skip to content

Commit 61c1da1

Browse files
StypoxStypox
committed
Some refactorings after review comments
1 parent 3692858 commit 61c1da1

5 files changed

Lines changed: 42 additions & 60 deletions

File tree

‎app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,11 +1311,12 @@ private void addVideoPlayerView() {
13111311
setHeightThumbnail();
13121312

13131313
// Prevent from re-adding a view multiple times
1314-
new Handler().post(() -> player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> {
1315-
playerUi.removeViewFromParent();
1316-
binding.playerPlaceholder.addView(playerUi.getBinding().getRoot());
1317-
playerUi.setupVideoSurfaceIfNeeded();
1318-
}));
1314+
new Handler(Looper.getMainLooper()).post(() ->
1315+
player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> {
1316+
playerUi.removeViewFromParent();
1317+
binding.playerPlaceholder.addView(playerUi.getBinding().getRoot());
1318+
playerUi.setupVideoSurfaceIfNeeded();
1319+
}));
13191320
}
13201321

13211322
private void removeVideoPlayerView() {

‎app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistDialog.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public static Disposable showForPlayQueue(
168168

169169
final List<StreamEntity> streamEntities = Stream.of(player.getPlayQueue())
170170
.filter(Objects::nonNull)
171-
.flatMap(playQueue -> Objects.requireNonNull(playQueue).getStreams().stream())
171+
.flatMap(playQueue -> playQueue.getStreams().stream())
172172
.map(StreamEntity::new)
173173
.collect(Collectors.toList());
174174
if (streamEntities.isEmpty()) {

‎app/src/main/java/org/schabi/newpipe/player/PlayerService.java‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@
3333

3434
/**
3535
* One service for all players.
36-
*
37-
* @author mauriciocolli
3836
*/
3937
public final class PlayerService extends Service {
4038
private static final String TAG = PlayerService.class.getSimpleName();
@@ -72,14 +70,16 @@ public int onStartCommand(final Intent intent, final int flags, final int startI
7270
+ "], flags = [" + flags + "], startId = [" + startId + "]");
7371
}
7472

75-
if (!Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
76-
|| player.getPlayQueue() != null) {
77-
// ^ no need to process media button's action if player is not working
73+
if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
74+
&& player.getPlayQueue() == null) {
75+
// No need to process media button's actions if the player is not working, otherwise the
76+
// player service would strangely start with nothing to play
77+
return START_NOT_STICKY;
78+
}
7879

79-
player.handleIntent(intent);
80-
if (player.getMediaSessionManager() != null) {
81-
player.getMediaSessionManager().handleMediaButtonIntent(intent);
82-
}
80+
player.handleIntent(intent);
81+
if (player.getMediaSessionManager() != null) {
82+
player.getMediaSessionManager().handleMediaButtonIntent(intent);
8383
}
8484

8585
return START_NOT_STICKY;
@@ -97,11 +97,6 @@ public void stopForImmediateReusing() {
9797
// We can't just pause the player here because it will make transition
9898
// from one stream to a new stream not smooth
9999
player.smoothStopForImmediateReusing();
100-
101-
// Notification shows information about old stream but if a user selects
102-
// a stream from backStack it's not actual anymore
103-
// So we should hide the notification at all.
104-
// When autoplay enabled such notification flashing is annoying so skip this case
105100
}
106101
}
107102

‎app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java‎

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@
7575
public final class MainPlayerUi extends VideoPlayerUi implements View.OnLayoutChangeListener {
7676
private static final String TAG = MainPlayerUi.class.getSimpleName();
7777

78+
// see the Javadoc of calculateMaxEndScreenThumbnailHeight for information
79+
private static final int DETAIL_ROOT_MINIMUM_HEIGHT = 85; // dp
80+
private static final int DETAIL_TITLE_TEXT_SIZE_TV = 16; // sp
81+
private static final int DETAIL_TITLE_TEXT_SIZE_TABLET = 15; // sp
82+
7883
private boolean isFullscreen = false;
7984
private boolean isVerticalVideo = false;
8085
private boolean fragmentIsVisible = false;
@@ -262,13 +267,8 @@ protected void setupElementsVisibility() {
262267
binding.topControls.setClickable(true);
263268
binding.topControls.setFocusable(true);
264269

265-
if (isFullscreen) {
266-
binding.titleTextView.setVisibility(View.VISIBLE);
267-
binding.channelTextView.setVisibility(View.VISIBLE);
268-
} else {
269-
binding.titleTextView.setVisibility(View.GONE);
270-
binding.channelTextView.setVisibility(View.GONE);
271-
}
270+
binding.titleTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
271+
binding.channelTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
272272
}
273273

274274
@Override
@@ -450,13 +450,12 @@ public void hideSystemUIIfNeeded() {
450450
* The calculating follows these rules:
451451
* <ul>
452452
* <li>
453-
* Show at least stream title and content creator on TVs and tablets
454-
* when in landscape (always the case for TVs) and not in fullscreen mode.
455-
* This requires to have at least <code>85dp</code> free space for {@link R.id.detail_root}
456-
* and additional space for the stream title text size
457-
* ({@link R.id.detail_title_root_layout}).
458-
* The text size is <code>15sp</code> on tablets and <code>16sp</code> on TVs,
459-
* see {@link R.id.titleTextView}.
453+
* Show at least stream title and content creator on TVs and tablets when in landscape
454+
* (always the case for TVs) and not in fullscreen mode. This requires to have at least
455+
* {@link #DETAIL_ROOT_MINIMUM_HEIGHT} free space for {@link R.id.detail_root} and
456+
* additional space for the stream title text size ({@link R.id.detail_title_root_layout}).
457+
* The text size is {@link #DETAIL_TITLE_TEXT_SIZE_TABLET} on tablets and
458+
* {@link #DETAIL_TITLE_TEXT_SIZE_TV} on TVs, see {@link R.id.titleTextView}.
460459
* </li>
461460
* <li>
462461
* Otherwise, the max thumbnail height is the screen height.
@@ -472,12 +471,12 @@ protected float calculateMaxEndScreenThumbnailHeight(@NonNull final Bitmap bitma
472471
final int screenHeight = context.getResources().getDisplayMetrics().heightPixels;
473472

474473
if (DeviceUtils.isTv(context) && !isFullscreen()) {
475-
final int videoInfoHeight =
476-
DeviceUtils.dpToPx(85, context) + DeviceUtils.spToPx(16, context);
474+
final int videoInfoHeight = DeviceUtils.dpToPx(DETAIL_ROOT_MINIMUM_HEIGHT, context)
475+
+ DeviceUtils.spToPx(DETAIL_TITLE_TEXT_SIZE_TV, context);
477476
return Math.min(bitmap.getHeight(), screenHeight - videoInfoHeight);
478477
} else if (DeviceUtils.isTablet(context) && isLandscape() && !isFullscreen()) {
479-
final int videoInfoHeight =
480-
DeviceUtils.dpToPx(85, context) + DeviceUtils.spToPx(15, context);
478+
final int videoInfoHeight = DeviceUtils.dpToPx(DETAIL_ROOT_MINIMUM_HEIGHT, context)
479+
+ DeviceUtils.spToPx(DETAIL_TITLE_TEXT_SIZE_TABLET, context);
481480
return Math.min(bitmap.getHeight(), screenHeight - videoInfoHeight);
482481
} else { // fullscreen player: max height is the device height
483482
return Math.min(bitmap.getHeight(), screenHeight);
@@ -933,15 +932,9 @@ public void toggleFullscreen() {
933932
}
934933
fragmentListener.onFullscreenStateChanged(isFullscreen);
935934

936-
if (isFullscreen) {
937-
binding.titleTextView.setVisibility(View.VISIBLE);
938-
binding.channelTextView.setVisibility(View.VISIBLE);
939-
binding.playerCloseButton.setVisibility(View.GONE);
940-
} else {
941-
binding.titleTextView.setVisibility(View.GONE);
942-
binding.channelTextView.setVisibility(View.GONE);
943-
binding.playerCloseButton.setVisibility(View.VISIBLE);
944-
}
935+
binding.titleTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
936+
binding.channelTextView.setVisibility(isFullscreen ? View.VISIBLE : View.GONE);
937+
binding.playerCloseButton.setVisibility(isFullscreen ? View.GONE : View.VISIBLE);
945938
setupScreenRotationButton();
946939
}
947940

‎app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.schabi.newpipe.player.ui;
22

33
import static com.google.android.exoplayer2.Player.REPEAT_MODE_ALL;
4-
import static com.google.android.exoplayer2.Player.REPEAT_MODE_OFF;
54
import static com.google.android.exoplayer2.Player.REPEAT_MODE_ONE;
65
import static org.schabi.newpipe.MainActivity.DEBUG;
76
import static org.schabi.newpipe.ktx.ViewUtils.animate;
@@ -912,18 +911,12 @@ public void onShuffleClicked() {
912911
public void onRepeatModeChanged(@RepeatMode final int repeatMode) {
913912
super.onRepeatModeChanged(repeatMode);
914913

915-
switch (repeatMode) {
916-
case REPEAT_MODE_OFF:
917-
binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_off);
918-
break;
919-
case REPEAT_MODE_ONE:
920-
binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_one);
921-
break;
922-
case REPEAT_MODE_ALL:
923-
binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_all);
924-
break;
925-
default:
926-
break; // unreachable
914+
if (repeatMode == REPEAT_MODE_ALL) {
915+
binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_all);
916+
} else if (repeatMode == REPEAT_MODE_ONE) {
917+
binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_one);
918+
} else /* repeatMode == REPEAT_MODE_OFF */ {
919+
binding.repeatButton.setImageResource(R.drawable.exo_controls_repeat_off);
927920
}
928921
}
929922

0 commit comments

Comments
 (0)