Skip to content

Hold a wake lock during playback to reduce background playback stops#5491

Draft
joashrajin wants to merge 3 commits into
mainfrom
pcdroid-555-android-system-kills-pocket-casts-process-during-background
Draft

Hold a wake lock during playback to reduce background playback stops#5491
joashrajin wants to merge 3 commits into
mainfrom
pcdroid-555-android-system-kills-pocket-casts-process-during-background

Conversation

@joashrajin

@joashrajin joashrajin commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Description

Background playback of a long local Files episode can silently stop on its own (Pixel 6 / Android 16). The logs in PCDROID-555 show the system SIGKILLing the Pocket Casts foreground-service process mid-playback (reason=2 SIGNALED status=9, importance=125), with frequent FREEZER/cached-app activity on surrounding days — not an internal pause or audio-focus loss.

Root cause (closed gap): the background-audio ExoPlayer is built with no wake mode set, so it defaults to WAKE_MODE_NONE and holds no wake lock during playback. That's the missing standard configuration for background audio — without a wake lock the system is free to suspend the CPU / drop Wi-Fi and aggressively freeze/reclaim the process during long uninterrupted playback.

This sets a source-aware wake mode on the one background-audio player (SimplePlayer, shared by both the Media3 and legacy service paths): WAKE_MODE_LOCAL (partial CPU wake lock) for downloaded/local episodes, and WAKE_MODE_NETWORK (partial wake lock + Wi-Fi lock) for streams. isStreaming is known at build time, the WAKE_LOCK permission is already declared, and Media3 ties the lock to the playing state (it's released automatically on pause/stop), so there's no paused-but-locked battery drain.

Honest scope: this is best-effort hardening that closes a real, standard-config gap, not a guaranteed cure. Because the process was still at foreground-service importance (importance=125) when killed, a wake lock reduces the system's freeze/kill eligibility and prevents screen-off CPU suspend, but cannot prove it eliminates every memory-pressure SIGKILL. It should be verified on-device before PCDROID-555 is closed (see Testing). Related to PCDROID-555 (intentionally not auto-closing).

History note for reviewers: wake mode was previously added in #5059 and reverted in #5105 — but there it was bundled with audio-offload (setAudioAttributes() + AudioOffloadPreferences track selection) behind Feature.AUDIO_OFFLOAD, which defaulted to debug-only and so never shipped to production. Audio offload is the risky, device-specific part and the likely reason for that revert. This change re-introduces only the setWakeMode line, decoupled from offload and audio attributes — a strict, benign subset of what was reverted.

Changes

  • SimplePlayer.prepare() — add .setWakeMode(if (isStreaming) C.WAKE_MODE_NETWORK else C.WAKE_MODE_LOCAL) to the ExoPlayer.Builder chain, with an explanatory comment. No manifest, import, or permission changes needed (C already imported; WAKE_LOCK already declared).

Testing Instructions

  • Install this build on a device (preferably Pixel 6 / Android 16).
  • Add a long local Files episode (~45 min), start playback, turn the screen off, and leave the app in the background.
  • Confirm playback continues to the end without silently stopping (the original symptom). Re-test a few times, since the kill was intermittent.
  • While playing with the screen off, confirm the app holds a partial wake lock — e.g. adb shell dumpsys power | grep -i wake shows a lock held by the package during playback.
  • Pause for a while and confirm the wake lock is released (no lock held while paused) — i.e. no idle battery drain.
  • Play a streaming (non-downloaded) episode and confirm a Wi-Fi lock is held during playback (adb shell dumpsys wifi | grep -i lock).
  • After a long background session, check adb shell dumpsys activity exit-info <pkg> and confirm the SIGKILL during playback no longer recurs (or recurs less often). If it still recurs, the residual cause is likely memory pressure — see follow-ups below.
  • Regression sanity: normal play/pause/skip/seek, lock screen controls, and Bluetooth/Android Auto playback behave as before.

Not easily unit-testable: this is ExoPlayer.Builder configuration that the Media3 API doesn't expose for assertion. Behaviour is validated on-device. Follow-ups if the kill persists after verification: onTrimMemory handling in the playback service and moving episode-metadata decode off the player thread (tracked separately, lower confidence).

Screenshots or Screencast

n/a — no UI changes.

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • I have considered whether it makes sense to add tests for my changes

Considered: this is non-asserting ExoPlayer.Builder config (Media3 doesn't expose wake mode for verification), so it's covered by on-device testing rather than a unit test.

  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml — n/a (no strings)
  • Any jetpack compose components I added or changed are covered by compose previews — n/a (no Compose changes)
  • I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics — n/a (no analytics changes)

I have tested any UI changes...

n/a — this PR contains no UI changes.

Set a source-aware ExoPlayer wake mode (WAKE_MODE_LOCAL for downloaded,
WAKE_MODE_NETWORK for streams) in SimplePlayer.prepare() so the player
holds a wake lock for the duration of playback instead of the Media3
default WAKE_MODE_NONE. Without a wake lock the system can suspend the
CPU / drop Wi-Fi and aggressively freeze or reclaim the foreground-service
process during long uninterrupted background playback.

This re-introduces only the setWakeMode line, decoupled from the audio
offload code it was previously bundled with (added in #5059, reverted in
#5105). The WAKE_LOCK permission is already declared.

Related to PCDROID-555.
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Claude finished @joashrajin's task in 1m 6s —— View job


Code Review

I reviewed the single-line change to SimplePlayer.prepare(). This is a clean, well-scoped change with an excellent PR description. Summary: looks correct and safe to merge.

  • Read the diff and surrounding code
  • Verify isStreaming semantics and timing
  • Verify WAKE_LOCK permission coverage across app/automotive/wear
  • Check for regressions / edge cases

Assessment

The change adds .setWakeMode(if (isStreaming) C.WAKE_MODE_NETWORK else C.WAKE_MODE_LOCAL) to the ExoPlayer.Builder chain. This is the standard, recommended Media3 configuration for background audio, and the reasoning in the PR body is sound.

Correctness ✅

  • isStreaming is defined in LocalPlayer as episodeLocation is EpisodeLocation.Stream (LocalPlayer.kt:46), and is already used the same way a few lines above for buffer sizing (SimplePlayer.kt:192-194). Reading it at prepare() time (player build time) is the correct moment — it's stable for the lifetime of that player instance.
  • C is already imported (SimplePlayer.kt:10), so no import changes are needed, as claimed.
  • Media3 ties the wake/Wi-Fi lock to the playing state and releases it automatically on pause/stop, so there's no paused-but-locked drain. Good.

Permissions ✅ (with one thing to confirm)

  • WAKE_LOCK is declared in app (app/src/main/AndroidManifest.xml:16) and wear (wear/src/main/AndroidManifest.xml:13), and also in the localization and analytics service manifests — so it merges in transitively.
  • The automotive manifest does not declare WAKE_LOCK directly, so it relies on manifest merging from a dependency (e.g. localization/analytics). This is almost certainly fine, but since this PR touches the shared repositories player used by the automotive path too, it's worth a quick sanity check that an automotive build still merges the permission. A robust alternative would be to declare WAKE_LOCK in the repositories module manifest itself (the module that actually needs it), making the requirement self-documenting rather than implicit.

Minor observations (non-blocking)

  1. Wake mode is fixed at build time. If an episode transitions from streaming to downloaded mid-session, the wake mode won't change until a new player is prepared. In practice the player is re-prepared on episode/source changes, so this is an acceptable edge case — just noting it.
  2. WAKE_MODE_NETWORK + cellular streaming. When streaming over cellular rather than Wi-Fi, the additional Wi-Fi lock is simply a no-op, so there's no downside. Worth being aware of, nothing to change.
  3. CHANGELOG checklist item is ticked but the diff is a single file. Since this is not user-facing, skipping the CHANGELOG is reasonable — just flagging the mismatch between the checked box and the diff.

Testing
As the PR notes, this isn't unit-testable (Media3 doesn't expose the wake mode for assertion), so the on-device verification checklist in the description is the right approach. The plan to confirm the held/released lock via dumpsys power and dumpsys wifi is solid.

History context
The note about #5059 / #5105 is helpful — decoupling setWakeMode from the reverted audio-offload work (setAudioAttributes + AudioOffloadPreferences) makes this a strict, benign subset of what was previously reverted. That's the right call.


Nice, focused change. The only thing I'd actively suggest before merge is confirming the automotive build still has WAKE_LOCK after merging (and optionally declaring it in repositories). Everything else is informational.

@dangermattic

Copy link
Copy Markdown
Collaborator
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@joashrajin joashrajin added [Type] Bug Not functioning as intended. [Area] Playback Episode playback issue labels Jun 29, 2026
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Claude finished @joashrajin's task in 1m 44s —— View job


Code Review

  • Read PR diff and changed files
  • Verify isStreaming semantics and player lifecycle
  • Check imports/permissions/build-time assumptions
  • Post review

Reviewed the change in SimplePlayer.kt and the CHANGELOG.md entry. This is a solid, minimal, and correct change. It re-introduces only the standard setWakeMode configuration, decoupled from the risky audio-offload work that was reverted in #5105. The PR description is also unusually honest about scope and the need for on-device verification — appreciated.

Correctness ✅

  • isStreaming is stable at build time. It's derived from episodeLocation (LocalPlayer.kt:46-47), which is set via setEpisode() before load()/play(). The ExoPlayer is rebuilt inside prepare() on every fresh prepare cycle (guarded by prepared, reset to false in handleStop() at SimplePlayer.kt:121), so the wake mode is always evaluated for the current source. This is exactly consistent with how the surrounding buffer config already branches on isStreaming (SimplePlayer.kt:192-194), so the change fits the established pattern.
  • No new imports/permissions needed. androidx.media3.common.C is already imported (SimplePlayer.kt:10), and WAKE_LOCK is already declared — verified the claim holds.
  • Lock lifecycle is safe. Media3 ties the wake lock to the playing/ready state and releases it on pause/stop/idle, so there's no paused-but-locked drain. handleStop() also releases the player entirely, dropping the lock.

Minor considerations (non-blocking)

  1. Source-switch edge case. PlaybackManager has logic for a streaming episode that becomes downloaded mid-session (PlaybackManager.kt:1997). As long as that path triggers a stop + fresh prepare() (which it does, since the player is released on stop), the wake mode is re-evaluated correctly. Worth keeping in mind during on-device testing, but I don't see a correctness problem — just flagging it as the one scenario where a stale wake mode would matter if the player were ever reused without rebuilding.
  2. WAKE_MODE_NETWORK Wi-Fi lock on cellular. The Wi-Fi lock portion is effectively a no-op when streaming over cellular, which is the correct/expected behavior — just noting it so the streaming test step is run on Wi-Fi to actually observe the lock.

Testing

Agree with the PR's stance that this isn't meaningfully unit-testable (ExoPlayer.Builder config isn't exposed for assertion). The on-device verification checklist in the description is thorough and the right approach here. The two unchecked checklist items worth completing before merge are the spotlessCheck run and the on-device repro.

No changes requested — looks good to merge once on-device verification and spotlessCheck pass.

@joashrajin joashrajin self-assigned this Jun 30, 2026
SimplePlayer's ExoPlayer wake mode needs WAKE_LOCK. It already merged in
transitively (via the analytics/localization modules), but declaring it in
the module that actually uses it makes the requirement self-documenting and
guarantees coverage for the app, wear, and automotive builds.

Related to PCDROID-555.
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Claude finished @joashrajin's task in 39s —— View job


Code Review

  • Read the updated diff (git diff origin/main...HEAD)
  • Verify the new repositories manifest WAKE_LOCK declaration
  • Re-verify setWakeMode / isStreaming correctness
  • Post review

This update addresses the one actionable item from the previous reviews: WAKE_LOCK is now declared in modules/services/repositories/src/main/AndroidManifest.xml — the module that actually uses it — rather than relying on a transitive merge from unrelated modules (localization/analytics). This makes the requirement self-documenting and guarantees coverage for the automotive build, which didn't declare it directly. 👍

Looks correct and ready to merge (pending the on-device verification the PR description lays out).

Correctness ✅

  • setWakeMode(if (isStreaming) C.WAKE_MODE_NETWORK else C.WAKE_MODE_LOCAL) is the standard Media3 background-audio config. It's evaluated at player build time inside prepare() (SimplePlayer.kt:216), which is correct: isStreaming is stable for the player instance and prepare() rebuilds the player on each fresh prepare cycle, so source changes re-evaluate the wake mode. This matches the existing isStreaming branch used for buffer sizing just above (SimplePlayer.kt:192-194).
  • C is already imported; no import change needed.
  • Media3 ties the wake/Wi-Fi lock to the playing state and releases it on pause/stop/idle, so there's no paused-but-locked drain. handleStop() releases the player entirely.

Manifest declaration ✅

  • The placement and the explanatory comment are good. Declaring the permission in the consuming module is the more robust approach and removes the implicit-merge concern raised earlier.
  • One small consistency note (non-blocking): app and wear still declare WAKE_LOCK in their own manifests. That's harmless — duplicate uses-permission entries merge cleanly — but you could optionally remove the now-redundant declarations there since repositories provides it transitively. Leaving them is also fine and arguably safer (explicit at the app level).

Minor, unchanged from prior reviews (non-blocking)

  1. Source-switch edge case — a stream that becomes downloaded mid-session only picks up the new wake mode on the next prepare(). Since switching triggers a stop + rebuild, this is fine in practice; just one to watch during on-device testing.
  2. WAKE_MODE_NETWORK on cellular — the Wi-Fi lock portion is a no-op off Wi-Fi, which is the expected behavior. Run the streaming verification step on Wi-Fi to actually observe the lock.

Testing

Agree this isn't meaningfully unit-testable (ExoPlayer.Builder config isn't exposed for assertion). The on-device checklist in the description is the right approach — the key boxes to complete before closing PCDROID-555 are the long-background-playback repro and the dumpsys power / dumpsys wifi lock-held/lock-released checks. Also worth running ./gradlew spotlessCheck (still unchecked in the PR checklist).

No changes requested.

@wpmobilebot

Copy link
Copy Markdown
Collaborator

Project manifest changes for automotive

The following changes in the automotive's merged AndroidManifest.xml file were detected (build variant: release):

--- ./build/reports/diff_manifest/automotive/release/base_manifest.txt	2026-06-30 09:30:56.434897196 +0000
+++ ./build/reports/diff_manifest/automotive/release/head_manifest.txt	2026-06-30 09:30:57.860626038 +0000
@@ -54,8 +54,14 @@
 
     <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
     <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
-    <uses-permission android:name="android.permission.INTERNET" />
+    <!--
+         Required by ExoPlayer's wake mode (SimplePlayer) to hold a wake lock
+         during background playback. Declared here, in the module that uses it,
+         so every consumer (app, wear, automotive) gets it without relying on
+         transitive merge from unrelated modules.
+    -->
     <uses-permission android:name="android.permission.WAKE_LOCK" />
+    <uses-permission android:name="android.permission.INTERNET" />
     <uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED" />
     <uses-permission android:name="com.android.vending.BILLING" />
     <uses-permission android:name="com.google.android.finsky.permission.BIND_GET_INSTALL_REFERRER_SERVICE" />

Go to https://buildkite.com/automattic/pocket-casts-android/builds/17359/canvas?sid=019f17dc-44d4-4db9-bb10-c33c876a02af, click on the Artifacts tab and audit the files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Area] Playback Episode playback issue [Type] Bug Not functioning as intended.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants