Skip to content

Commit

Permalink
convert PlayerHolder to Singleton, handle context within, bugfix Serv…
Browse files Browse the repository at this point in the history
…iceConnection leak

- bugfix: have ServiceConnection created only once!

- select the context within the PlayerHolder to start, stop, bind or unbind the service
  -> we have to make sure the Service is started AND stopped within the same context
  -> so let PlayerHolder be the one to select the context

- throw RuntimeException if a null listener is set in setListener()
  -> set setListener() to be a private method as no one outside is using it
- Compatibility: use ContextCompat.startForegroundService instead of startService()
  • Loading branch information
evermind-zz committed Jun 25, 2021
1 parent dc6685d commit dc4443a
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 74 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ public final class VideoDetailFragment
@Nullable
private MainPlayer playerService;
private Player player;
private PlayerHolder playerHolder = PlayerHolder.getInstance();

/*//////////////////////////////////////////////////////////////////////////
// Service management
Expand Down Expand Up @@ -360,9 +361,9 @@ public void onDestroy() {
// Stop the service when user leaves the app with double back press
// if video player is selected. Otherwise unbind
if (activity.isFinishing() && isPlayerAvailable() && player.videoPlayerSelected()) {
PlayerHolder.stopService(App.getApp());
playerHolder.stopService();
} else {
PlayerHolder.removeListener();
playerHolder.removeListener();
}

PreferenceManager.getDefaultSharedPreferences(activity)
Expand Down Expand Up @@ -660,10 +661,10 @@ protected void initListeners() {
});

setupBottomPlayer();
if (!PlayerHolder.bound) {
if (!playerHolder.bound) {
setHeightThumbnail();
} else {
PlayerHolder.startService(App.getApp(), false, this);
playerHolder.startService(false, this);
}
}

Expand Down Expand Up @@ -1097,7 +1098,7 @@ private void openPopupPlayer(final boolean append) {

// See UI changes while remote playQueue changes
if (!isPlayerAvailable()) {
PlayerHolder.startService(App.getApp(), false, this);
playerHolder.startService(false, this);
}

toggleFullscreenIfInFullscreenMode();
Expand All @@ -1123,7 +1124,7 @@ public void openVideoPlayer() {
private void openNormalBackgroundPlayer(final boolean append) {
// See UI changes while remote playQueue changes
if (!isPlayerAvailable()) {
PlayerHolder.startService(App.getApp(), false, this);
playerHolder.startService(false, this);
}

final PlayQueue queue = setupPlayQueueForIntent(append);
Expand All @@ -1137,7 +1138,7 @@ private void openNormalBackgroundPlayer(final boolean append) {

private void openMainPlayer() {
if (!isPlayerServiceAvailable()) {
PlayerHolder.startService(App.getApp(), autoPlayEnabled, this);
playerHolder.startService(autoPlayEnabled, this);
return;
}
if (currentInfo == null) {
Expand All @@ -1155,7 +1156,7 @@ private void openMainPlayer() {

final Intent playerIntent = NavigationHelper
.getPlayerIntent(requireContext(), MainPlayer.class, queue, true, autoPlayEnabled);
activity.startService(playerIntent);
ContextCompat.startForegroundService(activity, playerIntent);
}

private void hideMainPlayer() {
Expand Down Expand Up @@ -1373,9 +1374,9 @@ public void onReceive(final Context context, final Intent intent) {
bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED);
}
// Rebound to the service if it was closed via notification or mini player
if (!PlayerHolder.bound) {
PlayerHolder.startService(
App.getApp(), false, VideoDetailFragment.this);
if (!playerHolder.bound) {
playerHolder.startService(
false, VideoDetailFragment.this);
}
break;
}
Expand Down Expand Up @@ -2119,7 +2120,7 @@ private void cleanUp() {
if (currentWorker != null) {
currentWorker.dispose();
}
PlayerHolder.stopService(App.getApp());
playerHolder.stopService();
setInitialData(0, null, "", null);
currentInfo = null;
updateOverlayData(null, null, null);
Expand Down
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
128 changes: 76 additions & 52 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";

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,25 +52,29 @@ 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) {
private void setListener(final PlayerServiceExtendedEventListener newListener) {
if (newListener == null) {
throw new RuntimeException("Listener should never be null");
}

listener = newListener;
// Force reload data from service
if (player != null) {
Expand All @@ -69,14 +83,19 @@ public static void setListener(final PlayerServiceExtendedEventListener newListe
}
}

public static void removeListener() {
public 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 +104,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;
}

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;

private static void bind(final Context context) {
playerService = localBinder.getService();
player = localBinder.getPlayer();
if (listener != null) {
listener.onServiceConnected(player, playerService, playAfterConnect);
}
startPlayerListener();
}
};

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 +179,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 +266,7 @@ public void onServiceStopped() {
if (listener != null) {
listener.onServiceStopped();
}
unbind(App.getApp());
unbind(getCommonContext());
}
};
}
Loading

0 comments on commit dc4443a

Please sign in to comment.