Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@ interface NotificationDrawer {
sessionToken: MediaSessionCompat.Token,
useEpisodeArtwork: Boolean,
): Notification

fun buildPreparingNotification(
sessionToken: MediaSessionCompat.Token,
): Notification
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,25 @@ class NotificationDrawerImpl @Inject constructor(
.build()
}

override fun buildPreparingNotification(
sessionToken: MediaSessionCompat.Token,
): Notification {
val mediaStyle = MediaStyle()
.setMediaSession(sessionToken)
.setShowCancelButton(true)
.setCancelButtonIntent(stopPendingIntent)

return notificationHelper.playbackChannelBuilder()
.setContentTitle(context.getString(LR.string.loading))
.setDeleteIntent(stopPendingIntent)
.setOnlyAlertOnce(true)
.setShowWhen(false)
.setSmallIcon(IR.drawable.notification)
.setStyle(mediaStyle)
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
.build()
}

private fun loadArtwork(episode: BaseEpisode, useEpisodeArtwork: Boolean): Bitmap? {
val request = imageRequestFactory.create(episode, useEpisodeArtwork)
return when (val result = context.imageLoader.executeBlocking(request)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.app.ForegroundServiceStartNotAllowedException
import android.app.Notification
import android.content.Intent
import android.content.pm.ServiceInfo
import android.os.Build
import android.os.Bundle
import android.support.v4.media.MediaBrowserCompat
Expand All @@ -11,6 +12,7 @@
import android.support.v4.media.session.MediaControllerCompat
import android.support.v4.media.session.MediaSessionCompat
import android.support.v4.media.session.PlaybackStateCompat
import androidx.core.app.ServiceCompat
import androidx.media.MediaBrowserServiceCompat
import au.com.shiftyjelly.pocketcasts.analytics.SourceView
import au.com.shiftyjelly.pocketcasts.preferences.Settings
Expand Down Expand Up @@ -109,6 +111,35 @@
sleepTimerHandler = SleepTimerHandler(sleepTimer, playbackManager, { applicationContext }, this).also { it.observe() }
}

override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
// Action-only: once started via startForegroundService the FGS contract must be honoured even if the flag flips off.
if (intent?.action == MediaSessionManager.ACTION_START_PLAYBACK_FOREGROUND) {
startForegroundEarly()
}
return super.onStartCommand(intent, flags, startId)
}

private fun startForegroundEarly() {
if (isForeground || Util.isAutomotive(this)) return
val mediaSession = playbackManager.mediaSessionManager.mediaSession ?: return
try {
val notification = notificationDrawer.buildPreparingNotification(mediaSession.sessionToken)
ServiceCompat.startForeground(this, Settings.NotificationId.PLAYING.value, notification, ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK)

Check warning

Code scanning / Android Lint

Using inlined constants on older versions Warning

Field requires API level 29 (current min is 24): android.content.pm.ServiceInfo#FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK
isForeground = true
notificationManager.enteredForeground(notification)
playbackManager.mediaSessionManager.setServiceForeground(true)
LogBuffer.i(LogBuffer.TAG_PLAYBACK, "startForeground early (preparing)")
} catch (e: Exception) {
LogBuffer.e(LogBuffer.TAG_PLAYBACK, "startForeground early failed: $e")
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && e is ForegroundServiceStartNotAllowedException) {
val currentValue = settings.getTimesToShowBatteryWarning()
settings.setTimesToShowBatteryWarning(2 + currentValue)
eventHorizon.track(com.automattic.eventhorizon.PlaybackForegroundServiceErrorEvent)
}
stopSelf()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor inconsistency with the Media3 path: here stopSelf() is called unconditionally on failure, whereas PlaybackService.startForegroundEarly() only calls stopSelf() when session?.connectedControllers.isNullOrEmpty(). If a Bluetooth/Auto controller is already connected and driving playback when this early-start fails, an unconditional stopSelf() could tear the session down. Consider guarding this the same way the Media3 path does (only stop when there's nothing actively connected/playing).

}
}

