Skip to content

Compile the C ABI in CI, and export the tap::samplerate alias - #43

Merged
tap merged 1 commit into
mainfrom
claude/repository-inconsistencies-nkocvd
Jul 28, 2026
Merged

Compile the C ABI in CI, and export the tap::samplerate alias#43
tap merged 1 commit into
mainfrom
claude/repository-inconsistencies-nkocvd

Conversation

@tap

@tap tap commented Jul 28, 2026

Copy link
Copy Markdown
Owner

What this changes

Turns SRT_BUILD_CAPI on in the primary CI matrix (Linux and macOS legs), and exports the tap::samplerate CMake alias.

Why

This repo ships a C ABI under tools/capi that the notebooks drive via ctypes, but the option defaults to OFF and CI never turned it on — so nothing in the pipeline compiled it, and a kernel signature change could break the ABI with CI staying green. AmbiTap and DspTap already build theirs.

tap::samplerate is the alias taphouse's namespace convention has documented all along but this repo never created (see tap/TapHouse#6).

Verification

  • Configure and build succeed with SRT_WERROR=ON and the new flag.
  • libsrt_capi.so links and exports its 8 entry points (nm -D).
  • Full suite passes 73/73.

Notes for the reviewer

  • Enabled on Linux and macOS only. tools/capi carries no __declspec(dllexport) (unlike DspTap's), so an MSVC build would link a DLL exporting nothing — it would pass CI without gating anything. Recorded in the matrix comment and in taphouse's known-divergences list, to be flipped on in the same change that gives the C ABI an export decoration. The Windows leg's SRT_WERROR=OFF policy (pending /W4 triage, per docs/PERFORMANCE.md "Known debt") is untouched.
  • SampleRateTap::SampleRateTap stays alongside tap::samplerate, so no consumer changes — including RatioTap, which pins this repo as a test-only submodule for its pinned-eps cross-validation.
  • Worth flagging: this repo is the family's odd one out twice over on header paths. include/srt/ is an abbreviation matching neither the repo name nor the tap::samplerate namespace, where the convention wants tap/samplerate/. Renaming breaks every consumer #include, so it stays a migrate-when-touched item (tracked in taphouse's README) rather than something to slip into a CI change.
  • No contract change, no submodule pin moved, notebooks untouched. The instruction-count baselines are unaffected — this adds a target rather than touching codegen.

Generated by Claude Code

This repo ships a C ABI under tools/capi that the notebooks drive via ctypes, but
SRT_BUILD_CAPI defaults to OFF and CI never turned it on -- so nothing in the
pipeline compiled it, and a kernel signature change could break the ABI with CI
staying green. AmbiTap and DspTap already build theirs; this brings SampleRateTap
in line.

Enabled on the Linux and macOS legs only. tools/capi carries no
__declspec(dllexport) (unlike DspTap's), so an MSVC build would link a DLL that
exports nothing -- it would pass without gating anything. Recorded in the matrix
comment and in taphouse's known-divergences list, to be flipped on in the same
change that gives the C ABI an export decoration.

Also exports `tap::samplerate`, the alias taphouse's namespace convention has
documented all along but that this repo never created.
`SampleRateTap::SampleRateTap` stays, so existing consumers are unaffected. Note
this repo is the family's odd one out twice over on header paths: include/srt/ is
an abbreviation matching neither the repo name nor the tap::samplerate namespace.
Renaming it breaks every consumer #include, so it stays a migrate-when-touched
item (tracked in taphouse's README).

Verified locally: configure and build succeed with SRT_WERROR=ON, the shared
library links and exports its 8 entry points, and the full suite passes 73/73.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhhQ93r2E1QTnCx46YfX8j
@tap
tap merged commit 6369386 into main Jul 28, 2026
32 checks passed
@tap
tap deleted the claude/repository-inconsistencies-nkocvd branch July 28, 2026 17:27
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.

2 participants