Skip to content

refactor: vendor upstream whisper.cpp as a git subtree under third_party/ + umbrella superbuild (QIP PR 2)#96

Open
ogad-tether wants to merge 13 commits into
feat/engines-movefrom
feat/whisper-subtree
Open

refactor: vendor upstream whisper.cpp as a git subtree under third_party/ + umbrella superbuild (QIP PR 2)#96
ogad-tether wants to merge 13 commits into
feat/engines-movefrom
feat/whisper-subtree

Conversation

@ogad-tether

Copy link
Copy Markdown

What

PR 2 of the repo-reorg QIP (#94) — the vendor conversion. Stacked on #95 (PR 1); retarget to master after it merges.

  1. Remove the upstream tree from root (git rm of every upstream-owned path + upstream workflow files).
  2. git subtree add upstream v1.9.1 under third_party/whisper.cpp (--squash, real subtree metadata so future git subtree pull works).
  3. Re-apply the QVAC whisper delta as one auditable commit + PATCHES.md manifest + UPSTREAM_PIN. The in-tree ggml delta is not re-applied — every ggml patch (Android in-APK loading, Adreno Vulkan workarounds) was verified present on qvac-ext-ggml@speech; ggml divergence is banned in this repo from now on.
  4. Root umbrella superbuild (SPEECH_BUILD_{WHISPER,PARAKEET,TTS,TESTS,EXECUTABLES}), one find_package(ggml) for the whole stack; new root README.
  5. whisper-ci.yml: umbrella build+test lanes (linux/mac) + whisper-cli transcription smoke — replaces upstream's build.yml, which goes inert with this PR (GitHub only runs root .github/workflows) — that also permanently silences the unrelated-failure noise (ggml-ci-mac-vulkan, Ruby bindings) on every PR.

Integration finds (all fixed here, declared in PATCHES.md where in-subtree)

  • Upstream now ships its own C-API parakeet; its parakeet/parakeet-cli targets collide with engines/parakeet → new WHISPER_BUILD_PARAKEET gate (default ON = stock behavior; umbrella sets OFF). Upstreaming candidate.
  • CMAKE_SOURCE_DIR vs PROJECT_SOURCE_DIR in the whisper include export broke subdirectory builds.
  • whisper's cmake_minimum_required(3.5) leaves CMP0077 OLD → umbrella must set whisper options via CACHE FORCE.
  • Both engines had a ctest target literally named test-streaming → renamed engine-scoped (test-parakeet-streaming / test-tts-streaming); no public target changes.

Validation

Local (macOS arm64): fresh umbrella configure + build clean; 69/69 non-GPU tests pass; whisper-cli transcribes jfk.wav with ggml-tiny.en. On this PR, CI runs the full matrix: umbrella lanes (this PR's whisper-ci), engine lanes (linux/mac build+test), Windows build+test, Android/iOS compile smokes.

Coordination

  • The registry's whisper-cpp port builds this repo's root — after this merges it must point at third_party/whisper.cpp (or move to the umbrella port, Ticket 4). Same pin-bump PR as the engine port path updates from refactor: move engines to engines/{parakeet,tts} (QIP PR 1) #95.
  • Downstream mobile e2e via a qvac overlay branch against this tree is the next validation step (QIP task 5).

🤖 Generated with Claude Code

ogad-tether and others added 8 commits July 17, 2026 10:44
… root

Everything upstream-owned at root (src/ include/ ggml/ examples/
bindings/ cmake/ CMakeLists.txt Makefile media/ models/ samples/
grammars/ tests/ ci/ scripts/ .devops/ .pi/ + upstream workflow files)
goes away; the next commit re-vendors upstream v1.9.1 under
third_party/whisper.cpp as a proper git subtree, and a follow-up
re-applies the QVAC whisper delta there (QIP #94, PR 2).

Kept at root: LICENSE, .gitignore, our workflows (.github/workflows/
{parakeet,tts,engines-cross}-ci.yml, check-approvals, security-baseline),
.github/CODEOWNERS, engines/.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
git-subtree-dir: third_party/whisper.cpp
git-subtree-split: f049fff
… manifest

One auditable divergence commit on top of the pristine v1.9.1 subtree:
- src/whisper.cpp + src/whisper-logits-slice.h + include/whisper.h:
  vocab-logits slice and decoder QKV matmul fusion (QVAC-21623)
- CMakeLists.txt / src/CMakeLists.txt / cmake/{git-vars,whisper-config}:
  install-export and tarball-build fixes the vcpkg port relies on

The in-tree ggml delta is intentionally NOT re-applied: every ggml patch
(Android in-APK backend loading, Adreno Vulkan workarounds) was verified
present on qvac-ext-ggml@speech — the ggml-speech port source — which is
the single home for ggml changes from now on. third_party/whisper.cpp/ggml
stays byte-identical to upstream and is never compiled by our builds.

PATCHES.md is the manifest the divergence-guard CI (QIP PR 3) enforces.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gate + UPSTREAM_PIN

Umbrella integration surfaced three issues, all added to PATCHES.md:
- WHISPER_BUILD_PARAKEET option (default ON = stock upstream): upstream
  ships its own C-API parakeet whose `parakeet`/`parakeet-cli` targets
  collide with engines/parakeet; the umbrella turns it off. Upstreaming
  candidate.
- PROJECT_SOURCE_DIR (not CMAKE_SOURCE_DIR) in the whisper include
  export — CMAKE_SOURCE_DIR points at the umbrella root when built as a
  subdirectory, breaking '#include "whisper.h"'.
- UPSTREAM_PIN: machine-readable upstream tag for the divergence-guard
  CI (QIP PR 3).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… build

Both engines defined a ctest target literally named `test-streaming`,
which collides when they configure into one superbuild. Rename to
test-parakeet-streaming / test-tts-streaming (CMake target + ctest name
+ README references only; PROGRESS history docs untouched). No library
or public target names change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- CMakeLists.txt: feature-gated superbuild (SPEECH_BUILD_{WHISPER,
  PARAKEET,TTS,TESTS,EXECUTABLES}); one find_package(ggml) shared by
  every component via their common `if (NOT TARGET ggml)` guards.
  whisper vars set with CACHE FORCE (its cmake_minimum_required(3.5)
  leaves CMP0077 OLD, so option() would clobber plain set()s).
- whisper-ci.yml: umbrella build on linux+mac (all engines, one ggml,
  full non-GPU ctest) + whisper-cli transcription smoke on jfk.wav with
  a cached ggml-tiny.en — replaces the useful slice of upstream's
  build.yml, which went inert when the upstream tree moved under
  third_party/.
- README.md: repo layout, build quick-start, third_party ground rules.

Validated locally (macOS arm64): fresh umbrella configure+build clean,
69/69 non-GPU tests pass, whisper-cli smoke transcribes jfk.wav.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ogad-tether
ogad-tether marked this pull request as ready for review July 17, 2026 10:06
@ogad-tether
ogad-tether requested review from a team as code owners July 17, 2026 10:06
ogad-tether and others added 4 commits July 17, 2026 11:09
…patch

include(GNUInstallDirs) must precede add_subdirectory(src): the whisper
INSTALL_INTERFACE expands ${CMAKE_INSTALL_INCLUDEDIR}, and when unset it
exports a bogus "/whisper" include path that breaks install-tree
find_package(whisper) consumers (the exact failure the registry's
0001-move-gnuinstalldirs patch fixed). Carrying it in the subtree delta
lets the whisper-cpp port drop its patch when it re-pins onto the
reorged tree. Declared in PATCHES.md; upstreaming candidate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- README.md: per QIP review, document every engine and supported model
  (whisper GGUFs; Parakeet CTC/TDT/EOU/Sortformer; Chatterbox Turbo/MTL,
  Supertonic, LavaSR), install + build, verified run commands for each
  CLI (flags checked against the engines' docs and the umbrella build
  outputs), and representative cross-platform perf tables sourced from
  the per-engine READMEs (linked for methodology/reproduction).
- PATCHES.md: manifest rows no longer lean on ticket IDs — each row now
  states what the patch does and why on its own, per review feedback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread tests/test-whisper-logits-slice.cpp Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to re-add these files in appropriate place

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — the delta capture had only covered src/include/cmake, so everything the fork added under other upstream paths was dropped. Restored (and now running in CI, 73/73 locally): tests/test-whisper-logits-slice.cpp, tests/test-vad-streaming.cpp + their tests/CMakeLists.txt wiring, and the Java-binding VAD params — all declared in PATCHES.md. One file deliberately NOT restored (documented in PATCHES.md): the fork's examples/talk-llama/llama-hparams hunk references nextn_predict_layers which doesn't exist in v1.9.1's talk-llama snapshot — it never compiled and was the actual cause of the ubuntu-22-* failures on every master push of the old build.yml.

Comment thread .github/workflows/whisper-ci.yml Outdated
if: steps.model-cache.outputs.cache-hit != 'true'
run: |
mkdir -p models-cache
curl -L -o models-cache/ggml-tiny.en.bin \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curl -L without --fail writes the HTTP error body (e.g. a 404/503 HTML page) into ggml-tiny.en.bin and exits 0; the failure then surfaces as a confusing whisper-cli error two steps later. Use curl -fL so a bad fetch fails the step at the source. Consider also pinning the model URL to a fixed HF revision instead of resolve/main and adding a checksum, so the smoke can't silently drift.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: curl -fL, URL pinned to HF revision 5359861c, and sha256-verified after download (shasum -a 256 -c, portable to the macOS runner).

Comment thread README.md Outdated
cut off abruptly between segments when they're concatenated together.
| Backend / platform | RTF | vs real-time |
|---|--:|--:|
| Vulkan (CI, Linux x86-64) | 0.090 | 11.1× |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vulkan (CI, Linux x86-64) replace it with the Nvidia gpu name

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do the same change everywhere else

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done everywhere: GPU rows now say NVIDIA RTX 4000 SFF Ada (Vulkan) — the qvac-ubuntu2204-x64-gpu benchmark runner documented in both engine READMEs — and Mac rows name the machine (Mac Studio M3 Ultra). For the Linux CPU rows: the CPU model of that runner isn't published in any doc/artifact I can cite (benchmark artifacts with device info have expired, and the perf reporter only detects the GPU since QVAC-20499), so the row is labeled as the same x86-64 host rather than inventing a part number. If you know the CPU model, tell me and I'll put it in — and adding CPU detection next to _detectGpu() in the perf reporter would fix this permanently.

Comment thread README.md Outdated
|---|--:|--:|
| Vulkan (CI, Linux x86-64) | 0.090 | 11.1× |
| Metal (Mac Studio M3 Ultra) | 0.16 | 6.4× |
| CPU (CI, Linux x86-64) | 1.25 | 0.80× |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace it with the right CPU name

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do the same change everywhere else

- Restore the fork files the subtree conversion dropped (the delta
  capture missed upstream-owned paths beyond src/include/cmake):
  tests/test-whisper-logits-slice.cpp, tests/test-vad-streaming.cpp
  (+ tests/CMakeLists.txt wiring) and the Java-binding VAD params
  (WhisperFullParams.java, WhisperVadParams.java). All declared in
  PATCHES.md with self-contained rationale.
- Deliberately NOT restored (documented in PATCHES.md): the fork's
  examples/talk-llama/llama-hparams hunk — it references
  nextn_predict_layers, which doesn't exist in v1.9.1's talk-llama
  snapshot, didn't compile, and was the actual cause of the ubuntu-22
  matrix failures on every master push of the old upstream build.yml.
- Run the restored tests: new SPEECH_BUILD_WHISPER_TESTS umbrella
  option; whisper-ci enables it and widens the ctest exclusion to
  whisper's model-labeled transcription tests (tiny/base/...). Gate
  upstream's parakeet tests behind WHISPER_BUILD_PARAKEET (they link
  the gated target). Local: full umbrella build clean, 73/73.
- whisper-ci model fetch: curl -fL (fail on HTTP errors instead of
  writing the error page into the model), URL pinned to a fixed HF
  revision, sha256 verified (shasum -a 256: portable to macOS runners).
- README perf tables: name the hardware — GPU rows are the
  qvac-ubuntu2204-x64-gpu runner's NVIDIA RTX 4000 SFF Ada (Vulkan),
  Mac rows Mac Studio M3 Ultra; CPU row identified as the same x86-64
  host (its CPU model is not published in any doc we can cite).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants