Skip to content

Commit 1c51cc5

Browse files
skerkerclaudejensenpat
authored
fix(asr): guard model memory and clean up failed loads (#5773). Principle XI.
Guard automatic Copy Assist model promotion with free-memory and total-capacity requirements. Fail Whisper weight allocation cleanly and release model/VAD resources before recovery instead of dereferencing unbacked tensors or leaking prior allocations. Existing local ASR GPU-probe and linkage smoke tests passed 2/2. Windows prebuilt speech-library replacement remains outside this PR. Squashed-from: #5773 Co-authored-by: Jeff Skerker <7691216+skerker@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: jensenpat <patjensen@gmail.com>
1 parent e33adff commit 1c51cc5

6 files changed

Lines changed: 252 additions & 18 deletions

File tree

src/asr/WhisperAsrBackend.h

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,43 @@ inline AsrTierResolution asrReconcileDefaultTier(const QString& currentTier,
149149
return {currentTier, false};
150150
}
151151

152+
// Whether a model tier of `tierSizeBytes` (the weights file) can be expected to
153+
// load on a device reporting this much memory. Gates only the AUTOMATIC raise
154+
// to the GPU-default tier: "a GPU exists" says nothing about room, and a 1.6 GB
155+
// model auto-selected for a 2 GB card is #4972. An explicit operator choice is
156+
// never refused here — that stays the operator's call.
157+
//
158+
// The headroom is what whisper allocates beyond the weights (KV caches and
159+
// compute buffers). MEASURED (#4972 bench, RTX 5060 Laptop, ggml-vulkan,
160+
// 2026-09-16): large-v3-turbo occupies 1818 MiB against a 1549 MiB file
161+
// (+268 MiB), base 293 MiB against 141 MiB (+152 MiB); whisper's own load log
162+
// sums to the same figure. 300 MiB covers the larger of the two.
163+
//
164+
// The free figure is not always free memory: ggml-vulkan reports free == total
165+
// for a device without VK_EXT_memory_budget (ggml_backend_vk_get_device_memory),
166+
// so the free check alone can be handed the whole heap. The total must therefore
167+
// clear the same need plus a reserve for the desktop and AetherSDR's own
168+
// rendering on that card, which makes the answer independent of the reporting
169+
// mode. The reserve is a chosen margin, not a measurement; for scale, MEASURED
170+
// total minus free at the startup probe was 367 MiB (#5730 reporter log, GTX
171+
// 1050, 1809 of 2176 MB free) and 791 MiB (#4972 bench, 7360 of 8151 MB free).
172+
//
173+
// Both figures 0 means the device could not be asked (AsrGpuDevice) — unknown
174+
// is not "too small", so it keeps the previous behaviour. Integrated GPUs
175+
// report shared system memory and pass on their own numbers. Header-inline and
176+
// whisper-free, like asrReconcileDefaultTier above.
177+
inline constexpr quint64 kAsrTierVramHeadroomBytes = 300ull * 1024ull * 1024ull;
178+
inline constexpr quint64 kAsrTierVramDesktopReserveBytes = 512ull * 1024ull * 1024ull;
179+
180+
inline bool asrTierFitsVram(quint64 vramFreeBytes, quint64 vramTotalBytes, qint64 tierSizeBytes)
181+
{
182+
if (vramTotalBytes == 0 || tierSizeBytes <= 0) {
183+
return true;
184+
}
185+
const quint64 need = static_cast<quint64>(tierSizeBytes) + kAsrTierVramHeadroomBytes;
186+
return vramFreeBytes >= need && vramTotalBytes >= need + kAsrTierVramDesktopReserveBytes;
187+
}
188+
152189
// A selectable transcription language: `code` is the ISO code passed to the
153190
// backend (e.g. "en", "es"); `name` is the English display name ("English").
154191
struct AsrLanguage {

src/gui/CopyAssistController.cpp

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -650,17 +650,46 @@ void CopyAssistController::applyGpuDevices(std::vector<AsrGpuDevice> gpus)
650650
// walk-back, the load-time fallback arm kept large-v3-turbo running
651651
// on CPU: the "backlog climbing, no text" symptom this PR opens with
652652
// (#4767 review). An explicitly chosen tier is never changed.
653-
const bool resolvedGpuUsable = resolvedDevice >= 0 && [&] {
654-
for (const AsrGpuDevice& g : m_gpuDevices) {
655-
if (g.index == resolvedDevice) {
656-
return g.usable;
657-
}
653+
const AsrGpuDevice* resolvedGpu = nullptr;
654+
for (const AsrGpuDevice& g : m_gpuDevices) {
655+
if (g.index == resolvedDevice) {
656+
resolvedGpu = &g;
657+
break;
658+
}
659+
}
660+
const bool resolvedGpuUsable =
661+
resolvedDevice >= 0 && resolvedGpu != nullptr && resolvedGpu->usable;
662+
// The raise additionally needs ROOM: a usable GPU that cannot hold the
663+
// GPU-default tier must not be handed it (#4972 — 1.6 GB auto-selected
664+
// for a 2 GB card). Only the raise is gated. A tier already running is
665+
// not walked back on this figure: once a model is loaded, the device's
666+
// free memory is low because of that very model.
667+
const QString gpuDefaultTier = QStringLiteral("large-v3-turbo");
668+
bool wantGpuDefault = m_useGpuDefaultIfAvailable;
669+
if (wantGpuDefault && resolvedGpuUsable) {
670+
const AsrModelTier* gpuTier = AsrModelCatalog::tierById(gpuDefaultTier);
671+
const qint64 gpuTierBytes = gpuTier != nullptr ? gpuTier->sizeBytes : 0;
672+
if (!asrTierFitsVram(resolvedGpu->vramFreeBytes, resolvedGpu->vramTotalBytes,
673+
gpuTierBytes)) {
674+
wantGpuDefault = false;
675+
// Warning, not info: lcGui is declared QtWarningMsg, so an info
676+
// line would be absent from every default support log — and
677+
// this is the line that explains why the GPU tier was withheld.
678+
const quint64 needMb =
679+
(static_cast<quint64>(gpuTierBytes) + kAsrTierVramHeadroomBytes)
680+
/ (1024 * 1024);
681+
qCWarning(lcGui).nospace()
682+
<< "ASR: keeping the default model tier - " << resolvedGpu->name << " has "
683+
<< (resolvedGpu->vramFreeBytes / (1024 * 1024)) << " of "
684+
<< (resolvedGpu->vramTotalBytes / (1024 * 1024)) << " MB free, "
685+
<< gpuDefaultTier << " needs about " << needMb << " MB free on a device of "
686+
<< (needMb + kAsrTierVramDesktopReserveBytes / (1024 * 1024))
687+
<< " MB or more";
658688
}
659-
return false;
660-
}();
689+
}
661690
const AsrTierResolution tier = asrReconcileDefaultTier(
662-
m_tierId, m_useGpuDefaultIfAvailable, m_gpuDefaultTierActive,
663-
resolvedGpuUsable, QStringLiteral("large-v3-turbo"),
691+
m_tierId, wantGpuDefault, m_gpuDefaultTierActive,
692+
resolvedGpuUsable, gpuDefaultTier,
664693
AsrModelCatalog::defaultTierId());
665694
m_gpuDefaultTierActive = tier.gpuDefaultActive;
666695
if (tier.tierId != m_tierId) {

tests/asr_gpu_probe_test.cpp

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,82 @@ int main()
202202
"explicit choices untouched)\n");
203203
}
204204

205+
// VRAM gate on the automatic raise to the GPU-default tier (#4972): "a GPU
206+
// exists" must not be enough to select a 1.6 GB model.
207+
{
208+
constexpr quint64 kMiB = 1024ull * 1024ull;
209+
// Copy of the "large-v3-turbo" sizeBytes in AsrModelCatalog.cpp (this
210+
// target does not link the catalog) — keep the two in step.
211+
constexpr qint64 kTurboBytes = 1624555275;
212+
213+
// MEASURED (#4972 bench, RTX 5060 Laptop 8151 MiB under VRAM ballast,
214+
// 2026-09-16): the app logged "VRAM free 1126 of 8151 MB" and enabling
215+
// the auto-raised tier took SIGSEGV in the whisper model load.
216+
if (asrTierFitsVram(1126 * kMiB, 8151 * kMiB, kTurboBytes)) {
217+
std::fprintf(stderr, "[FAIL] 1126 MB free was judged enough for the "
218+
"1.6 GB tier (#4972)\n");
219+
return 1;
220+
}
221+
// MEASURED (#5730 reporter log, GTX 1050, 2026-09-15): "VRAM free 1809
222+
// of 2176 MB". The tier occupies 1818 MiB once loaded (same bench), so
223+
// a 2 GB-class card is never auto-raised; choosing it stays possible.
224+
if (asrTierFitsVram(1809 * kMiB, 2176 * kMiB, kTurboBytes)) {
225+
std::fprintf(stderr, "[FAIL] a 2 GB-class card (1809 MB free) was "
226+
"judged to fit the 1.6 GB tier (#4972)\n");
227+
return 1;
228+
}
229+
// MEASURED (same bench, no ballast): "VRAM free 7360 of 8151 MB".
230+
if (!asrTierFitsVram(7360 * kMiB, 8151 * kMiB, kTurboBytes)) {
231+
std::fprintf(stderr, "[FAIL] an 8 GB card with 7360 MB free was "
232+
"refused the GPU-default tier (#4972)\n");
233+
return 1;
234+
}
235+
// CONSTRUCTED: the boundary itself — weights + headroom fits, one byte
236+
// less does not.
237+
const quint64 need = static_cast<quint64>(kTurboBytes) + kAsrTierVramHeadroomBytes;
238+
if (!asrTierFitsVram(need, 4096 * kMiB, kTurboBytes)
239+
|| asrTierFitsVram(need - 1, 4096 * kMiB, kTurboBytes)) {
240+
std::fprintf(stderr, "[FAIL] VRAM gate boundary is not weights + "
241+
"headroom (#4972)\n");
242+
return 1;
243+
}
244+
// CONSTRUCTED input on a MEASURED total (2176 MB, #5730 reporter log):
245+
// ggml-vulkan reports free == total for a device without
246+
// VK_EXT_memory_budget (ggml_backend_vk_get_device_memory), so the same
247+
// card can present as 2176 of 2176 MB free. No capture of a driver in
248+
// that mode exists; the row pins that the answer does not depend on it.
249+
if (asrTierFitsVram(2176 * kMiB, 2176 * kMiB, kTurboBytes)) {
250+
std::fprintf(stderr, "[FAIL] a 2 GB-class card reporting free == total "
251+
"was judged to fit the 1.6 GB tier (#4972)\n");
252+
return 1;
253+
}
254+
// CONSTRUCTED: the total boundary — need + desktop reserve fits, one
255+
// byte less does not, with free memory ample in both.
256+
const quint64 needTotal = need + kAsrTierVramDesktopReserveBytes;
257+
if (!asrTierFitsVram(needTotal, needTotal, kTurboBytes)
258+
|| asrTierFitsVram(needTotal - 1, needTotal - 1, kTurboBytes)) {
259+
std::fprintf(stderr, "[FAIL] VRAM gate total boundary is not weights + "
260+
"headroom + desktop reserve (#4972)\n");
261+
return 1;
262+
}
263+
// CONSTRUCTED: memory unknown (AsrGpuDevice leaves both 0 when the
264+
// device could not be asked) is not "too small" — previous behaviour.
265+
if (!asrTierFitsVram(0, 0, kTurboBytes)) {
266+
std::fprintf(stderr, "[FAIL] unknown VRAM was treated as too small "
267+
"(#4972)\n");
268+
return 1;
269+
}
270+
// CONSTRUCTED: an unknown tier size cannot refuse a device.
271+
if (!asrTierFitsVram(64 * kMiB, 2048 * kMiB, 0)) {
272+
std::fprintf(stderr, "[FAIL] an unknown tier size refused a device "
273+
"(#4972)\n");
274+
return 1;
275+
}
276+
std::printf("[ok] #4972 VRAM gate on the GPU-default tier "
277+
"(weights + headroom, total clears a desktop reserve, "
278+
"unknown memory passes)\n");
279+
}
280+
205281
QElapsedTimer timer;
206282

207283
#ifdef Q_OS_MACOS

third_party/whisper.cpp/AETHERSDR-PATCHES.md

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@ The source snapshot is pinned to ggml-org/whisper.cpp commit
55
The tree is otherwise an exact — if trimmed — upstream snapshot; see
66
[`AETHER_VENDORING.md`](AETHER_VENDORING.md) for what was removed.
77

8-
AetherSDR carries three local changes: two in the ggml Metal backend, from the
9-
same fix (#4535, PR #4553), and one in the ggml CPU backend, a MinGW build fix
10-
(#4406) landed separately.
8+
AetherSDR carries four local changes: two in the ggml Metal backend, from the
9+
same fix (#4535, PR #4553), one in the ggml CPU backend, a MinGW build fix
10+
(#4406) landed separately, and one in `src/whisper.cpp`, a model-load crash fix
11+
(#4972).
1112

1213
1. `ggml/src/ggml-metal/CMakeLists.txt`: adds `GGML_METAL_EMBED_LIBRARY_COMPILED`.
1314
Upstream's `GGML_METAL_EMBED_LIBRARY` embeds the merged kernel **source** and
@@ -72,6 +73,52 @@ completion in 75 minutes on a Radeon Pro 560X.
7273
`ggml-org/ggml` and `ggml-org/whisper.cpp`, with no existing issue or PR —
7374
so a `COMMIT` bump alone would not have picked up a fix.
7475

76+
4. `src/whisper.cpp`: fails the model load when the weight buffer cannot be
77+
allocated, at both sites that share the pattern — `whisper_model_load()` and
78+
the VAD loader in `whisper_vad_init_with_params()` (the latter is not called
79+
by AetherSDR; it is patched so the two copies cannot drift).
80+
81+
Upstream writes
82+
83+
```cpp
84+
ggml_backend_buffer_t buf = ggml_backend_alloc_ctx_tensors_from_buft(ctx, buft);
85+
if (buf) { model.buffers.emplace_back(buf); ... }
86+
```
87+
88+
with no `else`, then uploads the weights with `ggml_backend_tensor_set()`.
89+
When the allocation fails — a GPU short of memory is the ordinary case —
90+
every tensor in that context still has no buffer, and the first upload
91+
dereferences it. Measured on Linux with ggml-vulkan (RTX 5060 Laptop, ~1.1 GB
92+
of device memory free, `ggml-large-v3-turbo.bin`): ggml logs
93+
`alloc_tensor_range: failed to allocate Vulkan1 buffer of size 551900160`,
94+
then the process takes SIGSEGV at address 0x10 in
95+
`ggml_vk_buffer_write_2d()` on the ASR worker thread. A signal is not an
96+
exception, so the guards in `WhisperAsrBackend` cannot intercept it (#4972).
97+
98+
The patch adds `whisper_ctx_has_unallocated_tensor()` and an `else if` on it
99+
at both sites that logs and returns failure. A bare `else` would be wrong:
100+
the allocator also returns NULL when every tensor in the context was already
101+
allocated, so the helper repeats the allocator's own "needs allocation" test
102+
(`data == NULL && view_src == NULL`, non-zero size) rather than treating NULL
103+
as the error. The patch also changes the failed-load cleanup in
104+
`whisper_init_with_params_no_state()` from `delete ctx` to
105+
`whisper_free(ctx)`: the model stores raw ggml context and buffer pointers,
106+
so deleting the C++ context alone leaks them, including any weight buffers
107+
allocated before a later group failed. The VAD allocation-failure path
108+
similarly calls `whisper_vad_free(vctx)` before returning NULL. The returned
109+
NULL lets `WhisperAsrBackend::load()` latch the device and retry on CPU
110+
after the failed attempt's resources have been released.
111+
112+
Checked upstream at the time of this patch (2026-09-16): both sites read the
113+
same on `ggml-org/whisper.cpp` `master`, so a `COMMIT` bump alone would not
114+
pick up a fix.
115+
116+
**The Windows release does not compile this file.** It links the prebuilt
117+
`whisper.lib` from the `whisper-gpu-<ver>` release asset
118+
(`ASR_USE_PREBUILT_WHISPER_GPU`, top-level `CMakeLists.txt`), so this patch
119+
reaches Windows release binaries only when that pack is rebuilt from a tree
120+
that contains it and its pinned SHA-256 is updated.
121+
75122
## Refreshing
76123
77124
When refreshing whisper.cpp, first check whether upstream has adopted an
@@ -95,8 +142,18 @@ configure + build (`cmake --build` from a MinGW-w64 Ninja toolchain); a
95142
regression here only shows up as a MinGW compile failure, not a test failure,
96143
since MSVC and non-Windows builds never exercise this branch.
97144

145+
For the `src/whisper.cpp` allocation check, look at both
146+
`ggml_backend_alloc_ctx_tensors_from_buft()` call sites: if upstream now fails
147+
the load when the returned buffer is NULL, drop the local patch. Otherwise
148+
reapply it at both, including the full context cleanup on the failure paths.
149+
There is no unit seam for it — forcing the failure needs a
150+
GPU backend that is short of memory — so confirm on a GPU host by occupying
151+
device memory until the large tier cannot fit and enabling Copy Assist: the log
152+
must show `GPU model load failed ... retrying on CPU` and the process must
153+
survive.
154+
98155
The authoritative diff for any of these files is its git history
99156
(`git log -p -- third_party/whisper.cpp/ggml/src/ggml-metal/<file>` or
100-
`.../ggml-cpu/ggml-cpu.c`). No checked-in `.patch` copy is kept for any of
157+
`.../ggml-cpu/ggml-cpu.c`, or `.../src/whisper.cpp`). No checked-in `.patch` copy is kept for any of
101158
them: it would need hand-syncing on every edit, and its context would not
102159
apply cleanly across an upstream bump anyway.

third_party/whisper.cpp/AETHER_VENDORING.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ upstream `LICENSE` covers the bundled `ggml/` tree too — upstream ships one MI
88
file for both.
99

1010
Keep this a pristine mirror: do **not** modify vendored sources in place unless
11-
the change genuinely cannot live outside the tree. Three files currently do;
11+
the change genuinely cannot live outside the tree. Four files currently do;
1212
every one is recorded in [`AETHERSDR-PATCHES.md`](AETHERSDR-PATCHES.md), and
1313
anything not listed there is a drift bug.
1414

@@ -52,7 +52,7 @@ fallback). `GGML_NATIVE=OFF` is forced for portable/Pi/CI binaries.
5252

5353
## Local patches (deviations from pristine upstream)
5454

55-
Three vendored files carry AetherSDR-local changes; the pristine-mirror rule
55+
Four vendored files carry AetherSDR-local changes; the pristine-mirror rule
5656
above holds for everything else. Each is described — with its rationale and
5757
the refresh checklist — in [`AETHERSDR-PATCHES.md`](AETHERSDR-PATCHES.md),
5858
following the same convention as `third_party/wdsp` and
@@ -67,6 +67,10 @@ following the same convention as `third_party/wdsp` and
6767
throttle guard to a feature-detect so it compiles under MinGW-w64, which
6868
doesn't declare `THREAD_POWER_THROTTLING_STATE`. Fixes the same MinGW-only
6969
compile break originally raised in #4406.
70+
- `src/whisper.cpp` — fails the model load when the weight buffer cannot be
71+
allocated instead of uploading into unbacked tensors (SIGSEGV on a GPU short
72+
of memory). Fixes #4972. Not in the prebuilt Windows `whisper-gpu` pack until
73+
that pack is rebuilt.
7074

7175
The two Metal changes are kept as thin as possible: the *policy* around them —
7276
required toolchain, missing-toolchain behaviour, deployment target, shader
@@ -85,5 +89,5 @@ To add a different GPU backend (CUDA, Metal, …), **re-copy that backend's
8589
directory** from upstream at the pinned commit and turn its `GGML_<X>` option ON
8690
(with the matching toolchain + CI runner). To refresh: clone upstream at
8791
`COMMIT`, re-run the same trim (keeping `ggml-cpu`, `ggml-blas`, `ggml-vulkan`),
88-
and diff — then re-apply the three local patches (see **Local patches** above);
89-
a clean diff plus exactly those three files is the expected end state.
92+
and diff — then re-apply the four local patches (see **Local patches** above);
93+
a clean diff plus exactly those four files is the expected end state.

0 commit comments

Comments
 (0)