v2 rebuild: GDI+ single-surface UI, self-calibrating progress, compact push-to-talk, GGML+Whisper integration

2026-06-11 15:30:19 +12:00
parent 81b3d0073e
commit 1f67c07a77
28 changed files with 11856 additions and 1948 deletions
@@ -0,0 +1,425 @@
+# Win Dictation — Findings, Fixes & Test Guide
+
+A consolidated record of everything diagnosed and changed while turning the slow,
+crashing build into a fast, stable push-to-talk dictation tool. Four parts:
+
+1. **Findings** — what was actually wrong, in the order we discovered it.
+2. **Fixes required** — the concrete code changes, by file.
+3. **Reasoning** — why each fix is correct (the non-obvious calls).
+4. **How to build & run tests** — making the core testable, plus regression tests for every bug here.
+
+Target machine: Dell, Intel Core i5 7th-gen (2 cores / 4 threads), 16 GB RAM, CPU-only.
+Stack: native C++ / Win32 + SDL2 + whisper.cpp (`examples/win-dictation`).
+
+---
+
+## Part 1 — Findings
+
+### F1. The original app was slow because of its *architecture*, not its model
+The first build used whisper.cpp's live-streaming pattern: it re-ran `whisper_full` on a
+rolling 5–6 second window every ~0.4 s (`length_ms` / `step_ms` in `worker_loop`). To keep
+up, the CPU must transcribe ~6 s of audio in well under 1 s — i.e. **>6× real-time**.
+
+- The README's "10–15× real-time" numbers were measured on a **24-thread** box.
+- A 2-core i5-7th-gen does tiny.en at roughly **2–4× real-time**.
+
+So each 6 s window took ~1.5–2.5 s while a new one was demanded every 0.4 s → the buffer
+backed up without bound, audio was re-transcribed in overlapping chunks (duplicated/garbled
+text), the CPU pegged, and 4 Whisper threads on 2 physical cores starved the UI. tiny.en was
+already the right model — **the streaming design was the bottleneck.**
+
+**Conclusion:** for dictation (not live captioning), switch to **batch / push-to-talk**:
+buffer audio cheaply while recording, run `whisper_full` exactly **once** on Stop.
+
+### F2. Build 1 crashed on Stop — null Whisper context
+After the batch rewrite, the app recorded fine (VU moved) but crashed the instant you hit
+Stop. Two defects combined:
+
+1. The preload thread set `g_modelLoaded = true` **even when `preload()` returned false**
+   (model file not found), so the UI showed "Ready" and let you record with no model.
+2. `transcribe_worker()` called `whisper_reset_timings(m_ctx)` **before** the `if (m_ctx)`
+   check. With `m_ctx == nullptr` that's an access violation. Recording never touches Whisper
+   — which is exactly why capture/VU worked and only Stop crashed.
+
+The empty model dropdown in the first screenshot was the tell: no model files were found at
+the new exe-relative path, so `preload()` failed silently.
+
+### F3. Build 1 — window wouldn't resize, and looked boxy
+- The style was `WS_POPUP | WS_CAPTION | WS_SYSMENU` — **no sizing border** — and there was
+  **no `WM_SIZE` handler**, so controls never reflowed. Frozen at ~360×200, Copy/Paste cut off.
+- The gray 3-D system buttons + chunky title bar were the "boxy Windows" look the brief
+  wanted to avoid.
+
+### F4. Build 2 — startup data race (found and fixed by the user)
+`RefreshModelList()` calls `CB_SETCURSEL`, which fires `CBN_SELCHANGE`. That spawned a
+`reload()` background thread that raced the initial `preload()` thread — both writing
+`m_cfg` (a `std::string`) and `m_ctx` at once. The corrupted context pointer made
+`whisper_full` crash ~15 s into processing. Fixed with a `g_initializing` guard (skip the
+combo handler during startup) and an `m_cfg_mtx` mutex around `m_cfg`/`m_ctx` writes.
+
+### F5. Build 3 — the "crash after transcribing" was **not a crash**
+Reported: "crashes after transcribing; relaunching the exe shows the last transcription, and
+a second run replaced the first instead of appending."
+
+That behavior is **impossible for a crashed process** — a dead process can't remember the
+last transcription (nothing is persisted to disk/registry), and a *fresh* launch couldn't
+show it. The only explanation: **the original process never died.**
+
+What actually happened:
+1. On a successful result, `WM_APP_RESULT` ran `ShowWindow(hWnd, SW_HIDE)` in the auto-paste
+   branch → the window **vanished** (looked like a crash).
+2. Double-clicking the `.exe` again hit the **single-instance guard**, which
+   `PostMessage(WM_APP_SHOW)` to the existing **hidden** window → it re-appeared, still holding
+   the last transcription.
+3. `SetDlgItemTextW` **replaced** the edit text, so run #2 overwrote run #1.
+
+Also: clicking the **Record button** (vs. the global hotkey) set `g_prevForeground` to the
+dictation window *itself*, so it hid and "pasted" into its own read-only box — nothing landed
+in a useful app.
+
+10-second confirmation: when it "crashes," the tray icon is still present and
+`win-dictation.exe` is still in Task Manager.
+
+### F6. The test harness is stale and never built
+`src/test-audio.cpp` calls `m_transcriber.init(...)` and `is_using_gpu()` — methods that the
+batch rewrite **removed** (`transcriber.h` now exposes `preload`/`reload`, no `init`). So it
+won't compile against the current code. And the **root** `CMakeLists.txt` (the one `build.ps1`
+actually uses) defines only the `win-dictation` target — the `test-audio` target lives only in
+the unused `src/CMakeLists.txt`. So there are currently **no working tests**. Part 4 fixes this.
+
+---
+
+## Part 2 — Fixes required
+
+Status legend: ✅ done in current code · ⬜ still to apply.
+
+### `transcriber.cpp` / `transcriber.h`
+
+- ✅ **Batch architecture** — `start_recording()` buffers audio cheaply; `stop_and_transcribe()`
+  runs `whisper_full` once on a worker thread; result delivered via `PostMessage` (F1).
+- ✅ **Null-context guard** in `transcribe_worker` — `if (!m_ctx) { m_busy=false; if(m_on_result) m_on_result(""); return; }`,
+  and the `whisper_reset_timings` call removed (F2).
+- ✅ **`reload()`** frees + re-inits under a mutex; **`m_cfg_mtx`** protects `m_cfg`/`m_ctx`;
+  **`threads()`** getter (F4 + thread-count display).
+- ⬜ **Refactor for testability** — extract the inference core so it can be called
+  synchronously from a test (see Part 4):
+
+  ```cpp
+  // transcriber.h  (public)
+  std::string transcribe_sync(std::vector<float> audio);   // headless / tests
+  // transcriber.h  (private)
+  std::string run_inference(std::vector<float>& audio);
+  ```
+
+  ```cpp
+  // transcriber.cpp
+  std::string Transcriber::run_inference(std::vector<float>& audio) {
+      if (!m_ctx) return "";
+      if (m_cfg.trim_silence) trim_silence(audio);
+      whisper_full_params wp = whisper_full_default_params(WHISPER_SAMPLING_GREEDY);
+      wp.print_progress = wp.print_realtime = wp.print_timestamps = false;
+      wp.no_timestamps = true; wp.translate = false;
+      wp.language = m_cfg.language.c_str();
+      wp.n_threads = m_cfg.n_threads;
+      wp.no_context = true; wp.suppress_blank = true; wp.suppress_nst = true;
+      wp.temperature = 0.0f;
+      std::string out;
+      if (whisper_full(m_ctx, wp, audio.data(), (int)audio.size()) == 0) {
+          int n = whisper_full_n_segments(m_ctx);
+          for (int i = 0; i < n; ++i) { const char* t = whisper_full_get_segment_text(m_ctx, i); if (t) out += t; }
+          out = clean_text(out);
+      }
+      return out;
+  }
+  void Transcriber::transcribe_worker(std::vector<float> audio) {
+      std::string out = run_inference(audio);
+      m_busy = false;
+      if (m_on_result) m_on_result(out);
+  }
+  std::string Transcriber::transcribe_sync(std::vector<float> audio) {
+      return run_inference(audio);   // assumes preload() already succeeded
+  }
+  ```
+
+### `main.cpp`
+
+- ✅ **Honest load state** — separate `g_modelOk` atomic; record is gated and shows
+  "Model not found: …\models\ggml-tiny.en.bin" instead of crashing (F2).
+- ✅ **Resizable** — `WS_OVERLAPPEDWINDOW` (has `WS_THICKFRAME`), `WM_SIZE → LayoutControls`,
+  `WM_GETMINMAXINFO` min size (F3).
+- ✅ **Startup race guard** — `g_initializing` skips the model-combo handler during startup (F4).
+- ⬜ **`WM_APP_RESULT` rewrite** — the F5 fix: **don't hide**, **append** (not replace),
+  clipboard = latest utterance, paste only into a *different* valid window:
+
+  ```cpp
+  case WM_APP_RESULT:
+  {
+      std::string* res = (std::string*)wParam;
+      if (res && !res->empty()) {
+          HWND hEdit = GetDlgItem(hWnd, ID_EDIT_TEXT);
+          int len = GetWindowTextLengthW(hEdit);
+          std::wstring cur;
+          if (len > 0) { cur.resize(len + 1); int g = GetWindowTextW(hEdit, &cur[0], len + 1); cur.resize(g); }
+          std::wstring combined = cur;
+          if (!combined.empty()) combined += L" ";
+          combined += to_w(*res);
+          SetWindowTextW(hEdit, combined.c_str());
+          SendMessageW(hEdit, EM_SETSEL, (WPARAM)combined.size(), (LPARAM)combined.size());
+          SendMessageW(hEdit, EM_SCROLLCARET, 0, 0);
+          UpdatePlaceholder(hWnd);
+
+          SetClipboardTextUtf8(hWnd, *res);   // clipboard = just the latest utterance
+
+          bool pasted = false;
+          if (g_autoPaste && g_prevForeground &&
+              g_prevForeground != hWnd && IsWindow(g_prevForeground)) {
+              PasteIntoWindow(g_prevForeground);
+              pasted = true;
+          }
+          SetStatus(hWnd, pasted ? L"Pasted ✓" : L"Copied ✓");
+      } else {
+          SetStatus(hWnd, L"No speech detected");
+      }
+      delete res;
+  }
+  break;
+  ```
+
+- ⬜ **Clear button** — `ID_BTN_CLEAR` is handled in `WM_COMMAND` but no button is created in
+  the modern layout, so the (now appending) transcript can't be cleared. Create it owner-drawn,
+  place it in `LayoutControls`, and call `UpdatePlaceholder(hWnd)` after clearing.
+- ⬜ **Optional `g_autoHide`** (default `false`) — only if you want the window to tuck away after
+  a successful paste into another app: `if (pasted && g_autoHide) ShowWindow(hWnd, SW_HIDE);`
+
+### Modern UI (separate doc)
+The flat dark restyle (GDI+ owner-drawn buttons, rounded panel, custom VU, dark caption +
+rounded corners) is fully specified in **`MODERN-UI-AND-FIXES.md`**, Part 2.
+
+---
+
+## Part 3 — Reasoning
+
+**Why batch beats streaming here (F1).** Streaming only wins if you need words *as you speak*
+(live captioning). Dictation doesn't: you speak a sentence, then want the text. Batch removes
+the real-time deadline entirely — Whisper processes each second of audio exactly once, at its
+own pace, after you stop. That means: near-zero CPU while speaking (just buffering), predictable
+"a few seconds after Stop" latency that **doesn't grow with clip length**, no overlap
+re-processing, and better accuracy (full context, no chunk-boundary word splits). On 2 cores
+this is the difference between unusable and snappy.
+
+**Why threads = physical cores (2), not logical (4).** Whisper's matmuls are memory-bandwidth
+bound. Two extra hyperthreads share the same execution ports and cache, so they add little
+throughput while stealing cycles from the UI/audio threads (janky window, laggy VU). Two
+threads leaves headroom for a responsive UI.
+
+**Why the Stop crash was a null deref (F2), and why the guard is the right fix.** Recording is
+pure SDL + a `std::vector` append — it never calls into Whisper, which is why only Stop crashed.
+`whisper_reset_timings(nullptr)` dereferences the context. The guard makes the worker a no-op
+when no model is loaded; the separate `g_modelOk` flag makes that state *visible* (a MessageBox
+with the path) instead of letting you walk into it. Defensive + diagnostic.
+
+**Why the "crash after transcribing" was actually a hide (F5) — the logic is conclusive.** A
+crashed process loses all memory. There is no on-disk persistence in this app. Therefore a
+re-launched process **cannot** display the previous transcription, and a second run **cannot**
+replace text in "the same box." The observed behavior (prior text reappears; second replaces
+first) is only possible if it's the *same* live process — which means it hid, and the
+single-instance guard re-showed it. This is why "is the process still in Task Manager?" is the
+decisive test, not "did the window disappear?"
+
+**Why clipboard = latest utterance but the box appends.** Two different jobs. The on-screen box
+is your running log (you want history). The clipboard is what you paste into another app — you
+want *just what you last said*, not the whole session. Splitting them gives both.
+
+**Why paste must exclude our own window.** Auto-paste replays Ctrl+V into the foreground window
+captured before the popup. If you triggered recording by clicking the Record button, that
+"previous" window is the dictation app itself — pasting into its read-only box is a no-op and
+(with the old hide) made the app appear to swallow the text. Restricting to
+`g_prevForeground != hWnd && IsWindow(...)` makes the global-hotkey workflow the reliable path
+and the button a safe "copy only."
+
+---
+
+## Part 4 — How to build & run tests
+
+The goal: catch the bugs above automatically, without a human watching a window. UI behavior
+(resize, hide, paste) needs a short **manual** checklist, but the **core** (model load,
+transcription, null-safety, text append) can and should be tested headlessly.
+
+### 4.1 Make the core testable
+Apply the `run_inference` / `transcribe_sync` refactor from Part 2 so a test can feed a buffer
+and get text synchronously. Also extract the append rule as a pure function so it can be tested
+with zero Win32:
+
+```cpp
+// text_util.h  (new, tiny, UI-free)
+#pragma once
+#include <string>
+inline std::wstring append_transcript(const std::wstring& cur, const std::wstring& add) {
+    if (add.empty()) return cur;
+    if (cur.empty()) return add;
+    return cur + L" " + add;
+}
+```
+Use it in `WM_APP_RESULT` (`combined = append_transcript(cur, to_w(*res));`) so the UI and the
+test exercise the *same* rule.
+
+### 4.2 The test program (`tests/test_core.cpp`)
+Replaces the stale `src/test-audio.cpp`. Compiles against the **current** API and tests the real
+code path (`preload` + `transcribe_sync`).
+
+```cpp
+#include "transcriber.h"
+#include "text_util.h"
+#include <cstdio>
+#include <cstdint>
+#include <fstream>
+#include <vector>
+#include <string>
+#include <cctype>
+
+static int g_fail = 0;
+#define CHECK(cond, msg) do { if (!(cond)) { printf("  FAIL: %s\n", msg); ++g_fail; } \
+                              else            printf("  ok:   %s\n", msg); } while(0)
+
+// Minimal 16-bit PCM mono WAV loader -> float [-1,1]. Assumes 16 kHz mono (whisper.cpp samples are).
+static bool load_wav(const std::string& path, std::vector<float>& out) {
+    std::ifstream f(path, std::ios::binary);
+    if (!f) return false;
+    char hdr[44];
+    f.read(hdr, 44);
+    if (std::string(hdr, 4) != "RIFF") return false;
+    std::vector<int16_t> pcm((std::istreambuf_iterator<char>(f)), {});  // crude: rest of file
+    out.clear(); out.reserve(pcm.size());
+    for (int16_t s : pcm) out.push_back(s / 32768.0f);
+    return !out.empty();
+}
+
+static std::string lower(std::string s) { for (char& c : s) c = (char)tolower((unsigned char)c); return s; }
+
+int main(int argc, char** argv) {
+    std::string model = (argc > 1) ? argv[1] : "models/ggml-tiny.en.bin";
+    std::string wav   = (argc > 2) ? argv[2] : "samples/jfk.wav";
+
+    // --- Pure logic: append rule (no model needed) ---
+    CHECK(append_transcript(L"", L"hello") == L"hello",        "append: empty + a");
+    CHECK(append_transcript(L"hello", L"world") == L"hello world", "append: a + b spaced");
+    CHECK(append_transcript(L"hello", L"") == L"hello",        "append: a + empty");
+
+    // --- Regression F2: bad model path must NOT crash, returns "" ---
+    {
+        Transcriber t; WhisperConfig bad; bad.model_path = "models\\does-not-exist.bin";
+        bool ok = t.preload(bad);
+        CHECK(!ok, "bad model path: preload returns false");
+        std::vector<float> a(16000, 0.0f);               // 1 s of silence
+        std::string r = t.transcribe_sync(a);            // must be a safe no-op
+        CHECK(r.empty(), "bad model path: transcribe_sync returns empty, no crash");
+    }
+
+    // --- Happy path: real model + known clip -> non-empty, expected words ---
+    {
+        Transcriber t; WhisperConfig cfg; cfg.model_path = model; cfg.n_threads = 4;
+        bool ok = t.preload(cfg);
+        CHECK(ok, "model loads");
+        if (ok) {
+            std::vector<float> audio;
+            bool loaded = load_wav(wav, audio);
+            CHECK(loaded, "wav loads");
+            if (loaded) {
+                std::string text = lower(t.transcribe_sync(audio));
+                CHECK(!text.empty(), "transcription is non-empty");
+                // jfk.wav: "...ask not what your country can do for you..."
+                CHECK(text.find("country") != std::string::npos, "transcription contains 'country'");
+            }
+        }
+    }
+
+    // --- Edge: sub-300ms / empty audio -> "" , no crash ---
+    {
+        Transcriber t; WhisperConfig cfg; cfg.model_path = model;
+        if (t.preload(cfg)) {
+            std::vector<float> tiny(100, 0.1f);
+            std::string r = t.transcribe_sync(tiny);
+            CHECK(true, "short audio did not crash");   // reaching here = no crash
+        }
+    }
+
+    printf("\n%s (%d failure%s)\n", g_fail ? "TESTS FAILED" : "ALL TESTS PASSED",
+           g_fail, g_fail == 1 ? "" : "s");
+    return g_fail ? 1 : 0;
+}
+```
+
+> The WAV loader above is deliberately minimal (good enough for whisper.cpp's 16 kHz mono
+> `samples/*.wav`). If you test arbitrary WAVs, parse the `fmt `/`data` chunks properly.
+
+### 4.3 Build the tests (root `CMakeLists.txt`)
+The active build file builds only `win-dictation`. Add a test target next to it:
+
+```cmake
+# --- tests ---
+add_executable(test-core
+    tests/test_core.cpp
+    src/transcriber.cpp
+    src/transcriber.h
+)
+target_link_libraries(test-core PRIVATE whisper ${SDL2_LIBRARIES})
+target_include_directories(test-core PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}/src
+    ${CMAKE_CURRENT_SOURCE_DIR}/common
+)
+target_compile_definitions(test-core PRIVATE UNICODE _UNICODE)
+```
+(`transcriber.cpp` references SDL symbols even though the test never opens a device, so the test
+still links `${SDL2_LIBRARIES}`.)
+
+Build and run:
+```powershell
+cmake --build build --config Release --target test-core
+# run from a dir where SDL2.dll + models/ + samples/ are reachable:
+cd build\bin\Release
+copy ..\..\..\deps\SDL2-2.28.5\lib\x64\SDL2.dll .   # if not already beside the exe
+.\test-core.exe models\ggml-tiny.en.bin ..\..\..\samples\jfk.wav
+```
+Exit code `0` = all passed (usable in CI / a pre-commit hook).
+
+### 4.4 What each test guards
+| Test | Guards against |
+|---|---|
+| append rule (3 cases) | F5 regression — replace-instead-of-append |
+| bad model path → false + empty | F2 — null-context crash on Stop |
+| model loads / clip → non-empty, has "country" | core transcription works end to end |
+| short/empty audio → no crash | the `< 0.3 s` guard + null-safety |
+
+### 4.5 Manual UI checklist (can't be unit-tested)
+- [ ] **Not a crash:** after Stop, the window **stays visible**; if it ever vanishes, the tray
+      icon / `win-dictation.exe` in Task Manager confirms whether it's hiding vs. truly gone.
+- [ ] **Append:** two dictations in a row → second is appended after the first, view scrolls down.
+- [ ] **Clipboard:** after a dictation, paste into Notepad → only the *latest* utterance appears.
+- [ ] **Auto-paste:** focus Notepad, press the global hotkey, speak, press it again → text lands
+      in Notepad. Triggering via the Record button instead → "Copied ✓" (no self-paste).
+- [ ] **Resize:** drag edges; transcript grows; min size respected.
+- [ ] **Performance:** a ~10 s clip transcribes in a few seconds and the UI stays responsive;
+      latency does **not** grow with longer recordings.
+
+### 4.6 Performance smoke test (optional)
+`test-core` can also print timing — wrap `transcribe_sync` in `std::chrono::steady_clock` and
+assert `elapsed < audio_seconds * k` for a sanity bound (e.g. `k = 1.0`, i.e. faster than
+real-time on tiny.en). Useful to catch a future regression that quietly reintroduces the
+streaming-style slowdown.
+
+---
+
+## Part 5 — Status & remaining work
+
+**Verified fixed:** F1 (batch), F2 (null-context crash), F3 (resize), F4 (startup race).
+
+**Apply next (⬜ in Part 2):**
+1. `run_inference` / `transcribe_sync` refactor (unblocks tests).
+2. `WM_APP_RESULT` rewrite (the F5 fix — no-hide + append + safe paste). **Highest priority** —
+   it's what's making the app *look* like it crashes.
+3. Add the Clear button (needed now that text appends).
+4. Add `tests/test_core.cpp` + the CMake target; run it.
+
+**Then re-run** the Part 4 manual checklist. Expected result: the window no longer disappears,
+transcriptions append, the clipboard holds the latest utterance, and `test-core.exe` exits `0`.
+