diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordAudio.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordAudio.kt index 1d8f2ff23b..6d5c7d8964 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordAudio.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordAudio.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.ui.actions.uploads import android.Manifest import android.widget.Toast import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf @@ -39,6 +40,7 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.ui.components.ClickAndHoldBoxComposable import com.vitorpamplona.amethyst.ui.stringRes import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive @OptIn(ExperimentalPermissionsApi::class) @Composable @@ -50,15 +52,41 @@ fun RecordAudioBox( val mediaRecorder = remember { mutableStateOf(null) } val context = LocalContext.current var elapsedSeconds by remember { mutableIntStateOf(0) } + var wantsToRecord by remember { mutableStateOf(false) } + + // Must be called at Composable scope, not in callback + val recordPermissionState = rememberPermissionState(Manifest.permission.RECORD_AUDIO) + val scope = rememberCoroutineScope() + + DisposableEffect(Unit) { + onDispose { + wantsToRecord = false + mediaRecorder.value?.stop() + mediaRecorder.value = null + } + } + + // Start recording once permission is granted AND user wants to record + LaunchedEffect(recordPermissionState.status.isGranted, wantsToRecord) { + if (recordPermissionState.status.isGranted && wantsToRecord && mediaRecorder.value == null) { + elapsedSeconds = 0 + mediaRecorder.value = VoiceMessageRecorder() + mediaRecorder.value?.start(context, scope) + } + } // Track elapsed time while recording LaunchedEffect(mediaRecorder.value) { - if (mediaRecorder.value != null) { - while (mediaRecorder.value != null) { + // Capture the current recorder state to avoid repeated reads of volatile state + val currentRecorder = mediaRecorder.value + if (currentRecorder != null) { + // Loop while coroutine is active - LaunchedEffect will cancel when mediaRecorder.value changes + while (isActive) { delay(1000) elapsedSeconds++ } } else { + // Reset elapsed time when not recording elapsedSeconds = 0 } } @@ -66,19 +94,17 @@ fun RecordAudioBox( ClickAndHoldBoxComposable( modifier = modifier, onPress = { - val recordPermissionState = rememberPermissionState(Manifest.permission.RECORD_AUDIO) - val scope = rememberCoroutineScope() - LaunchedEffect(Unit) { - if (!recordPermissionState.status.isGranted) { - recordPermissionState.launchPermissionRequest() - } else { - elapsedSeconds = 0 - mediaRecorder.value = VoiceMessageRecorder() - mediaRecorder.value?.start(context, scope) - } + wantsToRecord = true + if (!recordPermissionState.status.isGranted) { + recordPermissionState.launchPermissionRequest() + } else if (mediaRecorder.value == null) { + elapsedSeconds = 0 + mediaRecorder.value = VoiceMessageRecorder() + mediaRecorder.value?.start(context, scope) } }, onRelease = { + wantsToRecord = false val result = mediaRecorder.value?.stop() mediaRecorder.value = null if (result != null) { @@ -94,6 +120,7 @@ fun RecordAudioBox( } }, onCancel = { + wantsToRecord = false mediaRecorder.value?.stop() mediaRecorder.value = null }, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordVoiceButton.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordVoiceButton.kt index f390eea3ed..9cedeeab6a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordVoiceButton.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordVoiceButton.kt @@ -29,6 +29,7 @@ import androidx.compose.material.icons.filled.Mic import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.SideEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf @@ -61,12 +62,14 @@ fun RecordVoiceButton(onVoiceTaken: (RecordingResult) -> Unit) { onVoiceTaken(recording) }, ) { recordingState, elapsed -> - // Update parent state to trigger recompositions - if (isRecording != recordingState) { - isRecording = recordingState - } - if (elapsedSeconds != elapsed) { - elapsedSeconds = elapsed + // Update parent state after composition completes + SideEffect { + if (isRecording != recordingState) { + isRecording = recordingState + } + if (elapsedSeconds != elapsed) { + elapsedSeconds = elapsed + } } Box( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordingIndicators.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordingIndicators.kt index ae2c25aab7..45b41fb7dc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordingIndicators.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/RecordingIndicators.kt @@ -228,7 +228,7 @@ fun FloatingRecordingIndicator( ) Text( - text = "Recording ${formatSeconds(elapsedSeconds)}", + text = "Recording ${formatSecondsToTime(elapsedSeconds)}", color = Color.White, fontSize = 14.sp, fontWeight = FontWeight.Medium, @@ -236,13 +236,3 @@ fun FloatingRecordingIndicator( } } } - -private fun formatSeconds(seconds: Int): String { - val minutes = seconds / 60 - val secs = seconds % 60 - return if (minutes > 0) { - String.format("%d:%02d", minutes, secs) - } else { - String.format("0:%02d", secs) - } -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/TimeFormatUtils.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/TimeFormatUtils.kt new file mode 100644 index 0000000000..80cac54e02 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/TimeFormatUtils.kt @@ -0,0 +1,39 @@ +/** + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.actions.uploads + +import java.util.Locale + +/** + * Formats seconds into a human-readable time string (M:SS or MM:SS format). + * + * @param seconds The number of seconds to format + * @return Formatted time string (e.g., "0:05", "1:23", "12:45") + */ +fun formatSecondsToTime(seconds: Int): String { + val minutes = seconds / 60 + val secs = seconds % 60 + return if (minutes > 0) { + String.format(Locale.getDefault(), "%d:%02d", minutes, secs) + } else { + String.format(Locale.getDefault(), "0:%02d", secs) + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessagePreview.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessagePreview.kt index bff7e3efbf..7a2fb1759e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessagePreview.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessagePreview.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.ui.actions.uploads import android.content.Context import android.media.MediaPlayer +import android.util.Log import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -77,9 +78,20 @@ fun VoiceMessagePreview( // Initialize MediaPlayer DisposableEffect(voiceMetadata.url, localFile) { val player = createMediaPlayer(context, voiceMetadata.url, localFile) + player?.setOnCompletionListener { + isPlaying = false + progress = 0f + } mediaPlayer = player onDispose { + // Stop playback and clean up + try { + player?.stop() + } catch (e: IllegalStateException) { + // Player might already be stopped + Log.d("VoiceMessagePreview", "MediaPlayer stop failed (already stopped)", e) + } player?.release() mediaPlayer = null isPlaying = false @@ -87,19 +99,37 @@ fun VoiceMessagePreview( } // Update progress while playing - LaunchedEffect(isPlaying) { - if (isPlaying && mediaPlayer != null) { - while (isActive && isPlaying) { - val player = mediaPlayer - if (player != null && player.isPlaying) { - val current = player.currentPosition.toFloat() - val duration = player.duration.toFloat() - progress = if (duration > 0) current / duration else 0f - delay(100) - } else { + LaunchedEffect(mediaPlayer, isPlaying) { + // Capture player reference to avoid reading volatile state repeatedly + val player = mediaPlayer + if (player != null && isPlaying) { + while (isActive) { + try { + if (player.isPlaying) { + val current = player.currentPosition.toFloat() + val duration = player.duration.toFloat() + // Validate values before calculating progress + val newProgress = + if (duration > 0 && current >= 0) { + (current / duration).coerceIn(0f, 1f) + } else { + 0f + } + // Only update if value is valid (not NaN or Infinity) + if (newProgress.isFinite()) { + progress = newProgress + } + } else { + // Player stopped, exit loop and let LaunchedEffect restart + break + } + } catch (e: IllegalStateException) { + // Player in invalid state, stop tracking + Log.w("VoiceMessagePreview", "MediaPlayer in invalid state during progress tracking", e) isPlaying = false - progress = 0f + break } + delay(100) } } } @@ -121,17 +151,25 @@ fun VoiceMessagePreview( // Play/Pause Button IconButton( onClick = { - mediaPlayer?.let { player -> - if (isPlaying) { - player.pause() - isPlaying = false - } else { - if (progress >= 1f) { - player.seekTo(0) - progress = 0f + val player = mediaPlayer + if (player != null) { + try { + if (isPlaying) { + player.pause() + isPlaying = false + } else { + // Validate progress before comparison + if (progress.isFinite() && progress >= 1f) { + player.seekTo(0) + progress = 0f + } + player.start() + isPlaying = true } - player.start() - isPlaying = true + } catch (e: IllegalStateException) { + // MediaPlayer in invalid state, ignore + Log.w("VoiceMessagePreview", "MediaPlayer operation failed in onClick handler", e) + isPlaying = false } } }, @@ -157,16 +195,29 @@ fun VoiceMessagePreview( waveformBrush = Brush.linearGradient(listOf(MaterialTheme.colorScheme.onSurfaceVariant, MaterialTheme.colorScheme.onSurfaceVariant)), progressBrush = Brush.linearGradient(listOf(MaterialTheme.colorScheme.primary, MaterialTheme.colorScheme.primary)), onProgressChange = { newProgress -> - mediaPlayer?.let { player -> - val newPosition = (newProgress * player.duration).toInt() - player.seekTo(newPosition) - progress = newProgress + // Validate incoming progress value + if (newProgress.isFinite() && newProgress >= 0f && newProgress <= 1f) { + val player = mediaPlayer + if (player != null) { + try { + val duration = player.duration + // Only seek if duration is valid + if (duration > 0) { + val newPosition = (newProgress * duration).toInt() + player.seekTo(newPosition) + progress = newProgress + } + } catch (e: IllegalStateException) { + // MediaPlayer in invalid state, ignore + Log.w("VoiceMessagePreview", "MediaPlayer seek failed in onProgressChange", e) + } + } } }, ) Text( - text = formatDuration(voiceMetadata.duration ?: 0), + text = formatSecondsToTime(voiceMetadata.duration ?: 0), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.padding(top = 4.dp), @@ -203,17 +254,7 @@ private fun createMediaPlayer( setDataSource(url) } prepare() - setOnCompletionListener { - // Reset to beginning when playback completes - seekTo(0) - } } } catch (e: Exception) { null } - -private fun formatDuration(seconds: Int): String { - val minutes = seconds / 60 - val secs = seconds % 60 - return String.format("%d:%02d", minutes, secs) -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessageRecorder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessageRecorder.kt index 6e5323cc41..dae416df77 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessageRecorder.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/uploads/VoiceMessageRecorder.kt @@ -23,13 +23,17 @@ package com.vitorpamplona.amethyst.ui.actions.uploads import android.content.Context import android.media.MediaRecorder import android.os.Build -import android.util.Log import androidx.media3.common.MimeTypes +import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.RandomInstance import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import java.io.File @@ -41,30 +45,44 @@ class RecordingResult( ) class VoiceMessageRecorder { + @Volatile private var recorder: MediaRecorder? = null private var outputFile: File? = null private var startTime: Long = 0 - private var job: Job? = null + + // Own scope to manage lifecycle independently from caller + private var recorderScope: CoroutineScope? = null + + @Volatile + private var amplitudeSamplingJob: Job? = null private var amplitudes: MutableList = mutableListOf() private fun createRecorder(context: Context): MediaRecorder = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { - MediaRecorder(context) + MediaRecorder(context.applicationContext) } else { MediaRecorder() } + @Synchronized fun start( context: Context, - scope: CoroutineScope, + parentScope: CoroutineScope, ) { + // Clean up any existing recording first + cleanup() + val fileName = RandomInstance.randomChars(16) + ".mp4" - val outputFile = File(context.cacheDir, "/voice/$fileName") + val outputFile = File(context.cacheDir, "voice/$fileName") outputFile.parentFile?.mkdirs() this.outputFile = outputFile this.startTime = TimeUtils.now() this.amplitudes.clear() + // Create own scope with SupervisorJob so failures don't cascade + val scopeJob = SupervisorJob(parentScope.coroutineContext[Job]) + recorderScope = CoroutineScope(Dispatchers.Main.immediate + scopeJob) + createRecorder(context).apply { setAudioEncodingBitRate(16 * 44100) setAudioSamplingRate(44100) // Set the desired audio sampling rate (e.g., 44.1 kHz) @@ -79,38 +97,103 @@ class VoiceMessageRecorder { recorder = this } - job?.cancel() - job = - scope.launch { - while (recorder != null) { - amplitudes.add(recorder?.maxAmplitude?.toFloat() ?: 0f) + // Launch amplitude sampling in our own scope + amplitudeSamplingJob = + recorderScope?.launch { + while (isActive) { + val recorderRef = recorder ?: break + try { + val amplitude = recorderRef.maxAmplitude.toFloat() + synchronized(amplitudes) { + amplitudes.add(amplitude) + } + } catch (e: IllegalStateException) { + // MediaRecorder might be in invalid state, stop sampling + Log.w("VoiceMessageRecorder", "MediaRecorder in invalid state during amplitude sampling", e) + break + } delay(1000) } } } + @Synchronized fun stop(): RecordingResult? { - job?.cancel() - job = null - - try { - recorder?.stop() - } catch (e: RuntimeException) { - Log.w("VoiceMessageRecorder", "Failed to stop recording... Too short?", e) + if (recorder == null) { + cleanup() + return null } - recorder?.reset() - recorder = null val currentTime = TimeUtils.now() val file = outputFile - return if (currentTime - startTime >= 1 && file != null) { + + // Capture amplitudes before cleanup + val amplitudesCopy = + synchronized(amplitudes) { + amplitudes.toList() + } + val duration = (currentTime - startTime).toInt() + + // Clean up recorder and scope + cleanup() + + return if (duration >= 1 && file != null) { RecordingResult( file, MimeTypes.AUDIO_AAC, - amplitudes, - (currentTime - startTime).toInt(), + amplitudesCopy, + duration, ) } else { null } } + + /** + * Cleans up all resources: stops recorder, cancels jobs, cancels scope. + * Safe to call multiple times. + */ + @Synchronized + private fun cleanup() { + // Cancel amplitude sampling job + amplitudeSamplingJob?.cancel() + amplitudeSamplingJob = null + + // Stop any remaining coroutines before touching the recorder + recorderScope?.cancel() + recorderScope = null + + // Swap local reference so we always null out the volatile field + val recorderToRelease = recorder + recorder = null + + recorderToRelease?.let { mediaRecorder -> + try { + mediaRecorder.stop() + } catch (e: IllegalStateException) { + Log.w("VoiceMessageRecorder", "Failed to stop MediaRecorder due to illegal state", e) + } catch (e: RuntimeException) { + // MediaRecorder.stop() can throw RuntimeException if the recording is too short + // or if no valid audio data was captured. This is a known Android issue. + Log.w("VoiceMessageRecorder", "Failed to stop MediaRecorder (recording may be too short or invalid)", e) + } finally { + try { + mediaRecorder.reset() + } catch (resetError: Exception) { + Log.w("VoiceMessageRecorder", "Failed to reset MediaRecorder before release", resetError) + } + try { + mediaRecorder.release() + } catch (releaseError: Exception) { + Log.w("VoiceMessageRecorder", "Failed to release MediaRecorder resources", releaseError) + } + } + } + + // Reset transient state so a fresh recording always starts cleanly + outputFile = null + startTime = 0 + synchronized(amplitudes) { + amplitudes.clear() + } + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ClickableBox.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ClickableBox.kt index 100f123985..6df79b5cda 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ClickableBox.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ClickableBox.kt @@ -115,7 +115,7 @@ fun ClickAndHoldBox( @Composable fun ClickAndHoldBoxComposable( modifier: Modifier = Modifier, - onPress: @Composable () -> Unit, + onPress: () -> Unit, onRelease: suspend () -> Unit, onCancel: suspend () -> Unit, content: @Composable (Boolean) -> Unit, @@ -123,15 +123,16 @@ fun ClickAndHoldBoxComposable( val interactionSource = remember { MutableInteractionSource() } var isPressed by remember { mutableStateOf(false) } - if (isPressed) { - onPress() - } - LaunchedEffect(interactionSource) { val pressInteractions = mutableListOf() interactionSource.interactions.collect { interaction -> when (interaction) { - is PressInteraction.Press -> pressInteractions.add(interaction) + is PressInteraction.Press -> { + if (pressInteractions.isEmpty()) { + onPress() + } + pressInteractions.add(interaction) + } is PressInteraction.Release -> { onRelease() pressInteractions.remove(interaction.press)