Resume explicit media play immediately

[joashrajin]

Stack / review order

1. Review and merge prerequisite Harden media button event sequencing #5646 first. It contains the concurrency-safe
   media-button processor and its focused unit tests.
2. Review this PR second. It integrates that processor into the Media3 and legacy
   callbacks and adds the user-facing playback behavior.

This PR is intentionally based on codex/serialize-media-button-events. After
#5646 merges, retarget this PR to main.

Description

KEYCODE_MEDIA_PLAY currently waits for the 600 ms multi-tap window before
resuming playback. When the phone has been idle with the screen off, that
coroutine can remain suspended for several seconds, so Bluetooth play controls
appear unresponsive until the device wakes.

This PR builds on #5646 and supersedes #5535:

• Wire the shared ordered button processor into both Media3 and legacy media
  sessions.
• Keep explicit play semantics, so redundant KEYCODE_MEDIA_PLAY events never
  toggle active playback into a pause.
• Run the play-only action as soon as the queue accepts the first tap instead of
  waiting for the multi-tap timeout.
• Keep the first tap registered so second and third taps still resolve to the
  configured headphone action.
• Continue suppressing the spurious KEYCODE_MEDIA_PLAY some headphones send
  after KEYCODE_MEDIA_NEXT or KEYCODE_MEDIA_PREVIOUS.
• Log when the explicit-play fast path starts playback.

The release-default phone path is the legacy media session unless Firebase Remote
Config enables Media3 MediaSession; debug and prototype builds default to
Media3. Both paths have direct unit coverage.

Behavior notes

• If a paused headset double-tap begins with KEYCODE_MEDIA_PLAY, playback now
  resumes immediately and the resolved double-tap action still runs. For example,
  an ADD_BOOKMARK double tap resumes and then bookmarks.
• KEYCODE_MEDIA_PLAY is intentionally play-only. While already playing it does
  not pause; headsets using a toggle command must send KEYCODE_MEDIA_PLAY_PAUSE
  or KEYCODE_MEDIA_PAUSE for pause behavior.

Fixes #5631

Testing Instructions

Automated

1. Run the focused suites:

   ./gradlew :modules:services:repositories:testDebugUnitTest \
     --tests '*MediaEventQueueTest*' \
     --tests '*MediaButtonEventHandlerTest*' \
     --tests '*MediaSessionManagerCallbackTest*' \
     --tests '*Media3SessionCallbackTest*'

2. Confirm all 57 focused tests pass.

3. Run ./gradlew :modules:services:repositories:testDebugUnitTest and confirm all
   860 tests pass.

4. Run ./gradlew spotlessCheck.

Manual: test the release-default legacy path first

1. In a debug/debugProd build, open Settings → Beta Features and turn
   Media3 MediaSession off.
2. Force-stop and relaunch the app because media-session selection is cached for
   the process.
3. Confirm LogBuffer contains Legacy playback service created.
4. Connect Bluetooth headphones, start playback, pause it, turn the screen off,
   and leave the phone idle for several minutes.
5. Press the dedicated play control and confirm playback resumes immediately.
6. Confirm rapid repeated play commands start playback only once, and a redundant
   play command while already playing does not pause it.
7. Confirm NEXT/PREVIOUS still run their configured actions and a following
   spurious play event does not trigger a single-tap action.
8. While paused, confirm a double tap resumes immediately and then runs the
   configured double-tap action, including ADD_BOOKMARK.
9. Turn Media3 MediaSession on, force-stop and relaunch, and repeat steps 4–8
   for the Media3 path.

Manual result (2026-08-07)

Installed GitHub's stacked merge result
c20537ff7c021b7431c0f33c332cb52084505338 on a physical Samsung SM-G990E
running Android 16. Both the release-default legacy session and the Media3
session passed:

• A dedicated KEYCODE_MEDIA_PLAY resumed paused playback while the display
  remained dozing; the explicit-play handler ran in approximately 5 ms on the
  legacy path and 10 ms on Media3, rather than after the 600 ms tap window.
