mirror of
https://github.com/d4rken-org/capod.git
synced 2026-09-14 18:26:11 -04:00
fix(media): Move audio playback callback off the main thread
MediaControl registered its AudioPlaybackCallback with a null Handler, binding delivery to the main looper. Both the callback body and the constructor's seed read call AudioManager.isMusicActive, a binder transaction into AudioService, producing two ANR clusters: one in onPlaybackConfigChanged and one in <init>, the latter on the cold-start critical path since MediaControl is constructed during App.onCreate. Registration and seeding now run on a dedicated, injected Handler backed by a "CAPod-MediaControl" HandlerThread, and the callback is delivered on that same looper. Registration happens before seeding so a transition during registration is queued behind the seed instead of being lost. The handler is constructor-injected via a new @AudioCallbackHandler qualifier so unit tests can drive it without Robolectric.
This commit is contained in:
@@ -3,7 +3,9 @@ package eu.darken.capod.common
|
||||
import android.media.AudioManager
|
||||
import android.media.AudioPlaybackConfiguration
|
||||
import android.os.Build
|
||||
import android.os.Handler
|
||||
import android.view.KeyEvent
|
||||
import eu.darken.capod.common.dagger.AudioCallbackHandler
|
||||
import eu.darken.capod.common.debug.logging.Logging.Priority.INFO
|
||||
import eu.darken.capod.common.debug.logging.Logging.Priority.WARN
|
||||
import eu.darken.capod.common.debug.logging.log
|
||||
@@ -16,6 +18,7 @@ import javax.inject.Singleton
|
||||
class MediaControl @Inject constructor(
|
||||
private val audioManager: AudioManager,
|
||||
private val timeSource: TimeSource,
|
||||
@AudioCallbackHandler private val audioCallbackHandler: Handler,
|
||||
) {
|
||||
/**
|
||||
* Set when [sendPause] dispatches a pause we expect to take effect, cleared when [sendPlay]
|
||||
@@ -43,10 +46,25 @@ class MediaControl @Inject constructor(
|
||||
}
|
||||
|
||||
init {
|
||||
// Seed the active flag from current state so we won't miss the next inactive→active
|
||||
// transition if music is already playing when MediaControl is constructed.
|
||||
lastKnownMusicActive = audioManager.isMusicActive
|
||||
audioManager.registerAudioPlaybackCallback(playbackCallback, null)
|
||||
// Both calls below are binder transactions into AudioService. On the main thread they sat on
|
||||
// the cold-start critical path (MediaControl is constructed during App.onCreate via Hilt) and
|
||||
// produced ANRs.
|
||||
//
|
||||
// Register BEFORE seeding: both run on this handler's Looper, so any playback callback that
|
||||
// fires during registration is queued behind this runnable and cannot execute until the seed
|
||||
// is done. Seeding first would instead leave a gap in which a transition is neither delivered
|
||||
// (not yet registered) nor reflected in the seed.
|
||||
//
|
||||
// Residual window: between construction and this runnable draining, no callback is live. That
|
||||
// is one runnable-drain on an empty queue. If an auto-pause armed capPaused and the user
|
||||
// manually resumed inside it, the inactive→active edge is missed and capPaused stays stale,
|
||||
// costing one redundant (idempotent) MEDIA_PLAY on a later pod-in. Accepted deliberately;
|
||||
// gating reactions on a readiness latch would risk auto-play/pause failing silently instead.
|
||||
audioCallbackHandler.post {
|
||||
audioManager.registerAudioPlaybackCallback(playbackCallback, audioCallbackHandler)
|
||||
lastKnownMusicActive = audioManager.isMusicActive
|
||||
log(TAG, INFO) { "Playback callback registered on ${Thread.currentThread().name}" }
|
||||
}
|
||||
}
|
||||
|
||||
val isPlaying: Boolean
|
||||
|
||||
@@ -5,10 +5,13 @@ import android.app.NotificationManager
|
||||
import android.bluetooth.BluetoothManager
|
||||
import android.content.Context
|
||||
import android.media.AudioManager
|
||||
import android.os.Handler
|
||||
import android.os.HandlerThread
|
||||
import dagger.Module
|
||||
import dagger.Provides
|
||||
import dagger.hilt.InstallIn
|
||||
import dagger.hilt.components.SingletonComponent
|
||||
import javax.inject.Qualifier
|
||||
import javax.inject.Singleton
|
||||
|
||||
@InstallIn(SingletonComponent::class)
|
||||
@@ -34,4 +37,15 @@ class AndroidModule {
|
||||
fun audioManager(context: Context): AudioManager =
|
||||
context.getSystemService(Context.AUDIO_SERVICE) as AudioManager
|
||||
|
||||
@Provides
|
||||
@Singleton
|
||||
@AudioCallbackHandler
|
||||
fun audioCallbackHandler(): Handler =
|
||||
Handler(HandlerThread("CAPod-MediaControl").apply { start() }.looper)
|
||||
|
||||
}
|
||||
|
||||
@Qualifier
|
||||
@MustBeDocumented
|
||||
@Retention(AnnotationRetention.RUNTIME)
|
||||
annotation class AudioCallbackHandler
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package eu.darken.capod.common
|
||||
|
||||
import android.media.AudioManager
|
||||
import android.os.Handler
|
||||
import io.mockk.CapturingSlot
|
||||
import io.mockk.Runs
|
||||
import io.mockk.clearMocks
|
||||
@@ -9,6 +10,7 @@ import io.mockk.just
|
||||
import io.mockk.mockk
|
||||
import io.mockk.slot
|
||||
import io.mockk.verify
|
||||
import io.mockk.verifyOrder
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.jupiter.api.Assertions.assertFalse
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
@@ -22,7 +24,9 @@ class MediaControlTest : BaseTest() {
|
||||
private lateinit var audioManager: AudioManager
|
||||
private lateinit var mediaControl: MediaControl
|
||||
private lateinit var timeSource: TestTimeSource
|
||||
private lateinit var handler: Handler
|
||||
private lateinit var playbackCallbackSlot: CapturingSlot<AudioManager.AudioPlaybackCallback>
|
||||
private lateinit var initRunnableSlot: CapturingSlot<Runnable>
|
||||
|
||||
@BeforeEach
|
||||
fun setup() {
|
||||
@@ -35,7 +39,15 @@ class MediaControlTest : BaseTest() {
|
||||
every { audioManager.isMusicActive } returns false
|
||||
playbackCallbackSlot = slot()
|
||||
every { audioManager.registerAudioPlaybackCallback(capture(playbackCallbackSlot), any()) } just Runs
|
||||
mediaControl = MediaControl(audioManager, timeSource)
|
||||
// The handler is captured, not inlined: registration and seeding now happen on a posted
|
||||
// runnable, and an unconditionally-inline post would hide the window that exists between
|
||||
// construction and that runnable draining.
|
||||
handler = mockk()
|
||||
initRunnableSlot = slot()
|
||||
every { handler.post(capture(initRunnableSlot)) } returns true
|
||||
mediaControl = MediaControl(audioManager, timeSource, handler)
|
||||
// Drain the init runnable so `playbackCallbackSlot` is populated for `fireCallback()`.
|
||||
initRunnableSlot.captured.run()
|
||||
}
|
||||
|
||||
private fun fireCallback() {
|
||||
@@ -228,4 +240,72 @@ class MediaControlTest : BaseTest() {
|
||||
// we don't want to claim our pause "stuck" when audio is playing.
|
||||
assertFalse(mediaControl.wasRecentlyPausedByCap)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `playback callback is registered on the injected background handler, not the main looper`() {
|
||||
// Regression for the ANR cluster in onPlaybackConfigChanged: passing `null` here binds
|
||||
// callback delivery to the main looper, and the callback body does a binder call.
|
||||
verify { audioManager.registerAudioPlaybackCallback(any(), handler) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `construction does no binder work on the calling thread`() {
|
||||
// Regression for the ANR cluster in MediaControl's constructor: it runs during
|
||||
// App.onCreate via Hilt, so nothing here may touch AudioService inline.
|
||||
val freshAudioManager = mockk<AudioManager>(relaxed = true)
|
||||
val freshHandler = mockk<Handler>()
|
||||
every { freshHandler.post(any()) } returns true
|
||||
|
||||
MediaControl(freshAudioManager, timeSource, freshHandler)
|
||||
|
||||
verify(exactly = 0) { freshAudioManager.isMusicActive }
|
||||
verify(exactly = 0) { freshAudioManager.registerAudioPlaybackCallback(any(), any()) }
|
||||
verify(exactly = 1) { freshHandler.post(any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `init registers the callback before seeding the active flag`() {
|
||||
// Seeding first would leave a gap in which a transition is neither delivered (not yet
|
||||
// registered) nor reflected in the seed.
|
||||
val freshAudioManager = mockk<AudioManager>(relaxed = true)
|
||||
every { freshAudioManager.isMusicActive } returns false
|
||||
every { freshAudioManager.registerAudioPlaybackCallback(any(), any()) } just Runs
|
||||
val freshHandler = mockk<Handler>()
|
||||
val runnableSlot = slot<Runnable>()
|
||||
every { freshHandler.post(capture(runnableSlot)) } returns true
|
||||
|
||||
MediaControl(freshAudioManager, timeSource, freshHandler)
|
||||
runnableSlot.captured.run()
|
||||
|
||||
verifyOrder {
|
||||
freshAudioManager.registerAudioPlaybackCallback(any(), freshHandler)
|
||||
freshAudioManager.isMusicActive
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `media control still works before the init runnable has drained`() = runTest {
|
||||
// The window between construction and the posted runnable running: no callback is live
|
||||
// yet, but the key-dispatch paths must behave normally.
|
||||
val freshAudioManager = mockk<AudioManager>(relaxed = true)
|
||||
every { freshAudioManager.isMusicActive } returns true
|
||||
every { freshAudioManager.dispatchMediaKeyEvent(any()) } just Runs
|
||||
every { freshAudioManager.registerAudioPlaybackCallback(any(), any()) } just Runs
|
||||
val freshHandler = mockk<Handler>()
|
||||
every { freshHandler.post(any()) } returns true
|
||||
|
||||
val undrained = MediaControl(freshAudioManager, timeSource, freshHandler)
|
||||
|
||||
assertFalse(undrained.wasRecentlyPausedByCap)
|
||||
|
||||
assertTrue(undrained.sendPause(rememberForResume = true))
|
||||
assertTrue(undrained.wasRecentlyPausedByCap)
|
||||
verify(exactly = 2) { freshAudioManager.dispatchMediaKeyEvent(any()) }
|
||||
|
||||
clearMocks(freshAudioManager, answers = false, recordedCalls = true)
|
||||
|
||||
undrained.sendPlay()
|
||||
assertFalse(undrained.wasRecentlyPausedByCap)
|
||||
verify(exactly = 2) { freshAudioManager.dispatchMediaKeyEvent(any()) }
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user