Files
win-dictate/old_text_files/FINDINGS-FIXES-TESTS.md

20 KiB
Raw Permalink Blame History

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 56 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 "1015× real-time" numbers were measured on a 24-thread box.
  • A 2-core i5-7th-gen does tiny.en at roughly 24× real-time.

So each 6 s window took ~1.52.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_SYSMENUno 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 architecturestart_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_workerif (!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):

    // transcriber.h  (public)
    std::string transcribe_sync(std::vector<float> audio);   // headless / tests
    // transcriber.h  (private)
    std::string run_inference(std::vector<float>& audio);
    
    // 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).

  • ResizableWS_OVERLAPPEDWINDOW (has WS_THICKFRAME), WM_SIZE → LayoutControls, WM_GETMINMAXINFO min size (F3).

  • Startup race guardg_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:

    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 buttonID_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:

// 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).

#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:

# --- 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:

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.