From 7e93f825775aa1e8dca2eb8689ab6455551b0668 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 19:51:25 +0000 Subject: [PATCH] fix(audio): switch ringtone from Ringtone to MediaPlayer + idempotent start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of "ringing survives reject on old Android": 1. startRinging() was NOT idempotent. When CallManager re-emitted IncomingCall state (e.g., when another group member rejected while still ringing), the state collector called startRinging() again. startRingtone() OVERWROTE the ringtone field reference without calling stop() on the old one. Old Ringtone instances kept playing with no reachable reference — stopRingtone() on reject only stopped the latest one. 2. Ringtone.isLooping was only set on API 28+. On older Android the Ringtone class had documented reliability problems with stop(). MediaPlayer has reliable stop/release on all API levels and supports looping universally. Fixes: - Switch from android.media.Ringtone to android.media.MediaPlayer. - Make startRinging() idempotent — always call stopRinging() first so any existing player/vibrator is torn down before a new one starts. - Add aggressive tracing logs throughout the reject path: * CallAudioManager: instance id + thread on every start/stop, MediaPlayer error listener, before-and-after player hashes. * CallSession: log every state collector tick, log close() entry. * CallManager: log rejectCall() entry/exit + transitionToEnded. * CallNotificationReceiver: log every action with state transitions. With these logs, if ringing still survives reject we can trace exactly which primitive is failing. https://claude.ai/code/session_019yNnDjGKmJb19gadmojq54 --- .../amethyst/service/call/CallAudioManager.kt | 78 +++++++++++++++---- .../service/call/CallNotificationReceiver.kt | 14 +++- .../amethyst/ui/call/session/CallSession.kt | 2 + .../amethyst/commons/call/CallManager.kt | 8 +- 4 files changed, 87 insertions(+), 15 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallAudioManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallAudioManager.kt index 2fc33b00d9..5a0fac755f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallAudioManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallAudioManager.kt @@ -33,7 +33,7 @@ import android.hardware.SensorManager import android.media.AudioAttributes import android.media.AudioDeviceInfo import android.media.AudioManager -import android.media.Ringtone +import android.media.MediaPlayer import android.media.RingtoneManager import android.media.ToneGenerator import android.os.Build @@ -42,10 +42,13 @@ import android.os.VibrationEffect import android.os.Vibrator import android.os.VibratorManager import androidx.core.content.ContextCompat +import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +private const val TAG = "CallAudioManager" + enum class AudioRoute { EARPIECE, SPEAKER, @@ -55,7 +58,14 @@ enum class AudioRoute { class CallAudioManager( private val context: Context, ) { - private var ringtone: Ringtone? = null + /** + * Instance id — every log line from this audio manager is prefixed + * with this so we can trace whether multiple CallAudioManager + * instances are fighting over the ringtone. + */ + private val instanceId = nextInstanceId.getAndIncrement() + + private var ringtonePlayer: MediaPlayer? = null private var vibrator: Vibrator? = null private var proximityWakeLock: PowerManager.WakeLock? = null private var ringbackTone: ToneGenerator? = null @@ -63,6 +73,14 @@ class CallAudioManager( private var previousAudioMode: Int = AudioManager.MODE_NORMAL private var scoReceiver: BroadcastReceiver? = null + init { + Log.d(TAG) { "[#$instanceId] created on ${Thread.currentThread().name}" } + } + + companion object { + private val nextInstanceId = java.util.concurrent.atomic.AtomicInteger(0) + } + private val _audioRoute = MutableStateFlow(AudioRoute.EARPIECE) val audioRoute: StateFlow = _audioRoute.asStateFlow() @@ -97,6 +115,13 @@ class CallAudioManager( } fun startRinging() { + Log.d(TAG) { "[#$instanceId] startRinging on ${Thread.currentThread().name} — existing player=${ringtonePlayer?.hashCode()}" } + // IDEMPOTENT: always stop any existing player/vibrator before + // starting a new one. Without this, rapid re-entry (e.g. state + // flow re-emission when a group member rejects while we are + // ringing) would leak MediaPlayer/Ringtone instances that keep + // playing because the old reference is overwritten before stop. + stopRinging() val ringerMode = audioManager.ringerMode if (ringerMode != AudioManager.RINGER_MODE_SILENT) { if (ringerMode == AudioManager.RINGER_MODE_NORMAL) { @@ -107,6 +132,7 @@ class CallAudioManager( } fun stopRinging() { + Log.d(TAG) { "[#$instanceId] stopRinging on ${Thread.currentThread().name} — player=${ringtonePlayer?.hashCode()}" } stopRingtone() stopVibration() } @@ -216,6 +242,7 @@ class CallAudioManager( } fun release() { + Log.d(TAG) { "[#$instanceId] release on ${Thread.currentThread().name}" } stopRinging() stopRingbackTone() restoreAudioMode() @@ -349,32 +376,57 @@ class CallAudioManager( scoReceiver = null } + /** + * Starts the ringtone via [MediaPlayer] (not [android.media.Ringtone]). + * + * Why MediaPlayer: `Ringtone.stop()` has documented reliability + * problems on older Android versions where stop() returns but + * playback continues, and `isLooping` is only settable on API 28+. + * `MediaPlayer` has reliable stop/release on all API levels and + * supports looping universally. + */ private fun startRingtone() { try { val ringtoneUri = RingtoneManager.getDefaultUri(RingtoneManager.TYPE_RINGTONE) - ringtone = - RingtoneManager.getRingtone(context, ringtoneUri)?.apply { - audioAttributes = + val player = + MediaPlayer().apply { + setAudioAttributes( AudioAttributes .Builder() .setUsage(AudioAttributes.USAGE_NOTIFICATION_RINGTONE) .setContentType(AudioAttributes.CONTENT_TYPE_SONIFICATION) - .build() - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { - isLooping = true + .build(), + ) + setDataSource(context, ringtoneUri) + isLooping = true + setOnErrorListener { _, what, extra -> + Log.e(TAG) { "[#$instanceId] ringtone MediaPlayer error what=$what extra=$extra" } + true } - play() + prepare() + start() } - } catch (_: Exception) { + ringtonePlayer = player + Log.d(TAG) { "[#$instanceId] startRingtone: player=${player.hashCode()} started" } + } catch (e: Exception) { + Log.e(TAG, "[#$instanceId] startRingtone failed", e) } } private fun stopRingtone() { + val player = ringtonePlayer ?: return + Log.d(TAG) { "[#$instanceId] stopRingtone: player=${player.hashCode()} isPlaying=${runCatching { player.isPlaying }.getOrDefault(false)}" } try { - ringtone?.stop() - } catch (_: Exception) { + if (player.isPlaying) player.stop() + } catch (e: Exception) { + Log.e(TAG, "[#$instanceId] stopRingtone: stop() failed", e) } - ringtone = null + try { + player.release() + } catch (e: Exception) { + Log.e(TAG, "[#$instanceId] stopRingtone: release() failed", e) + } + ringtonePlayer = null } private fun startVibration() { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallNotificationReceiver.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallNotificationReceiver.kt index 1cc399b766..449840f3b0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallNotificationReceiver.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallNotificationReceiver.kt @@ -24,11 +24,14 @@ import android.content.BroadcastReceiver import android.content.Context import android.content.Intent import com.vitorpamplona.amethyst.service.call.notification.CallNotifier +import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.launch +private const val TAG = "CallNotificationReceiver" + /** * Handles the Reject action from the incoming call notification. * @@ -41,19 +44,28 @@ class CallNotificationReceiver : BroadcastReceiver() { context: Context, intent: Intent, ) { - val callManager = CallSessionBridge.callManager ?: return + Log.d(TAG) { "onReceive action=${intent.action} thread=${Thread.currentThread().name}" } + val callManager = CallSessionBridge.callManager + if (callManager == null) { + Log.e(TAG) { "onReceive: callManager is null — cannot process ${intent.action}" } + return + } val pendingResult = goAsync() val scope = CoroutineScope(SupervisorJob() + kotlinx.coroutines.Dispatchers.Main.immediate) scope.launch { try { when (intent.action) { ACTION_REJECT_CALL -> { + Log.d(TAG) { "REJECT_CALL: state=${callManager.state.value::class.simpleName}" } CallNotifier.cancelIncomingCall(context) callManager.rejectCall() + Log.d(TAG) { "REJECT_CALL: after rejectCall(), state=${callManager.state.value::class.simpleName}" } } ACTION_HANGUP_CALL -> { + Log.d(TAG) { "HANGUP_CALL: state=${callManager.state.value::class.simpleName}" } callManager.hangup() + Log.d(TAG) { "HANGUP_CALL: after hangup(), state=${callManager.state.value::class.simpleName}" } } } } finally { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt index 1b48ba93eb..c5f7abaeb4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt @@ -181,6 +181,7 @@ class CallSession( scope.launch { callManager.state.collect { state -> + Log.d(TAG) { "state collector received: ${state::class.simpleName} closed=$closed thread=${Thread.currentThread().name}" } if (closed) return@collect when (state) { is CallState.IncomingCall -> { @@ -705,6 +706,7 @@ class CallSession( * once from [onDestroy]. */ override fun close() { + Log.d(TAG) { "close() called on ${Thread.currentThread().name} state=${callManager.state.value::class.simpleName}" } // Signal the init collectors to stop touching resources. closed = true diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt index 9d825d3a41..980f8ae159 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt @@ -468,12 +468,17 @@ class CallManager( } suspend fun rejectCall() { + Log.d("CallManager") { "rejectCall: enter state=${_state.value::class.simpleName}" } val current: CallState.IncomingCall stateMutex.withLock { val s = _state.value - if (s !is CallState.IncomingCall) return + if (s !is CallState.IncomingCall) { + Log.d("CallManager") { "rejectCall: state is ${s::class.simpleName}, not IncomingCall — ignoring" } + return + } current = s transitionToEnded(current.callId, current.peerPubKeys(), EndReason.REJECTED) + Log.d("CallManager") { "rejectCall: transitioned to Ended, publishing reject events" } } val otherMembers = current.groupMembers - signer.pubKey @@ -1031,6 +1036,7 @@ class CallManager( peerPubKeys: Set, reason: EndReason, ) { + Log.d("CallManager") { "transitionToEnded: callId=$callId reason=$reason peers=${peerPubKeys.size}" } cappedAdd(completedCallIds, callId, MAX_COMPLETED_CALL_IDS) discoveredCalleePeers.clear() _state.value = CallState.Ended(callId, peerPubKeys, reason)