• A redundant play command while already playing remained a no-op.
• Rapid repeated play commands started playback once and still resolved the
  configured double-tap skip-forward action after the window.
• NEXT/PREVIOUS followed immediately by the spurious PLAY event kept
  the configured skip action and suppressed the trailing play action.
• No crash or ANR was recorded.

Playback was left paused, the Media3 beta flag was restored to its original
enabled state, and the display was returned to dozing. No Bluetooth audio
accessory was connected, so media keys were injected through Android's media-key
dispatcher with adb. This verifies the app/session and screen-off behavior,
but not a physical headset's Bluetooth transport behavior.

Screenshots or Screencast

Not applicable; there are 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
• All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
• Any jetpack compose components I added or changed are covered by compose previews
• I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics.

I have tested any UI changes...

Not applicable; there are no UI changes.

[claude]

Claude finished @joashrajin's task in 2m 37s —— View job

---

Code Review — PR #5645: Resume explicit media play immediately

• Read changed files & understand the flow
• Review MediaEventQueue changes
• Review Media3SessionCallback & MediaSessionManager changes
• Review PlaybackManager changes
• Review tests

Overall this is a clean, well-targeted fix with good test coverage. The design correctly keeps KEYCODE_MEDIA_PLAY inside the multi-tap queue (so spurious play-after-next/prev is still suppressed) while firing the play-only action immediately on the first tap. playIfNotPlaying mirrors playPause's not-playing branch exactly (playQueue(sourceView)), so no logic is lost by bypassing the toggle. Nice work.

A few observations, mostly minor:

1. Double/triple tap of KEYCODE_MEDIA_PLAY while paused now resumes and skips

Because the immediate action fires on the first accepted tap and the tap is still counted, a double-tap of the dedicated play key while paused will resume playback and then perform the configured skip action. This is intentional and explicitly tested (KEYCODE_MEDIA_PLAY still starts a double-tap sequence), and it's the right tradeoff versus waiting 600 ms — just worth confirming the UX (brief resume-then-skip) is acceptable. Note this only affects KEYCODE_MEDIA_PLAY; HEADSETHOOK/KEYCODE_MEDIA_PLAY_PAUSE still behave exactly as before.

2. Suppression keyed implicitly on onImmediateSingleTap != null

In MediaEventQueue.handleSingleTapEvent:

newSingleTapJob.event().takeUnless {
    it == MediaEvent.SingleTap && onImmediateSingleTap != null
}

The callback parameter does double duty: "run this now" and "suppress the delayed single-tap result." It works, but the coupling is a bit implicit — a reader has to infer that providing a callback also means "don't re-fire on timeout." A short comment (or a dedicated suppressDelayedSingleTap concept) would make the intent clearer at the call site. Not blocking.

3. Duplicated call-site block across the two callbacks

The

onImmediateSingleTap = if (keyEvent.keyCode == KeyEvent.KEYCODE_MEDIA_PLAY) {
    { playbackManager.playIfNotPlaying(sourceView = source) }
} else {
    null
}

block is duplicated verbatim in both Media3SessionCallback and MediaSessionManager.MediaSessionCallback. That's fine and keeps each path self-contained, but if the mapping ever changes (e.g. another key code should get explicit-play semantics) both sites must be kept in sync. Just flagging for future maintenance.

Things I checked and found correct

• Spurious KEYCODE_MEDIA_PLAY after NEXT/PREVIOUS is still suppressed and does not trigger the immediate play — the multiTapJob?.isActive branch returns before onImmediateSingleTap is invoked (covered by KEYCODE_MEDIA_PLAY stays suppressed after KEYCODE_MEDIA_NEXT).
• Redundant KEYCODE_MEDIA_PLAY while already playing no longer pauses (isPlaying() guard + playQueue, verified never().pause(...)).
• No new concurrency pattern — the immediate action runs on the same scope.launch used by the existing single-tap playPause handler, so no new threading concern is introduced.
• PlaybackManagerPlayIfNotPlayingTest, the MediaEventQueue tests, and the Media3SessionCallback tests cover the key branches (immediate play, multi-tap continuation, suppression after next).

