Restore player service start handling before player UI separation

This behavior was present before 0.24.0 and the player UI separation and
avoided crashes for which their exception contained
"Context.startForegroundService() did not then call Service.startForeground()".

Some player nullability checks have been also added, and the player service is
now stopped when it has been started from a media button and there is nothing
to play.
This commit is contained in:
AudricV 2023-09-17 20:49:13 +02:00
parent 6d694518fe
commit 84d50da009
No known key found for this signature in database
GPG Key ID: DA92EC7905614198
2 changed files with 43 additions and 18 deletions

View File

@ -29,6 +29,7 @@ import android.os.IBinder;
import android.util.Log; import android.util.Log;
import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi; import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
import org.schabi.newpipe.player.notification.NotificationPlayerUi;
import org.schabi.newpipe.util.ThemeHelper; import org.schabi.newpipe.util.ThemeHelper;
import java.lang.ref.WeakReference; import java.lang.ref.WeakReference;
@ -59,6 +60,14 @@ public final class PlayerService extends Service {
ThemeHelper.setTheme(this); ThemeHelper.setTheme(this);
player = new Player(this); player = new Player(this);
/*
Create the player notification and start immediately the service in foreground,
otherwise if nothing is played or initializing the player and its components (especially
loading stream metadata) takes a lot of time, the app would crash on Android 8+ as the
service would never be put in the foreground while we said to the system we would do so
*/
player.UIs().get(NotificationPlayerUi.class)
.ifPresent(NotificationPlayerUi::createNotificationAndStartForeground);
} }
@Override @Override
@ -68,16 +77,38 @@ public final class PlayerService extends Service {
+ "], flags = [" + flags + "], startId = [" + startId + "]"); + "], flags = [" + flags + "], startId = [" + startId + "]");
} }
/*
Be sure that the player notification is set and the service is started in foreground,
otherwise, the app may crash on Android 8+ as the service would never be put in the
foreground while we said to the system we would do so
The service is always requested to be started in foreground, so always creating a
notification if there is no one already and starting the service in foreground should
not create any issues
If the service is already started in foreground, requesting it to be started shouldn't
do anything
*/
if (player != null) {
player.UIs().get(NotificationPlayerUi.class)
.ifPresent(NotificationPlayerUi::createNotificationAndStartForeground);
}
if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction()) if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
&& player.getPlayQueue() == null) { && (player == null || 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 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
Stop the service in this case, which will be removed from the foreground and its
notification cancelled in its destruction
*/
stopSelf();
return START_NOT_STICKY; return START_NOT_STICKY;
} }
player.handleIntent(intent); if (player != null) {
player.UIs().get(MediaSessionPlayerUi.class) player.handleIntent(intent);
.ifPresent(ui -> ui.handleMediaButtonIntent(intent)); player.UIs().get(MediaSessionPlayerUi.class)
.ifPresent(ui -> ui.handleMediaButtonIntent(intent));
}
return START_NOT_STICKY; return START_NOT_STICKY;
} }
@ -87,7 +118,7 @@ public final class PlayerService extends Service {
Log.d(TAG, "stopForImmediateReusing() called"); Log.d(TAG, "stopForImmediateReusing() called");
} }
if (!player.exoPlayerIsNull()) { if (player != null && !player.exoPlayerIsNull()) {
// Releases wifi & cpu, disables keepScreenOn, etc. // Releases wifi & cpu, disables keepScreenOn, etc.
// We can't just pause the player here because it will make transition // We can't just pause the player here because it will make transition
// from one stream to a new stream not smooth // from one stream to a new stream not smooth
@ -98,7 +129,7 @@ public final class PlayerService extends Service {
@Override @Override
public void onTaskRemoved(final Intent rootIntent) { public void onTaskRemoved(final Intent rootIntent) {
super.onTaskRemoved(rootIntent); super.onTaskRemoved(rootIntent);
if (!player.videoPlayerSelected()) { if (player != null && !player.videoPlayerSelected()) {
return; return;
} }
onDestroy(); onDestroy();

View File

@ -17,7 +17,6 @@ import org.schabi.newpipe.player.helper.PlayerHelper;
import org.schabi.newpipe.player.ui.PlayerUi; import org.schabi.newpipe.player.ui.PlayerUi;
public final class NotificationPlayerUi extends PlayerUi { public final class NotificationPlayerUi extends PlayerUi {
private boolean foregroundNotificationAlreadyCreated = false;
private final NotificationUtil notificationUtil; private final NotificationUtil notificationUtil;
public NotificationPlayerUi(@NonNull final Player player) { public NotificationPlayerUi(@NonNull final Player player) {
@ -25,15 +24,6 @@ public final class NotificationPlayerUi extends PlayerUi {
notificationUtil = new NotificationUtil(player); notificationUtil = new NotificationUtil(player);
} }
@Override
public void initPlayer() {
super.initPlayer();
if (!foregroundNotificationAlreadyCreated) {
notificationUtil.createNotificationAndStartForeground();
foregroundNotificationAlreadyCreated = true;
}
}
@Override @Override
public void destroy() { public void destroy() {
super.destroy(); super.destroy();
@ -122,4 +112,8 @@ public final class NotificationPlayerUi extends PlayerUi {
super.onPlayQueueEdited(); super.onPlayQueueEdited();
notificationUtil.createNotificationIfNeededAndUpdate(false); notificationUtil.createNotificationIfNeededAndUpdate(false);
} }
public void createNotificationAndStartForeground() {
notificationUtil.createNotificationAndStartForeground();
}
} }