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

Restore player service start handling before player UI separation and fix some issues in this service #10578

Merged
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
47 changes: 39 additions & 8 deletions app/src/main/java/org/schabi/newpipe/player/PlayerService.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import android.util.Log;

import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
import org.schabi.newpipe.player.notification.NotificationPlayerUi;
import org.schabi.newpipe.util.ThemeHelper;

import java.lang.ref.WeakReference;
Expand Down Expand Up @@ -59,6 +60,14 @@ public void onCreate() {
ThemeHelper.setTheme(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
Expand All @@ -68,16 +77,38 @@ public int onStartCommand(final Intent intent, final int flags, final int startI
+ "], 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())
&& 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
&& (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
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;
}

player.handleIntent(intent);
player.UIs().get(MediaSessionPlayerUi.class)
.ifPresent(ui -> ui.handleMediaButtonIntent(intent));
if (player != null) {
player.handleIntent(intent);
player.UIs().get(MediaSessionPlayerUi.class)
.ifPresent(ui -> ui.handleMediaButtonIntent(intent));
}

return START_NOT_STICKY;
}
Expand All @@ -87,7 +118,7 @@ public void stopForImmediateReusing() {
Log.d(TAG, "stopForImmediateReusing() called");
}

if (!player.exoPlayerIsNull()) {
if (player != null && !player.exoPlayerIsNull()) {
// Releases wifi & cpu, disables keepScreenOn, etc.
// We can't just pause the player here because it will make transition
// from one stream to a new stream not smooth
Expand All @@ -98,7 +129,7 @@ public void stopForImmediateReusing() {
@Override
public void onTaskRemoved(final Intent rootIntent) {
super.onTaskRemoved(rootIntent);
if (!player.videoPlayerSelected()) {
if (player != null && !player.videoPlayerSelected()) {
return;
}
onDestroy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,13 @@
import org.schabi.newpipe.player.ui.PlayerUi;

public final class NotificationPlayerUi extends PlayerUi {
private boolean foregroundNotificationAlreadyCreated = false;
private final NotificationUtil notificationUtil;

public NotificationPlayerUi(@NonNull final Player player) {
super(player);
notificationUtil = new NotificationUtil(player);
}

@Override
public void initPlayer() {
super.initPlayer();
if (!foregroundNotificationAlreadyCreated) {
notificationUtil.createNotificationAndStartForeground();
foregroundNotificationAlreadyCreated = true;
}
}

@Override
public void destroy() {
super.destroy();
Expand Down Expand Up @@ -122,4 +112,8 @@ public void onPlayQueueEdited() {
super.onPlayQueueEdited();
notificationUtil.createNotificationIfNeededAndUpdate(false);
}

public void createNotificationAndStartForeground() {
notificationUtil.createNotificationAndStartForeground();
}
}
Loading