No blocking issues from my side — items above are optional polish.

[claude]

Claude finished @joashrajin's task in 2m 46s —— View job

---

Code Review — PR #5645: Resume explicit media play immediately

• Read changed files & understand the flow
• Review new MediaButtonEventHandler + serialization
• Review MediaEventQueue, Media3SessionCallback, MediaSessionManager
• Review PlaybackManager.playIfNotPlaying
• Review tests

This synchronize adds the MediaButtonEventHandler extraction (commit Serialize media button event handling), which both de-duplicates the two call sites the previous review flagged and adds an ordering guarantee. The extraction is a clean improvement — nice iteration.

The ordering fix is correct (and worth calling out)

The key subtlety is scope.launch(start = CoroutineStart.UNDISPATCHED) in MediaButtonEventHandler.handle (MediaButtonEventHandler.kt:42). Because the queue never holds stateMutex across a suspension point (handleSingleTapEvent releases the lock before await(), and handleMultiTapEvent has no suspending call inside the lock), the uncontended withLock fast-path runs synchronously on the calling thread. So each event's tap is registered before handle() returns to the framework, and the coroutine only suspends later at newSingleTapJob.await(). This is what preserves delivery order — e.g. NEXT's multiTapJob is guaranteed to be set before the spurious MEDIA_PLAY reads it, so suppression still works. The design depends on that "no suspension under the lock" invariant, so it's worth guarding against regressions if anyone edits MediaEventQueue later.

assertEquals(1, immediatePlayCount) immediately after handle() in MediaButtonEventHandlerTest implicitly validates the synchronous-registration behavior. 👍

Observations (all minor / non-blocking)

1. Immediate play now runs on the callback (main) thread rather than Dispatchers.Default. Previously onImmediatePlay ran inside the dispatched scope.launch; with UNDISPATCHED it executes synchronously on whichever thread delivers the media-button event (main thread for both Media3 and the legacy MediaSessionCompat callback here). playIfNotPlaying only does isPlaying() + playQueue { launch … }, so this is fine — and arguably better — but it is a thread-context change worth being aware of if playIfNotPlaying ever grows heavier work.

2. Suppression still keyed implicitly on onImmediateSingleTap != null (MediaEventQueue.kt:50-52). Same point as the prior review — the callback doubles as "run now" and "suppress the delayed single-tap." The new comment on lines 48-49 helps, but a named concept (e.g. suppressDelayedSingleTap) would make intent clearer at a glance. Not blocking.

3. coroutineContext.ensureActive() at MediaButtonEventHandler.kt:44 is defensive but redundant — launch on a cancelled scope won't run the body. Harmless; leaving it is fine.

Things I checked and found correct

• Double/triple tap of MEDIA_PLAY while paused → immediate resume + skip after the window (intentional, covered by rapid KEYCODE_MEDIA_PLAY … emit a double tap).
• Redundant MEDIA_PLAY while playing → playIfNotPlaying no-ops (isPlaying() guard) and the delayed SingleTap is suppressed, so no pause. Correct.
• Spurious MEDIA_PLAY after NEXT/PREVIOUS → multiTapJob?.isActive branch returns before onImmediateSingleTap is invoked, so no immediate play and no single-tap action (KEYCODE_MEDIA_NEXT suppresses a following KEYCODE_MEDIA_PLAY).
• MEDIA_PLAY_PAUSE / HEADSETHOOK → onImmediateSingleTap is null, so behavior is unchanged (delayed multi-tap).
• Thread-safety: singleTapJob/multiTapJob/counter are only touched under stateMutex; the handle concurrent immediate single taps exactly once test exercises the mutex across 8 threads.
• Exception safety: a throw from onImmediatePlay is caught by the coroutine's try/catch (routed to onError) and never escapes handle().

