Compile the C ABI in CI, and export the tap::samplerate alias - #43
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Turns
SRT_BUILD_CAPIon in the primary CI matrix (Linux and macOS legs), and exports thetap::samplerateCMake alias.Why
This repo ships a C ABI under
tools/capithat 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::samplerateis the alias taphouse's namespace convention has documented all along but this repo never created (see tap/TapHouse#6).Verification
SRT_WERROR=ONand the new flag.libsrt_capi.solinks and exports its 8 entry points (nm -D).Notes for the reviewer
tools/capicarries 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'sSRT_WERROR=OFFpolicy (pending/W4triage, perdocs/PERFORMANCE.md"Known debt") is untouched.SampleRateTap::SampleRateTapstays alongsidetap::samplerate, so no consumer changes — including RatioTap, which pins this repo as a test-only submodule for its pinned-eps cross-validation.include/srt/is an abbreviation matching neither the repo name nor thetap::sampleratenamespace, where the convention wantstap/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.Generated by Claude Code