Fix rate conditions between subsequent recordings

reduce duplicate code
fix a few coding issues (state management, callback usage, scope management)

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<VoiceMessageRecorder?>(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
         },

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(

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)
-    }
-}

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)
+    }
+}

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)
-}

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<Float> = 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()
+        }
+    }
 }

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<PressInteraction.Press>()
         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)