No blocking issues — the items above are optional polish. Solid, well-tested fix.

[claude]

Claude finished @joashrajin's task in 3m 21s —— View job

---

Code Review — PR #5645: Resume explicit media play immediately

• Read changed files & understand the diff
• Review PlaybackManager.playIfNotPlaying
• Review Media3SessionCallback & MediaSessionManager wiring
• Review tests

This latest sync wires both media-session paths through the shared MediaButtonEventHandler and adds PlaybackManager.playIfNotPlaying. I re-verified the current state end-to-end — the two prior reviews still hold and I found no blocking issues. A few notes below, all minor.

Verified correct on the current diff

• playIfNotPlaying mirrors the toggle's not-playing branch (isPlaying() guard → playQueue(sourceView)), so nothing is lost by bypassing playPause. A redundant KEYCODE_MEDIA_PLAY while playing no-ops instead of pausing (PlaybackManager.kt:628).
• Synchronous registration invariant holds. MediaEventQueue never suspends while holding stateMutex (the withLock blocks only call launch/incrementTaps/cancel, and await() happens outside the lock), so the CoroutineStart.UNDISPATCHED launch in MediaButtonEventHandler.handle registers each tap before returning to the framework. The new MediaSessionManagerCallbackTest asserts playIfNotPlaying(...) immediately after onMediaButtonEvent(...) returns, which directly exercises this.
• Suppression after NEXT/PREVIOUS still works. For KEYCODE_MEDIA_PLAY, handleSingleTapEvent hits the multiTapJob?.isActive == true branch and returns null before onImmediateSingleTap?.invoke(), so the spurious play neither resumes nor fires a single-tap action. Covered in both callback test files.
• internal inner class MediaSessionCallback — only doc references exist outside the class (Media3SessionCallback.kt:48, MediaSessionActions.kt:23), and the sole production construction site was updated with scopeProvider = { scope }, so the visibility narrowing is safe.
• source resolution is correct in both callbacks (SourceView.MEDIA_BUTTON_BROADCAST_ACTION).

Minor observations (non-blocking)

1. playIfNotPlaying KDoc scopes the motivation to "wireless Android Auto … send it redundantly," but this PR's primary fix is latency — resuming immediately on screen-off Bluetooth rather than after the 600 ms window. The redundant-play case is real too, but a reader of the KDoc alone wouldn't learn about the fast-path/latency intent. Consider a sentence noting it also fires ahead of multi-tap disambiguation. (The CHANGELOG entry, by contrast, captures both symptoms well.)

2. Suppression is still keyed implicitly on onImmediateSingleTap != null (MediaEventQueue.kt:50-52) — the callback doubles as "run now" and "suppress the delayed single-tap." Flagged in both prior reviews; the comment on lines 48-49 helps. A named suppressDelayedSingleTap flag would make intent obvious at a glance, but it's fine as-is.

3. onImmediatePlay now executes on the media-button delivery thread (main) via the undispatched launch, rather than on the dispatched scope as before. playIfNotPlaying is cheap (isPlaying() + playQueue, which launches its own work), so this is fine — worth remembering only if that method ever grows heavier synchronous work.

Tests

Coverage is solid: the Media3 suite now asserts immediate playQueue before the window, resume-then-double-tap (ADD_BOOKMARK), post-NEXT suppression, and the already-playing no-op; the new legacy MediaSessionManagerCallbackTest gives the previously-untested MediaSessionCompat path direct coverage for both the fast path and NEXT-suppression. The removed double tap with ADD_BOOKMARK Media3 test is effectively subsumed by the new resume-before-double-tap test.

Nice, well-targeted iteration. 👍

[claude]

Claude finished @joashrajin's task in 2m 15s —— View job

---

Code Review — PR #5645: Resume explicit media play immediately