override fun onTaskRemoved(rootIntent: Intent?) {
if (Util.isAutomotive(this)) return
@Suppress("SENSELESS_COMPARISON") // mediaSession becomes nullable in the Media3 integration PR
Expand All @@ -124,6 +155,7 @@
override fun onDestroy() {
super.onDestroy()
isForeground = false
playbackManager.mediaSessionManager.setServiceForeground(false)

disposables.clear()
sleepTimerHandler?.dispose()
Expand Down Expand Up @@ -262,6 +294,7 @@
startForeground(Settings.NotificationId.PLAYING.value, notification)
isForeground = true
notificationManager.enteredForeground(notification)
playbackManager.mediaSessionManager.setServiceForeground(true)
LogBuffer.i(LogBuffer.TAG_PLAYBACK, "startForeground state: $state")
} catch (e: Exception) {
LogBuffer.e(LogBuffer.TAG_PLAYBACK, "attempted startForeground for state: $state, but that threw an exception we caught: $e")
Expand All @@ -283,7 +316,7 @@
val removeNotification = state != PlaybackStateCompat.STATE_PAUSED || settings.hideNotificationOnPause.value
if (removeNotification || isForegroundService) {
val isTransientLoss = playbackManager.playbackStateRelay.blockingFirst().transientLoss
if (isTransientLoss) {
if (isTransientLoss || playbackManager.mediaSessionManager.isSwitchingPlayer) {
return
}

Expand All @@ -296,6 +329,7 @@

stopForeground(if (removeNotification) STOP_FOREGROUND_REMOVE else STOP_FOREGROUND_DETACH)
isForeground = false
playbackManager.mediaSessionManager.setServiceForeground(false)
if (removeNotification) {
notificationManager.cancel(Settings.NotificationId.PLAYING.value)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import android.view.KeyEvent
import androidx.annotation.DrawableRes
import androidx.annotation.MainThread
import androidx.annotation.OptIn
import androidx.core.content.ContextCompat
import androidx.core.content.IntentCompat
import androidx.core.net.toUri
import androidx.media.utils.MediaConstants.PLAYBACK_STATE_EXTRAS_KEY_MEDIA_ID
Expand Down Expand Up @@ -66,16 +67,21 @@ import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.rx2.asObservable
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import timber.log.Timber
import au.com.shiftyjelly.pocketcasts.images.R as IR
import au.com.shiftyjelly.pocketcasts.localization.R as LR
Expand All @@ -95,6 +101,7 @@ class MediaSessionManager(
companion object {
const val EXTRA_TRANSIENT = "pocketcasts_transient_loss"
const val ACTION_NOT_SUPPORTED = "action_not_supported"
const val ACTION_START_PLAYBACK_FOREGROUND = "au.com.shiftyjelly.pocketcasts.action.START_PLAYBACK_FOREGROUND"

// These manufacturers have issues when the skip to next/previous track are missing from the media session.
private val MANUFACTURERS_TO_HIDE_CUSTOM_SKIP_BUTTONS = listOf("mercedes-benz")
Expand Down Expand Up @@ -207,6 +214,16 @@ class MediaSessionManager(
@Volatile
private var forwardingPlayer: PocketCastsForwardingPlayer? = null

private val _isServiceForeground = MutableStateFlow(false)
val isServiceForeground: StateFlow<Boolean> = _isServiceForeground.asStateFlow()

fun setServiceForeground(foreground: Boolean) {
_isServiceForeground.value = foreground
}

@Volatile
var isSwitchingPlayer: Boolean = false

@Volatile
internal var media3Callback: Media3SessionCallback? = null

Expand Down Expand Up @@ -581,21 +598,51 @@ class MediaSessionManager(
}

fun startServiceIfNeeded(context: Context) {
if (needsMedia3Session) {
if (media3Session != null) return
}
val component = resolveMediaBrowserServiceComponent(context)
if (component == null) {
Timber.e("No enabled media browser service found in manifest")
return
}
if (FeatureFlag.isEnabled(Feature.FOREGROUND_BEFORE_PLAYBACK)) {
startServiceForeground(context, component)
return
}
if (needsMedia3Session && media3Session != null) return
try {
context.startService(Intent().setComponent(component))
} catch (e: Exception) {
Timber.e(e, "Failed to start ${component.className}")
}
}

private fun startServiceForeground(context: Context, component: ComponentName): Boolean {
if (_isServiceForeground.value) return true
val intent = Intent(ACTION_START_PLAYBACK_FOREGROUND).setComponent(component)
return try {
ContextCompat.startForegroundService(context, intent)
true
} catch (e: Exception) {
Timber.e(e, "Failed to start foreground service ${component.className}")
setServiceForeground(false)
false
}
}

suspend fun ensureForegroundServiceStarted(context: Context, timeoutMs: Long = 2000L): Boolean {
if (_isServiceForeground.value) return true
val component = resolveMediaBrowserServiceComponent(context)
if (component == null) {
Timber.e("No enabled media browser service found for foreground start")
return false
}
if (!startServiceForeground(context, component)) return false
if (_isServiceForeground.value) return true
return withTimeoutOrNull(timeoutMs) {
isServiceForeground.first { it }
true
} ?: false
}

@OptIn(UnstableApi::class)
private fun observeForMedia3Updates() {
if (isAutomotive) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import com.automattic.eventhorizon.PlaybackContentType
import com.automattic.eventhorizon.PlaybackEpisodeAutoplayedEvent
import com.automattic.eventhorizon.PlaybackEpisodeDurationChangedEvent
import com.automattic.eventhorizon.PlaybackFailedEvent
import com.automattic.eventhorizon.PlaybackForegroundServiceErrorEvent
import com.automattic.eventhorizon.PlaybackPauseEvent
import com.automattic.eventhorizon.PlaybackPlayEvent
import com.automattic.eventhorizon.PlaybackSeekEvent
Expand Down Expand Up @@ -912,6 +913,8 @@ open class PlaybackManager @Inject constructor(
suspend fun stop() {
LogBuffer.i(LogBuffer.TAG_PLAYBACK, "Stopping playback")

mediaSessionManager.isSwitchingPlayer = false

cancelPrefetchNextEpisode()
cancelUpdateTimer()
cancelBufferUpdateTimer()
Expand Down Expand Up @@ -1290,6 +1293,7 @@ open class PlaybackManager @Inject constructor(

@OptIn(UnstableApi::class)
suspend fun onPlayerError(event: PlayerEvent.PlayerError) {
mediaSessionManager.isSwitchingPlayer = false
settings.recordErrorSession()
val episode = getCurrentEpisode()

Expand Down Expand Up @@ -1457,6 +1461,7 @@ open class PlaybackManager @Inject constructor(

fun onPlayerPlaying() {
Timber.i("PlaybackService onPlayerPlaying")
mediaSessionManager.isSwitchingPlayer = false
val episode = getCurrentEpisode() ?: return

playbackStateRelay.blockingFirst().let { playbackState ->
Expand All @@ -1476,6 +1481,7 @@ open class PlaybackManager @Inject constructor(
}

suspend fun onPlayerPaused() {
mediaSessionManager.isSwitchingPlayer = false
withContext(Dispatchers.Main) {
playbackStateRelay.blockingFirst().let { playbackState ->
playbackStateRelay.accept(playbackState.copy(state = PlaybackState.State.PAUSED, lastChangeFrom = LastChangeFrom.OnPlayerPaused.value))
Expand Down Expand Up @@ -2252,6 +2258,15 @@ open class PlaybackManager @Inject constructor(
return
}

// Android 17: reach the foreground service before requesting audio focus, otherwise focus is denied.
if (FeatureFlag.isEnabled(Feature.FOREGROUND_BEFORE_PLAYBACK) && !Util.isAutomotive(application) && player?.isRemote != true) {
val foreground = mediaSessionManager.ensureForegroundServiceStarted(application)
if (!foreground) {
LogBuffer.e(LogBuffer.TAG_PLAYBACK, "Foreground service not ready before playback, proceeding with fallback")
eventHorizon.track(PlaybackForegroundServiceErrorEvent)
}
}
Comment on lines +2261 to +2268

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Foreground promotion (and isSwitchingPlayer) isn't rolled back when play() returns early right after this.

By the time we get here resetPlayer() has already set isSwitchingPlayer = true, and ensureForegroundServiceStarted() has promoted the service to the foreground with the "Loading" notification. But immediately below, if tryToGetAudioFocus() returns false, play() returns and nothing undoes either side effect:

  • the service stays foreground showing a stale "Loading" notification with no audio, and nothing demotes it (there is no stop() on focus denial here — stopAsync(isAudioFocusFailed = true) is only called from the focus-loss listener);
  • isSwitchingPlayer stays true, so the new !isSwitchingPlayer gates in PlaybackService.onUpdateNotification and LegacyPlaybackService.handlePlaybackState will never remove the notification on pause afterwards → the FGS leaks.

Focus can still be denied even after going foreground (another app refusing to yield), so this is reachable. Consider resetting isSwitchingPlayer and demoting the early promotion on the focus-denied return path.


val hasAudioFocus = focusManager.tryToGetAudioFocus()
if (!hasAudioFocus) {
return
Expand Down Expand Up @@ -2337,6 +2352,9 @@ open class PlaybackManager @Inject constructor(
private suspend fun resetPlayer() {
if (resettingPlayer) return
resettingPlayer = true
if (FeatureFlag.isEnabled(Feature.FOREGROUND_BEFORE_PLAYBACK)) {
mediaSessionManager.isSwitchingPlayer = true
}

withContext(Dispatchers.Main) {
player?.stop()
Expand All @@ -2348,7 +2366,10 @@ open class PlaybackManager @Inject constructor(
player = playerManager.createSimplePlayer(this@PlaybackManager::onPlayerEvent)
// Start the service early so it's ready when we install the player later.
// The ExoPlayer doesn't exist yet — SimplePlayer creates it lazily in prepare().
mediaSessionManager.startServiceIfNeeded(application)
// When the flag is on, play() starts the foreground service via the gate instead.
if (!FeatureFlag.isEnabled(Feature.FOREGROUND_BEFORE_PLAYBACK)) {
mediaSessionManager.startServiceIfNeeded(application)
}
Timber.i("Creating media player of type SimplePlayer.")
}
}
Expand Down
Loading
Loading