Skip to content

Fix ghost notifications and fix "did not then call startForeground()" #12606

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.

Sign up for GitHub

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

Merged
Stypox merged 2 commits into from
Sep 30, 2025
Merged

Fix ghost notifications and fix "did not then call startForeground()" #12606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92a07a3
Use tryBindIfNeeded(), send player started only if player!=null
Stypox Sep 5, 2025
aa2b482
Post dummy notification then close player service on invalid intent
Stypox Sep 17, 2025
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
6 changes: 2 additions & 4 deletions app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1416,10 +1416,8 @@ 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.isBound()) {
playerHolder.startService(
false, VideoDetailFragment.this);
}
playerHolder.setListener(VideoDetailFragment.this);
playerHolder.tryBindIfNeeded(context);
break;
}
}
Expand Down
30 changes: 15 additions & 15 deletions app/src/main/java/org/schabi/newpipe/player/PlayerService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.schabi.newpipe.player.mediabrowser.MediaBrowserPlaybackPreparer;
import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
import org.schabi.newpipe.player.notification.NotificationPlayerUi;
import org.schabi.newpipe.player.notification.NotificationUtil;
import org.schabi.newpipe.util.ThemeHelper;

import java.lang.ref.WeakReference;
Expand Down Expand Up @@ -156,25 +157,24 @@ public int onStartCommand(final Intent intent, final int flags, final int startI
}
}

if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
&& (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
*/
if (player == null) {
// No need to process media button's actions or other system intents if the player is
// not running. However, since the current intent might have been issued by the system
// with `startForegroundService()` (for unknown reasons), we need to ensure that we post
// a (dummy) foreground notification, otherwise we'd incur in
// "Context.startForegroundService() did not then call Service.startForeground()". Then
// we stop the service again.
Log.d(TAG, "onStartCommand() got a useless intent, closing the service");
NotificationUtil.startForegroundWithDummyNotification(this);
destroyPlayerAndStopService();
return START_NOT_STICKY;
}

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

return START_NOT_STICKY;
}
Expand Down
8 changes: 5 additions & 3 deletions app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,11 @@ public void onServiceConnected(final ComponentName compName, final IBinder servi
startPlayerListener();
// ^ will call listener.onPlayerConnected() down the line if there is an active player

// notify the main activity that binding the service has completed, so that it can
// open the bottom mini-player
NavigationHelper.sendPlayerStartedEvent(localBinder.getService());
if (playerService != null && playerService.getPlayer() != null) {
// notify the main activity that binding the service has completed and that there is
// a player, so that it can open the bottom mini-player
NavigationHelper.sendPlayerStartedEvent(localBinder.getService());
}
}
}

Expand Down
61 changes: 40 additions & 21 deletions app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import static org.schabi.newpipe.player.notification.NotificationConstants.ACTION_CLOSE;

import android.annotation.SuppressLint;
import android.app.Notification;
import android.app.PendingIntent;
import android.content.Context;
import android.content.Intent;
import android.content.pm.ServiceInfo;
import android.graphics.Bitmap;
Expand All @@ -24,6 +26,7 @@
import org.schabi.newpipe.R;
import org.schabi.newpipe.player.Player;
import org.schabi.newpipe.player.PlayerIntentType;
import org.schabi.newpipe.player.PlayerService;
import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
import org.schabi.newpipe.util.NavigationHelper;

Expand Down Expand Up @@ -90,12 +93,9 @@ private synchronized NotificationCompat.Builder createNotification() {
Log.d(TAG, "createNotification()");
}
notificationManager = NotificationManagerCompat.from(player.getContext());
final NotificationCompat.Builder builder =
new NotificationCompat.Builder(player.getContext(),
player.getContext().getString(R.string.notification_channel_id));
final MediaStyle mediaStyle = new MediaStyle();

// setup media style (compact notification slots and media session)
final MediaStyle mediaStyle = new MediaStyle();
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
// notification actions are ignored on Android 13+, and are replaced by code in
// MediaSessionPlayerUi
Expand All @@ -108,18 +108,9 @@ private synchronized NotificationCompat.Builder createNotification() {
.ifPresent(mediaStyle::setMediaSession);

// setup notification builder
builder.setStyle(mediaStyle)
.setPriority(NotificationCompat.PRIORITY_HIGH)
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
.setCategory(NotificationCompat.CATEGORY_TRANSPORT)
.setShowWhen(false)
.setSmallIcon(R.drawable.ic_newpipe_triangle_white)
.setColor(ContextCompat.getColor(player.getContext(),
R.color.dark_background_color))
final var builder = setupNotificationBuilder(player.getContext(), mediaStyle)
.setColorized(player.getPrefs().getBoolean(
player.getContext().getString(R.string.notification_colorize_key), true))
.setDeleteIntent(PendingIntentCompat.getBroadcast(player.getContext(),
NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
player.getContext().getString(R.string.notification_colorize_key), true));

// set the initial value for the video thumbnail, updatable with updateNotificationThumbnail
setLargeIcon(builder);
Expand Down Expand Up @@ -168,17 +159,17 @@ public boolean shouldUpdateBufferingSlot() {
&& notificationBuilder.mActions.get(2).actionIntent != null);
}

public static void startForegroundWithDummyNotification(final PlayerService service) {
final var builder = setupNotificationBuilder(service, new MediaStyle());
startForeground(service, builder.build());
}

public void createNotificationAndStartForeground() {
if (notificationBuilder == null) {
notificationBuilder = createNotification();
}
updateNotification();

// ServiceInfo constants are not used below Android Q, so 0 is set here
final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
ServiceCompat.startForeground(player.getService(), NOTIFICATION_ID,
notificationBuilder.build(), serviceType);
startForeground(player.getService(), notificationBuilder.build());
}

public void cancelNotificationAndStopForeground() {
Expand All @@ -192,6 +183,34 @@ public void cancelNotificationAndStopForeground() {
}


/////////////////////////////////////////////////////
// STATIC FUNCTIONS IN COMMON BETWEEN DUMMY AND REAL NOTIFICATION
/////////////////////////////////////////////////////

private static NotificationCompat.Builder setupNotificationBuilder(final Context context,
final MediaStyle style) {
return new NotificationCompat.Builder(context,
context.getString(R.string.notification_channel_id))
.setStyle(style)
.setPriority(NotificationCompat.PRIORITY_HIGH)
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
.setCategory(NotificationCompat.CATEGORY_TRANSPORT)
.setShowWhen(false)
.setSmallIcon(R.drawable.ic_newpipe_triangle_white)
.setColor(ContextCompat.getColor(context, R.color.dark_background_color))
.setDeleteIntent(PendingIntentCompat.getBroadcast(context,
NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
}

private static void startForeground(final PlayerService service,
final Notification notification) {
// ServiceInfo constants are not used below Android Q, so 0 is set here
final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
ServiceCompat.startForeground(service, NOTIFICATION_ID, notification, serviceType);
}


/////////////////////////////////////////////////////
// ACTIONS
/////////////////////////////////////////////////////
Expand Down