• Read changed files & understand the diff
• Review PlaybackManager.playIfNotPlaying
• Review Media3SessionCallback & MediaSessionManager wiring
• Review tests

I re-reviewed the latest sync (Address stacked media play review feedback) against origin/codex/serialize-media-button-events. The new commit tightens wording and, more importantly, strengthens the test coverage. I found no blocking issues — the three prior reviews still hold.

What changed since the last review (and is correct)

1. KDoc + log wording (PlaybackManager.kt:624, :637) now mentions the fast-path/latency intent ("Media-button callbacks invoke it immediately instead of waiting for multi-tap disambiguation") and drops the redundant "Starting playback for…" phrasing. This directly resolves the KDoc-scoping nit from the previous review. 👍

2. Media3 test switched UnconfinedTestDispatcher → StandardTestDispatcher (Media3SessionCallbackTest.kt:70). This is the meaningful improvement: under the unconfined dispatcher the queue coroutine ran eagerly, so a test couldn't distinguish "played immediately" from "played after the window." With StandardTestDispatcher, nothing runs until advanceUntilIdle() — except the CoroutineStart.UNDISPATCHED body in MediaButtonEventHandler.handle, which runs synchronously up to newSingleTapJob.await(). So verify(playbackManager).playQueue(...) before advanceUntilIdle() now genuinely proves the synchronous-registration/immediate-play invariant rather than incidentally passing. Good hardening.

3. New legacy tests (MediaSessionManagerCallbackTest.kt): KEYCODE_HEADSETHOOK → playPause (never playIfNotPlaying), and rapid triple KEYCODE_MEDIA_PLAY → immediate play once + resolves the configured previous action. Both trace correctly through the queue (first tap creates the SingleTapJob and fires onImmediatePlay; follow-ups incrementTaps() and return null; the window resolves to TripleTap, which is not suppressed because suppression only drops a lone SingleTap). Coverage of the previously-untested MediaSessionCompat path is a real gap closed.

Things I re-verified end-to-end on the current diff

• playIfNotPlaying mirrors the toggle's not-playing branch — isPlaying() guard → playQueue(sourceView); a redundant KEYCODE_MEDIA_PLAY while playing no-ops instead of pausing (PlaybackManager.kt:628).
• source resolves correctly in both callbacks — Media3 via the source constructor param default (Media3SessionCallback.kt:64), legacy via the outer-class private val source (MediaSessionManager.kt:151), both MEDIA_BUTTON_BROADCAST_ACTION.
• Suppression after NEXT/PREVIOUS still holds — for KEYCODE_MEDIA_PLAY, handleSingleTapEvent hits the multiTapJob?.isActive == true branch and returns null before onImmediateSingleTap?.invoke(), so the spurious play neither resumes nor fires a single-tap action.
• internal inner class MediaSessionCallback — visibility narrowing is safe; the sole production construction site now passes scopeProvider = { scope }, and the test constructs it via the same package.
• Legacy onMediaButtonEvent refactor preserves behavior: non-ACTION_MEDIA_BUTTON and unhandled key codes still fall through to super.onMediaButtonEvent(...); handle() returns false for ACTION_UP, so those also reach super.

Remaining optional polish (non-blocking, unchanged from prior reviews)

1. Suppression keyed implicitly on onImmediateSingleTap != null (MediaEventQueue.kt:50-52) — the callback doubles as "run now" and "suppress the delayed single-tap." The comment helps; a named suppressDelayedSingleTap concept would make intent obvious at a glance.
2. onImmediatePlay executes on the media-button delivery (main) thread via the undispatched launch. playIfNotPlaying is cheap (isPlaying() + playQueue), so this is fine today — worth remembering only if that method ever grows heavier synchronous work.

Nice, well-tested iteration. LGTM. 👍

[wpmobilebot]

Version 8.18 has now entered code-freeze, so the milestone of this PR has been updated to 8.19.

[wpmobilebot]

Version 8.19 has now entered code-freeze, so the milestone of this PR has been updated to 8.20.