mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-02 21:05:28 +00:00
Ignore a second voice recording start while one is already running
The composer emits Start whenever the voice message state is Idle, and the state only leaves Idle once the recorder has produced its first audio buffer. A second tap inside that window started a second recording, which overwrote the output file, re-initialised the encoder while the first one was still encoding, and orphaned the first job together with its AudioReader. The orphan kept the microphone open and kept emitting Recording for the rest of the session, so Stop and Delete appeared to do nothing until the room was left and re-entered, and re-initialising the encoder under an active encode could crash natively. DefaultVoiceRecorder.startRecord now runs under the mutex it already owns and returns early when a recording job exists, so the second start is a no-op: no file, no encoder re-init and no second AudioReader. Relates to https://github.com/element-hq/element-x-android/issues/5780
This commit is contained in:
+12
-4
@@ -71,14 +71,22 @@ class DefaultVoiceRecorder(
|
||||
override val state: StateFlow<VoiceRecorderState> = _state
|
||||
|
||||
@RequiresPermission(Manifest.permission.RECORD_AUDIO)
|
||||
override suspend fun startRecord() {
|
||||
override suspend fun startRecord() = lock.withLock {
|
||||
// The composer can emit a second Start before the first recording has produced its first
|
||||
// buffer, i.e. before the state flips away from Idle. Without this guard the second call
|
||||
// orphans the first job and its AudioReader, which keeps the microphone open and keeps
|
||||
// emitting Recording forever, so Stop and Delete appear to do nothing.
|
||||
// See https://github.com/element-hq/element-x-android/issues/5780
|
||||
if (recordingJob != null) {
|
||||
Timber.w("Voice recorder is already recording, ignoring this start")
|
||||
return@withLock
|
||||
}
|
||||
|
||||
Timber.i("Voice recorder started recording")
|
||||
outputFile = fileManager.createFile()
|
||||
.also(encoder::init)
|
||||
|
||||
lock.withLock {
|
||||
levels.clear()
|
||||
}
|
||||
levels.clear()
|
||||
|
||||
val audioRecorder = audioReaderFactory.create(config, dispatchers).also { audioReader = it }
|
||||
|
||||
|
||||
+30
-3
@@ -39,6 +39,7 @@ import kotlin.time.TestTimeSource
|
||||
class DefaultVoiceRecorderTest {
|
||||
private val fakeFileSystem = FakeFileSystem()
|
||||
private val timeSource = TestTimeSource()
|
||||
private val audioReaderFactory = FakeAudioReaderFactory(audio = AUDIO)
|
||||
|
||||
@Test
|
||||
fun `it emits the initial state`() = runTest {
|
||||
@@ -109,6 +110,34 @@ class DefaultVoiceRecorderTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `when startRecord is called twice, the second call is ignored`() = runTest {
|
||||
val voiceRecorder = createDefaultVoiceRecorder()
|
||||
voiceRecorder.state.test {
|
||||
assertThat(awaitItem()).isEqualTo(VoiceRecorderState.Idle)
|
||||
|
||||
// Two taps landing before the state has flipped away from Idle.
|
||||
voiceRecorder.startRecord()
|
||||
voiceRecorder.startRecord()
|
||||
assertThat(audioReaderFactory.createdCount).isEqualTo(1)
|
||||
|
||||
skipItems(1)
|
||||
timeSource += 5.seconds
|
||||
skipItems(2)
|
||||
voiceRecorder.stopRecord()
|
||||
assertThat(awaitItem()).isEqualTo(
|
||||
VoiceRecorderState.Finished(
|
||||
file = File(FILE_PATH),
|
||||
mimeType = MimeTypes.Ogg,
|
||||
waveform = List(100) { 1f },
|
||||
duration = 5.seconds,
|
||||
)
|
||||
)
|
||||
// An orphaned recording job would keep emitting Recording after the stop.
|
||||
expectNoEvents()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `when cancelled, it deletes the file`() = runTest {
|
||||
val voiceRecorder = createDefaultVoiceRecorder()
|
||||
@@ -128,9 +157,7 @@ class DefaultVoiceRecorderTest {
|
||||
return DefaultVoiceRecorder(
|
||||
dispatchers = testCoroutineDispatchers(),
|
||||
timeSource = timeSource,
|
||||
audioReaderFactory = FakeAudioReaderFactory(
|
||||
audio = AUDIO,
|
||||
),
|
||||
audioReaderFactory = audioReaderFactory,
|
||||
encoder = FakeEncoder(fakeFileSystem),
|
||||
config = AudioConfig(
|
||||
format = audioFormat,
|
||||
|
||||
+5
@@ -16,7 +16,12 @@ import io.element.android.libraries.voicerecorder.impl.audio.AudioReader
|
||||
class FakeAudioReaderFactory(
|
||||
private val audio: List<Audio>
|
||||
) : AudioReader.Factory {
|
||||
/** Number of readers created so far, to assert that a second recording is never started. */
|
||||
var createdCount: Int = 0
|
||||
private set
|
||||
|
||||
override fun create(config: AudioConfig, dispatchers: CoroutineDispatchers): AudioReader {
|
||||
createdCount++
|
||||
return FakeAudioReader(dispatchers, audio)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user