Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert PlayerHolder to Singleton; cleanup in VideoDetailFragment; Player/MainPlayer do not call onDestroy() directly #6566

Merged
merged 5 commits into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/main/java/org/schabi/newpipe/MainActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ private void openMiniPlayerUponPlayerStarted() {
return;
}

if (PlayerHolder.isPlayerOpen()) {
if (PlayerHolder.getInstance().isPlayerOpen()) {
// if the player is already open, no need for a broadcast receiver
openMiniPlayerIfMissing();
} else {
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/org/schabi/newpipe/RouterActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ private List<AdapterChoiceItem> getChoicesForService(final StreamingService serv
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) {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ protected void showStreamDialog(final StreamInfoItem item) {

final List<StreamDialogEntry> entries = new ArrayList<>();

if (PlayerHolder.getType() != null) {
if (PlayerHolder.getInstance().getType() != null) {
entries.add(StreamDialogEntry.enqueue);
}
if (item.getStreamType() == StreamType.AUDIO_STREAM) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ protected void showStreamDialog(final StreamInfoItem item) {

final ArrayList<StreamDialogEntry> entries = new ArrayList<>();

if (PlayerHolder.getType() != null) {
if (PlayerHolder.getInstance().getType() != null) {
entries.add(StreamDialogEntry.enqueue);
}
if (item.getStreamType() == StreamType.AUDIO_STREAM) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ class FeedFragment : BaseStateFragment<FeedState>() {
if (context == null || context.resources == null || activity == null) return

val entries = ArrayList<StreamDialogEntry>()
if (PlayerHolder.getType() != null) {
if (PlayerHolder.getInstance().getType() != null) {
entries.add(StreamDialogEntry.enqueue)
}
if (item.streamType == StreamType.AUDIO_STREAM) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ private void showStreamDialog(final StreamStatisticsEntry item) {

final ArrayList<StreamDialogEntry> entries = new ArrayList<>();

if (PlayerHolder.getType() != null) {
if (PlayerHolder.getInstance().getType() != null) {
entries.add(StreamDialogEntry.enqueue);
}
if (infoItem.getStreamType() == StreamType.AUDIO_STREAM) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ protected void showStreamItemDialog(final PlaylistStreamEntry item) {

final ArrayList<StreamDialogEntry> entries = new ArrayList<>();

if (PlayerHolder.getType() != null) {
if (PlayerHolder.getInstance().getType() != null) {
entries.add(StreamDialogEntry.enqueue);
}
if (infoItem.getStreamType() == StreamType.AUDIO_STREAM) {
Expand Down
8 changes: 8 additions & 0 deletions app/src/main/java/org/schabi/newpipe/player/MainPlayer.java
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,10 @@ public void onDestroy() {
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()) {
Expand All @@ -191,9 +194,14 @@ public void onDestroy() {
player.stopActivityBinding();
player.removePopupFromView();
player.destroy();

player = null;
}
}

public void stopService() {
NotificationUtil.getInstance().cancelNotificationAndStopForeground(this);
cleanup();
stopSelf();
}

Expand Down
6 changes: 3 additions & 3 deletions app/src/main/java/org/schabi/newpipe/player/Player.java
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ public void onPlaybackShutdown() {
Log.d(TAG, "onPlaybackShutdown() called");
}
// destroys the service, which in turn will destroy the player
service.onDestroy();
service.stopService();
}

public void smoothStopPlayer() {
Expand Down Expand Up @@ -1097,7 +1097,7 @@ private void onBroadcastReceived(final Intent intent) {
pause();
break;
case ACTION_CLOSE:
service.onDestroy();
service.stopService();
break;
case ACTION_PLAY_PAUSE:
playPause();
Expand Down Expand Up @@ -1498,7 +1498,7 @@ private void end() {
Objects.requireNonNull(windowManager)
.removeView(closeOverlayBinding.getRoot());
closeOverlayBinding = null;
service.onDestroy();
service.stopService();
}
}).start();
}
Expand Down
129 changes: 75 additions & 54 deletions app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
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;
Expand All @@ -22,18 +23,27 @@
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 final boolean DEBUG = MainActivity.DEBUG;
private final String TAG = PlayerHolder.class.getSimpleName();

private static PlayerServiceExtendedEventListener listener;
private PlayerServiceExtendedEventListener listener;

private static ServiceConnection serviceConnection;
public static boolean bound;
private static MainPlayer playerService;
private static Player player;
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,
Expand All @@ -42,41 +52,47 @@ private 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);
startPlayerListener();
}
}

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;
Expand All @@ -85,58 +101,65 @@ public static void startService(final Context context,
// 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 {

private boolean playAfterConnect = false;

public void doPlayAfterConnect(final boolean playAfterConnection) {
this.playAfterConnect = playAfterConnection;
Stypox marked this conversation as resolved.
Show resolved Hide resolved
}

unbind(context);
@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;

playerService = localBinder.getService();
player = localBinder.getPlayer();
if (listener != null) {
listener.onServiceConnected(player, playerService, playAfterConnect);
}
startPlayerListener();
}
};

private static void bind(final Context context) {
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");
}
Expand All @@ -153,21 +176,19 @@ private static void unbind(final Context context) {
}
}


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) {
Expand Down Expand Up @@ -242,7 +263,7 @@ public void onServiceStopped() {
if (listener != null) {
listener.onServiceStopped();
}
unbind(App.getApp());
unbind(getCommonContext());
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -349,13 +349,13 @@ public static void openVideoDetailFragment(@NonNull final Context context,
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down