Skip to content
Closed
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* New Features
* Add option to switch to the HLS video stream when playing a downloaded episode
([#5620](https://github.com/Automattic/pocket-casts-android/pull/5620))
* Bug Fixes
* Fix playback repeatedly pausing on wireless Android Auto
([#5535](https://github.com/Automattic/pocket-casts-android/pull/5535))

8.17
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ internal class Media3SessionCallback(
try {
val outputEvent = mediaEventQueue.consumeEvent(inputEvent)
when (outputEvent) {
MediaEvent.SingleTap -> handleMediaButtonSingleTap()
MediaEvent.SingleTap -> handleMediaButtonSingleTap(playOnly = keyEvent.keyCode == KeyEvent.KEYCODE_MEDIA_PLAY)
MediaEvent.DoubleTap -> handleMediaButtonDoubleTap()
MediaEvent.TripleTap -> handleMediaButtonTripleTap()
null -> Unit
Expand All @@ -277,8 +277,12 @@ internal class Media3SessionCallback(
return false
}

private fun handleMediaButtonSingleTap() {
playbackManager.playPause(sourceView = source)
private fun handleMediaButtonSingleTap(playOnly: Boolean = false) {
if (playOnly) {
playbackManager.playIfNotPlaying(sourceView = source)
} else {
playbackManager.playPause(sourceView = source)
}
}

private fun handleMediaButtonDoubleTap() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ class MediaSessionManager(
val outputEvent = mediaEventQueue.consumeEvent(inputEvent)
LogBuffer.i(LogBuffer.TAG_PLAYBACK, "Media button output event: ${keyEvent.action}")
when (outputEvent) {
MediaEvent.SingleTap -> handleMediaButtonSingleTap()
MediaEvent.SingleTap -> handleMediaButtonSingleTap(playOnly = keyEvent.keyCode == KeyEvent.KEYCODE_MEDIA_PLAY)
MediaEvent.DoubleTap -> handleMediaButtonDoubleTap()
MediaEvent.TripleTap -> handleMediaButtonTripleTap()
null -> Unit
Expand Down Expand Up @@ -1281,8 +1281,12 @@ class MediaSessionManager(
LogBuffer.i(LogBuffer.TAG_PLAYBACK, "Event from Media Session to $action. ${userInfo.orEmpty()}")
}

private fun handleMediaButtonSingleTap() {
playbackManager.playPause(sourceView = source)
private fun handleMediaButtonSingleTap(playOnly: Boolean = false) {
if (playOnly) {
playbackManager.playIfNotPlaying(sourceView = source)
} else {
playbackManager.playPause(sourceView = source)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This guard is identical to the one just added in Media3SessionCallback.handleMediaButtonSingleTap (comment, isPlaying() check, log line, early return). Since the two callbacks must stay behaviourally in lockstep for play-only semantics, consider hoisting this into a single shared helper — e.g. an extension on PlaybackManager:

fun PlaybackManager.playIfNotPlaying(sourceView: SourceView) {
    if (isPlaying()) return
    playQueue(sourceView)
}

Then both single-tap handlers become if (playOnly) playbackManager.playIfNotPlaying(source) else playbackManager.playPause(source). This also removes the double isPlaying() read (guard + playPause()) and the tiny TOCTOU window between them. Non-blocking — the current duplication is correct, just drift-prone.

}

private fun handleMediaButtonDoubleTap() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,20 @@ open class PlaybackManager @Inject constructor(
}
}

/**
* Plays the queue only if playback isn't already running, unlike the [playPause] toggle.
* Used for KEYCODE_MEDIA_PLAY, which has explicit play semantics: some head units
* (wireless Android Auto in particular) send it redundantly while playback is already
* running, and toggling would pause playback.
*/
fun playIfNotPlaying(sourceView: SourceView = SourceView.UNKNOWN) {
if (isPlaying()) {
LogBuffer.i(LogBuffer.TAG_PLAYBACK, "Ignoring play request because playback is already playing")
} else {
playQueue(sourceView)
}
}

fun playQueue(
sourceView: SourceView = SourceView.UNKNOWN,
showedStreamWarning: Boolean = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,11 +340,22 @@ class Media3SessionCallbackTest {
// --- Headphone action handler tests ---

@Test
fun `KEYCODE_MEDIA_PLAY routes through multi-tap as single tap`() = runTest {
fun `KEYCODE_MEDIA_PLAY routes through multi-tap as play-only single tap`() = runTest {
sendMediaButtonEvent(KeyEvent.KEYCODE_MEDIA_PLAY)
testScope.advanceUntilIdle()

// Routed through MediaEventQueue — single tap resolves as play/pause
// A dedicated play key must never toggle playback into a pause
verify(playbackManager).playIfNotPlaying(sourceView = any())
verify(playbackManager, never()).playPause(sourceView = any())
}

@Test
fun `KEYCODE_MEDIA_PLAY_PAUSE still toggles while playing`() = runTest {
whenever(playbackManager.isPlaying()).thenReturn(true)

sendMediaButtonEvent(KeyEvent.KEYCODE_MEDIA_PLAY_PAUSE)
testScope.advanceUntilIdle()

verify(playbackManager).playPause(sourceView = any())
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package au.com.shiftyjelly.pocketcasts.repositories.playback

import au.com.shiftyjelly.pocketcasts.analytics.SourceView
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.kotlin.any
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import org.robolectric.RobolectricTestRunner

@RunWith(RobolectricTestRunner::class)
class PlaybackManagerPlayIfNotPlayingTest {

private val playbackManager = mock<PlaybackManager>().also {
whenever(it.playIfNotPlaying(any())).thenCallRealMethod()
}

@Test
fun `does nothing while already playing`() {
whenever(playbackManager.isPlaying()).thenReturn(true)

playbackManager.playIfNotPlaying(sourceView = SourceView.MEDIA_BUTTON_BROADCAST_ACTION)

verify(playbackManager, never()).playQueue(any(), any())
verify(playbackManager, never()).pause(any(), any())
}

@Test
fun `plays the queue while not playing`() {
whenever(playbackManager.isPlaying()).thenReturn(false)

playbackManager.playIfNotPlaying(sourceView = SourceView.MEDIA_BUTTON_BROADCAST_ACTION)

verify(playbackManager).playQueue(eq(SourceView.MEDIA_BUTTON_BROADCAST_ACTION), any())
}
}