From 5ed76dba3134a0fc80ee9b1d2fc723a1ca12b60b Mon Sep 17 00:00:00 2001 From: QIU Date: Thu, 27 Aug 2026 15:01:04 +0000 Subject: [PATCH] fix(cw): the record deleted its own text while decoding The record pane concatenated the live decode onto the archived text. The live decode is the 20 s window, replaced wholesale every 1.5 s because DeepCW is a whole-segment CTC model that rewrites earlier characters as more context arrives. So the tail of the record kept changing and could get shorter - text vanishing from under the operator while the decoder was still running. A previous attempt (828fd0fb) added decodePending() to cover the gap while audio waits to be archived, but the call site was never wired up. The function had no callers and pendingText was only ever cleared, never assigned, so that fix has never once run and the gap it targeted stayed open. Both are deleted here. The record now binds to archived text only, which is append-only, so it cannot shrink. That moves the whole problem to latency, which was 20 s window + 15 s batch: nothing at all in the record for the first 35 s of a session, and thereafter a stall of up to 15 s each cycle. The batch is now 4 s, holding the stall under the ~4.7 s a seven-character call sign takes at 18 WPM, while the archive path still fires less than half as often as the live redecode. Neither holding place drains on its own: the live window only reaches the archive by being pushed out by newer audio, and the pending batch only by filling up. So pausing or leaving the screen discarded whatever was in flight - the end of every transmission, the part with the call sign in it. flush() archives both, pending batch first so the text is not transposed, and is called on pause and before close(). On the way out it runs on appScope, because the screen's own scope is cancelled as it leaves and would abort the decode. Also: the record was an unlabelled grey box showing a bare ellipsis, which reads as a disabled text field. It now has a label, an empty state that says what it is for, and a copy button - until now there was no way to get the decoded text off the screen at all, so an operator who had just copied a call sign by ear had to transcribe it a second time by hand. CwArchiveTimingTest covers the timing against the real constants rather than copies; it caught a 5 s batch exceeding the call-sign bound during this change. The archive path had no test coverage before. --- .../look4sat/core/data/cw/CwDeepDecoder.kt | 100 +++++++----- .../core/data/cw/CwArchiveTimingTest.kt | 150 ++++++++++++++++++ .../look4sat/core/domain/cw/ICwDecoder.kt | 19 ++- .../look4sat/feature/cw/CwDecodeScreen.kt | 62 +++++++- .../cw/src/main/res/values-zh/app_values.xml | 4 + feature/cw/src/main/res/values/app_values.xml | 4 + 6 files changed, 293 insertions(+), 46 deletions(-) create mode 100644 core/data/src/test/java/com/rtbishop/look4sat/core/data/cw/CwArchiveTimingTest.kt diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/cw/CwDeepDecoder.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/cw/CwDeepDecoder.kt index 021db98b..95cc1c03 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/cw/CwDeepDecoder.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/cw/CwDeepDecoder.kt @@ -65,13 +65,28 @@ class CwDeepDecoder( private val isToneShiftEnabled: () -> Boolean = { false } ) : ICwDecoder { - private companion object { + // internal, not private: the archive timing is user-visible behaviour and its test asserts + // against these constants directly rather than a copy that could silently drift. + internal companion object { const val TAG = "CwDeepDecoder" const val MODEL_ASSET = "deepcw/model.onnx" const val METADATA_ASSET = "deepcw/model.onnx.json" - /** Evicted audio is decoded into permanent history once this much accumulates. */ - const val ARCHIVE_SECONDS = 15.0 + /** + * Evicted audio is decoded into permanent history once this much accumulates. + * + * This is the delay before decoded text reaches the record, and it is additive with + * the 20 s live window: at the old 15 s the record showed nothing for the first 35 s + * of a session, and thereafter text that had scrolled out of the live window sat + * invisible for up to 15 s before landing - the record appeared to stall and, when + * it was still concatenating the live window, to delete what it had just shown. + * + * Each batch is one full inference, so this trades CPU for latency. 4 s holds the gap + * under the ~4.7 s a seven-character call sign takes at 18 WPM - the record must not + * stall for longer than the one thing an operator most needs to read back - while the + * archive path still fires less than half as often as the 1.5 s live redecode cycle. + */ + const val ARCHIVE_SECONDS = 4.0 val ARCHIVE_THRESHOLD: Int = (CwDeepSpectrogram.SAMPLE_RATE * ARCHIVE_SECONDS).toInt() /** @@ -105,13 +120,12 @@ class CwDeepDecoder( override val decodedText: StateFlow = _decodedText.asStateFlow() /** - * Archived text, plus a provisional decode of audio not yet archived. + * Archived text: only appended to, so a pane bound to it never loses what it showed. * - * Kept as one flow of the two parts concatenated. Without the provisional part the - * transcript visibly shrank: audio leaving the 20 s window waits for a full - * [ARCHIVE_SECONDS] batch before it is decoded into the archive, so for up to 15 s its - * characters were in neither place - measured, up to 30 characters at 20 WPM would - * vanish and reappear later, which reads as the box deleting text. + * This once carried a provisional tail meant to cover the gap while audio waited to be + * archived, but the decode feeding that tail was never wired up, so the tail was always + * empty and the gap stayed. It is closed instead by archiving in [ARCHIVE_SECONDS] + * batches, which no longer concatenate anything that gets rewritten. */ private val _historyText = MutableStateFlow("") override val historyText: StateFlow = _historyText.asStateFlow() @@ -119,9 +133,6 @@ class CwDeepDecoder( /** Permanently archived text; the provisional tail is appended to this for display. */ private var committedText = "" - /** Provisional decode of the pending archive batch, replaced when it is archived. */ - private var pendingText = "" - private val _estimatedPitch = MutableStateFlow(null) override val estimatedPitch: StateFlow = _estimatedPitch.asStateFlow() @@ -143,11 +154,13 @@ class CwDeepDecoder( private val buffer = CwDeepBuffer() /** - * Evicted audio accumulates here until it reaches [ARCHIVE_SECONDS], then - * is decoded once and appended to [historyText]. Archiving in ~15 s chunks - * keeps the extra inference cheap (short window) while long enough to be - * decoded accurately — the content has already been through the 20 s window - * many times, so a slightly shorter archive decode loses almost nothing. + * Evicted audio accumulates here until it reaches [ARCHIVE_SECONDS], then is decoded + * once and appended to [historyText]. The batch stays short enough that the record does + * not visibly stall, and accuracy barely suffers: this content has already been through + * the 20 s window many times, so the archive decode is a confirmation, not a first look. + * + * Sized to the full window rather than the batch: [flush] hands over whatever the live + * window holds, which can be the whole 20 s. */ private val archiveBuffer = FloatArray(CwDeepBuffer.DEFAULT_MAX_SECONDS.toInt() * CwDeepSpectrogram.SAMPLE_RATE) private var archiveSize = 0 @@ -410,9 +423,7 @@ class CwDeepDecoder( private fun dropBufferedAudio() { buffer.reset() archiveSize = 0 - // The provisional text describes audio being discarded, so it goes with it. // Committed text stays: it was correct for audio that really was archived. - pendingText = "" _historyText.value = committedText } @@ -497,6 +508,37 @@ class CwDeepDecoder( updateSignalMetrics(spectrogram) } + /** + * Archive whatever audio is still in the pipeline, so stopping does not discard it. + * + * Two places hold audio that would otherwise never be decoded into the record: the + * batch accumulating towards [ARCHIVE_THRESHOLD], and the live window itself, whose + * contents only ever reach the archive by being pushed out by newer audio. Together + * that is the last [CwDeepBuffer.DEFAULT_MAX_SECONDS] + [ARCHIVE_SECONDS] of a session + * - which includes the end of every transmission, the part with the call sign in it. + * + * Order matters: the pending batch left the window before anything still in it, so it + * has to be archived first or the record comes out with its text transposed. + */ + override suspend fun flush() { + if (archiveSize > 0) { + val pending = archiveBuffer.copyOf(archiveSize) + archiveSize = 0 + runCatching { archiveDecode(pending) } + .onFailure { if (it is CancellationException) throw it } + } + // Draining the window empties it, so a second flush cannot double-archive the tail. + val window = buffer.snapshot() + if (window.isNotEmpty()) { + buffer.reset() + runCatching { archiveDecode(window) } + .onFailure { if (it is CancellationException) throw it } + } + // The live line described audio that is now in the record; leaving it would show the + // same characters twice, in two places, one of them stale. + _decodedText.value = "" + } + /** * Decode a chunk of audio that has scrolled out of the live window and * append it to [historyText]. Unlike the live window this never replaces — @@ -508,29 +550,10 @@ class CwDeepDecoder( if (audio.size < CwDeepSpectrogram.FFT_LENGTH) return@withContext val spectrogram = CwDeepSpectrogram.compute(audio) val text = runInference(activeSession, activeEnvironment, spectrogram) - // This batch is final, so its provisional decode is superseded rather than kept. committedText += text - pendingText = "" _historyText.value = committedText } - /** - * Decode the audio waiting to be archived, so it stays on screen until it is. - * - * Provisional: the batch is still growing, and the final decode sees all of it at once - * with more context. Runs on the redecode cycle rather than per capture chunk - - * decoding every 100 ms chunk separately measured 14x the inference load, over 250% of - * one core, and a chunk that short carries under two dot-lengths of context anyway. - */ - private suspend fun decodePending(audio: FloatArray) = withContext(Dispatchers.Default) { - val activeSession = session ?: return@withContext - val activeEnvironment = environment ?: return@withContext - if (audio.size < CwDeepSpectrogram.FFT_LENGTH) return@withContext - val spectrogram = CwDeepSpectrogram.compute(audio) - pendingText = runInference(activeSession, activeEnvironment, spectrogram) - _historyText.value = committedText + pendingText - } - /** Run the ONNX model over a pre-computed spectrogram and return the decoded text. */ private fun runInference( activeSession: OrtSession, @@ -611,7 +634,6 @@ class CwDeepDecoder( _decodedText.value = "" _historyText.value = "" committedText = "" - pendingText = "" archiveSize = 0 _estimatedPitch.value = null _detectedToneHz.value = null diff --git a/core/data/src/test/java/com/rtbishop/look4sat/core/data/cw/CwArchiveTimingTest.kt b/core/data/src/test/java/com/rtbishop/look4sat/core/data/cw/CwArchiveTimingTest.kt new file mode 100644 index 00000000..8e74b9ad --- /dev/null +++ b/core/data/src/test/java/com/rtbishop/look4sat/core/data/cw/CwArchiveTimingTest.kt @@ -0,0 +1,150 @@ +/* + * Look4Sat. Amateur radio satellite tracker and pass predictor. + * Copyright (C) 2019-2026 Arty Bishop and contributors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.rtbishop.look4sat.core.data.cw + +import com.rtbishop.look4sat.core.domain.cw.CwDeepBuffer +import com.rtbishop.look4sat.core.domain.cw.CwDeepSpectrogram +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import kotlin.math.floor + +/** + * How long audio waits before its text can reach the record pane. + * + * The record binds to archived text only. The live decode is rewritten from scratch every + * cycle, so a pane that concatenated it lost characters the operator had already read - the + * decoder appeared to delete its own output while still running. Binding to archived text + * makes the pane monotonic, and the cost is latency, which is additive: audio must first be + * pushed out of the live window, then accumulate into a full archive batch. + * + * [CwDeepDecoder] needs a Context and a loaded ONNX model, so it cannot be constructed here. + * These tests read its real constants rather than copies, so retuning one without + * reconsidering the user-visible delay fails here. + */ +class CwArchiveTimingTest { + + /** Sending speed for the character counts, typical for satellite CW. */ + private val wpm = 18.0 + + /** PARIS standard: one word is five characters. */ + private val charsPerSecond = wpm * 5 / 60.0 + + private val window = CwDeepBuffer.DEFAULT_MAX_SECONDS + private val batch = CwDeepDecoder.ARCHIVE_SECONDS + + /** Seconds of audio that have reached the archive after listening for [elapsed]. */ + private fun archivedSeconds(elapsed: Double): Double { + val evicted = elapsed - window + if (evicted <= 0.0) return 0.0 + return floor(evicted / batch) * batch + } + + @Test + fun thresholdMatchesTheDeclaredBatchLength() { + assertEquals( + "threshold must be the batch length in samples", + (CwDeepSpectrogram.SAMPLE_RATE * batch).toInt(), + CwDeepDecoder.ARCHIVE_THRESHOLD + ) + } + + @Test + fun archiveBatchIsShorterThanACallSign() { + // A seven-character call sign at 18 WPM takes about 4.7 s. A batch longer than that + // means the record can stall for longer than the single most important thing being + // sent, which is what made the stall read as deletion. + val callSignSeconds = 7 / charsPerSecond + assertTrue( + "batch $batch s must not exceed a call sign at $wpm WPM " + + "(${"%.1f".format(callSignSeconds)} s)", + batch <= callSignSeconds + ) + } + + @Test + fun firstTextReachesTheRecordWithinHalfAMinute() { + // At the previous 15 s batch this was 35 s, so a short exchange ended with the record + // still completely empty: every decoded character had only ever been in the live line, + // which shows 64 characters and overwrites them. + val firstArchive = window + batch + assertTrue("first archived text must appear within 30 s, got $firstArchive s", firstArchive <= 30.0) + } + + @Test + fun aThirtySecondSessionStillProducesARecord() { + val archived = archivedSeconds(30.0) + assertTrue("30 s of listening must archive something, got $archived s", archived > 0.0) + } + + @Test + fun theRecordNeverStallsForLongerThanOneBatch() { + var longestStall = 0.0 + var lastGrowthAt = window + var previous = 0.0 + var t = window + while (t <= 600.0) { + val archived = archivedSeconds(t) + if (archived > previous) { + longestStall = maxOf(longestStall, t - lastGrowthAt) + lastGrowthAt = t + previous = archived + } + t += 0.1 + } + assertTrue( + "record stalled ${"%.1f".format(longestStall)} s, one batch is $batch s", + longestStall <= batch + 0.11 + ) + } + + @Test + fun audioInFlightWhenCaptureStopsWouldLoseTheEndOfTheTransmission() { + // Why flush() exists. Neither holding place drains on its own: the live window only + // reaches the archive by being pushed out by newer audio, and the pending batch only + // by filling up. Both hold the end of the transmission, where the call sign is. + val worstCase = window + batch + val lostCharacters = worstCase * charsPerSecond + assertTrue( + "flush() must exist: ${"%.0f".format(lostCharacters)} characters would be lost", + lostCharacters > 20 + ) + } + + @Test + fun archivingStaysRarerThanTheLiveDecode() { + // Each batch is one inference. Shortening the batch trades CPU for latency, so it must + // stay rarer than the live redecode or the archive path becomes the dominant cost. + val liveIntervalSeconds = CwDeepBuffer.DEFAULT_REDECODE_INTERVAL_MS / 1000.0 + assertTrue( + "batch $batch s must stay longer than the live cycle $liveIntervalSeconds s", + batch > liveIntervalSeconds + ) + } + + @Test + fun aBatchIsLongEnoughToDecode() { + // compute() rejects audio shorter than one FFT frame, so a batch below that would be + // silently dropped by archiveDecode's size guard and its text lost outright. + assertTrue( + "batch of ${CwDeepDecoder.ARCHIVE_THRESHOLD} samples must exceed " + + "FFT_LENGTH ${CwDeepSpectrogram.FFT_LENGTH}", + CwDeepDecoder.ARCHIVE_THRESHOLD > CwDeepSpectrogram.FFT_LENGTH + ) + } +} diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/cw/ICwDecoder.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/cw/ICwDecoder.kt index 0746ceb3..c2c95353 100644 --- a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/cw/ICwDecoder.kt +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/cw/ICwDecoder.kt @@ -37,9 +37,11 @@ interface ICwDecoder { val decodedText: StateFlow /** - * Permanent transcript of everything that has scrolled out of the live - * window. Unlike [decodedText] this only ever grows (until [reset]); it is - * what the user reads back after a signal has passed. + * Transcript of audio that has been archived, and will not be revised. + * + * Only ever grows until [reset]. [decodedText] is rewritten from scratch on every + * redecode, so a pane that concatenates it loses text the operator has already read - + * which a paper log does not do. This is the flow such a pane must bind to. */ val historyText: StateFlow @@ -74,6 +76,17 @@ interface ICwDecoder { /** Non-null when the decoder cannot run, for example the model failed to load. */ val errorMessage: StateFlow + /** + * Decode whatever audio is still held in the pipeline into [historyText]. + * + * Nothing reaches [historyText] until audio has been pushed out of the live window and + * then accumulated into a full archive batch, so the last stretch of a session is always + * still in flight when capture stops - and neither holding place drains on its own. That + * stretch is the end of the transmission, the part with the call sign in it. Call on + * pause and before [close]. + */ + suspend fun flush() + /** Feed captured mono PCM in -1..1. Safe to call from a capture thread. */ suspend fun processBuffer(samples: FloatArray, sampleRate: Int) diff --git a/feature/cw/src/main/java/com/rtbishop/look4sat/feature/cw/CwDecodeScreen.kt b/feature/cw/src/main/java/com/rtbishop/look4sat/feature/cw/CwDecodeScreen.kt index 8f77c306..77395e7f 100644 --- a/feature/cw/src/main/java/com/rtbishop/look4sat/feature/cw/CwDecodeScreen.kt +++ b/feature/cw/src/main/java/com/rtbishop/look4sat/feature/cw/CwDecodeScreen.kt @@ -58,9 +58,11 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.platform.LocalClipboardManager import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp @@ -108,6 +110,12 @@ fun CwDecodeScreen() { // Hoisted so the toolbar button can reach it: the record below does not follow // the decode, so jumping to the newest text has to be an explicit action. val transcriptScroll = rememberScrollState() + val clipboard = LocalClipboardManager.current + val showToast = remember { container.provideShowToast() } + val copiedMessage = stringResource(R.string.cw_copied) + val emptyRecordMessage = stringResource(R.string.cw_record_empty) + // Archiving the tail on the way out has to outlive this composable's own scope. + val appScope = container.appScope val decodedText by decoder.decodedText.collectAsState() val historyText by decoder.historyText.collectAsState() @@ -130,7 +138,13 @@ fun CwDecodeScreen() { // permission so granting the permission mid-flow restarts collection // (isListening may already be true when the launcher returns). LaunchedEffect(isListening, permissionGranted) { - if (!isListening) return@LaunchedEffect + if (!isListening) { + // Pausing must not throw away the tail. The live window plus the batch waiting to + // be archived are both still holding audio at this point, and nothing else ever + // decodes either of them, so the end of the transmission would be lost outright. + decoder.flush() + return@LaunchedEffect + } if (!permissionGranted) { permissionLauncher.launch(Manifest.permission.RECORD_AUDIO) return@LaunchedEffect @@ -143,8 +157,14 @@ fun CwDecodeScreen() { DisposableEffect(Unit) { onDispose { - // OrtSession holds native memory and must be released explicitly. - decoder.close() + // Archive the tail before tearing down, on a scope that outlives this composable: + // the screen's own scope is cancelled as it leaves, which would abort the decode. + // close() waits for it, because the inference needs the session it is closing. + appScope.launch { + runCatching { decoder.flush() } + // OrtSession holds native memory and must be released explicitly. + decoder.close() + } } } @@ -162,6 +182,24 @@ fun CwDecodeScreen() { .weight(1f) .padding(start = 12.dp) ) + // Copy the record out. The decoder is built fresh every time this screen is opened, + // so leaving the screen loses the transcript - and until now there was no way at all + // to get the text off it. An operator who has just copied a callsign by ear should not + // have to transcribe it a second time by hand. + IconButton( + onClick = { + if (historyText.isNotEmpty()) { + clipboard.setText(AnnotatedString(historyText)) + showToast(copiedMessage) + } + }, + enabled = historyText.isNotEmpty() + ) { + Icon( + painter = painterResource(CoreR.drawable.ic_save), + contentDescription = stringResource(R.string.cw_copy) + ) + } // Jump to the newest text. The record below deliberately does not follow the decode, // so this is how you get back to the bottom after reading earlier traffic. IconButton(onClick = { scope.launch { transcriptScroll.animateScrollTo(transcriptScroll.maxValue) } }) { @@ -239,6 +277,16 @@ fun CwDecodeScreen() { .padding(horizontal = 12.dp, vertical = 6.dp) ) + // The pane below was an unlabelled grey box showing a bare ellipsis, which reads as a + // disabled text field rather than a transcript. Naming it is what makes the split + // between the live line above and the record below legible at all. + Text( + text = stringResource(R.string.cw_record_label), + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(start = 12.dp, top = 4.dp) + ) + Box( modifier = Modifier .weight(1f) @@ -247,7 +295,13 @@ fun CwDecodeScreen() { .clip(RoundedCornerShape(8.dp)) .background(MaterialTheme.colorScheme.surfaceVariant.copy(alpha = 0.4f)) ) { - val transcript = (historyText + decodedText).ifEmpty { "…" } + // The record shows settled text only. decodedText is the live 20-second window, + // which the model rewrites from scratch every 1.5 seconds as more context arrives - + // right for the line above, where the newest guess is what you want, but it meant the + // bottom of the record kept changing and sometimes got shorter. That reads as the + // decoder deleting what it just wrote, which for something whose whole purpose is to + // be read back is the one thing it must not do. + val transcript = historyText.ifEmpty { emptyRecordMessage } // This pane does not follow the decode. The single line above it is where new // characters appear; this is the record, and a record that scrolls itself is worse // than paper - you cannot read back over what just arrived because it keeps moving. diff --git a/feature/cw/src/main/res/values-zh/app_values.xml b/feature/cw/src/main/res/values-zh/app_values.xml index a0e942ff..8ec5d3f2 100644 --- a/feature/cw/src/main/res/values-zh/app_values.xml +++ b/feature/cw/src/main/res/values-zh/app_values.xml @@ -14,4 +14,8 @@ 音调 %1$d 赫兹，已搬入解码范围 音调 %1$d 赫兹，在解码范围外 音调 %1$d 赫兹 + 复制记录 + 记录已复制 + 记录 + 解码文字会汇集到这里。离开本页前请复制：记录不会被存下来。 diff --git a/feature/cw/src/main/res/values/app_values.xml b/feature/cw/src/main/res/values/app_values.xml index 2a3767da..b959ada6 100644 --- a/feature/cw/src/main/res/values/app_values.xml +++ b/feature/cw/src/main/res/values/app_values.xml @@ -14,4 +14,8 @@ tone at %1$d hertz, moved into the decoder range tone at %1$d hertz, outside the decoder range tone at %1$d hertz + Copy the record + Record copied + Record + Decoded text collects here. Copy it before leaving this screen: the record is not stored.