Commit Graph
7 Commits
Author SHA1 Message Date
shadowdaoandClaude Sonnet 5 f2a4932eea Fix review findings: stopping-flag race, unpinned SDK download, key-leak via redirect/logs
Code review findings from before merging feat/livekit-integration to main:

- I1: sourceDestroy set self->stopping outside self->mutex, then notified.
  The worker's condition-variable predicate reads `stopping` under that same
  mutex, so the store+notify could land between the worker's predicate check
  and it entering the wait, dropping the notification and leaving the worker
  asleep for its full backoff (up to 30s) with the OBS UI thread blocked in
  worker.join(). Now set under the lock, matching how `generation` is
  already mutated in applySettings.

- I3: the LiveKit SDK archive download in cmake/LiveKitSDK.cmake had no
  SHA256 pin wired up from the top-level CMakeLists.txt, unlike the obs-deps
  bootstrap right next to it. Added real SHA256 hashes -- computed by
  downloading each release archive and running sha256sum -- for every
  triple the pinned v1.10.1 release can resolve to (Linux x64/arm64, macOS
  x64/arm64, Windows x64), keyed by version+triple so a future version bump
  fails loudly (via message(WARNING)) instead of silently going unverified.
  Verified end-to-end locally: a deliberately wrong hash makes the configure
  step fail with a HASH mismatch error. Only Linux was also build-tested in
  this environment; macOS/Windows archives were downloaded and hashed but
  not build-tested here.

- I4: the curl HTTP backend followed up to 3 redirects while the read key
  travels as a URL query parameter, so a malicious/misconfigured redirect
  (including an HTTPS->HTTP downgrade, which curl doesn't refuse by default)
  could leak the key. This client only ever talks to two fixed, first-party
  endpoints, so redirects are disabled outright (CURLOPT_FOLLOWLOCATION 0),
  matching the WinHTTP backend's existing default behavior. Left
  normalizeServerUrl's explicit-http:// pass-through as-is with a comment,
  per review guidance.

- I5: ApiClient::redactedUrl was tested but never called. No current call
  site logs a request URL, so rather than inventing one, added a one-line
  comment marking it a deliberate guard rail for future logging.

- I7: the LiveKit SDK log bridge (livekitLogToObs) wrote SDK messages
  straight into the OBS log. LiveKit's signaling URL carries the access
  token as a query parameter; defensively scrub "access_token=" and "key="
  values before they ever reach obs_log. New ApiClient::redactSensitiveParams
  generalizes redactedUrl's redaction pattern to arbitrary text (not just a
  bare URL), with 6 new unit tests in test_api_client.cpp.

- I2: added a code comment on session.cpp's auto_subscribe=true noting the
  known, unaddressed bandwidth/CPU cost of pulling every participant's
  track in multi-camera rooms, and that per-publication unsubscribe is a
  future optimization. No behavior change (out of scope per review).

Verified: cmake configure + build + `ctest --test-dir build
--output-on-failure` all pass, 6/6 suites (test_api_client now 127 checks,
up from 121).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 22:59:57 -07:00
shadowdaoandClaude Sonnet 5 6829dd545a Stage the plugin into the directory layout OBS actually searches
Build / macOS (macos-latest) (push) Successful in 32s
Build / Linux (ubuntu-24.04) (push) Successful in 46s
Build / Windows (windows-latest) (push) Failing after 9m35s
The staged package put the module in build/package/bin. OBS does not look
there. From AddExtraModulePaths() in obs-studio's UI/window-basic-main.cpp,
the per-user plugin layout on Linux and Windows is:

  <config>/obs-studio/plugins/<name>/bin/64bit/<name>.{so,dll}
  <config>/obs-studio/plugins/<name>/data/

so build/package/ now uses bin/64bit and is a straight drop-in. Re-verified in
the headless libobs harness from the new path: the module loads, both sources
connect, frames arrive, the camera switch round-trips, and `ldd` on the staged
copy resolves liblivekit and liblivekit_ffi from bin/64bit via $ORIGIN.

macOS deliberately keeps the flat bin/ -- there OBS looks for a
<name>.plugin/Contents/MacOS bundle, which this build does not produce. That
gap is documented in README.md rather than papered over with a directory name
that would only look right.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 22:36:01 -07:00
shadowdaoandClaude Sonnet 5 f281b4c382 ci: keep the synthesised macOS SDK symlink out of the dependency directory
Build / macOS (macos-latest) (push) Failing after 18s
Build / Linux (ubuntu-24.04) (push) Successful in 52s
Build / Windows (windows-latest) (push) Failing after 8m15s
libobs now builds AND installs cleanly on macOS -- libobs.framework, its
headers, and libobsConfig/libobsTargets all land. The configure then fell over
one step later, in the template's own quarantine-clearing step:

  xattr: [Errno 13] Permission denied:
    '.../.deps/sdk/MacOSX.platform/Developer/SDKs/MacOSX26.5.sdk'

`xattr -r -d com.apple.quarantine "${dependencies_dir}"` recurses, so it
followed the SDK symlink into the read-only system SDK. The symlink moves to
the build directory, where that sweep never reaches it. The xattr call itself
also stops being fatal: clearing quarantine on downloaded dependencies is a
convenience, and it should not be able to take the whole configure down.

Separately, in the adapter: the last-frame-geometry marker is cleared whenever
the session leaves Connected, so the next stream logs its first frame again.
Verified in the headless libobs harness by switching the selected camera to a
dark slot and back -- previously the switch back was silent because the
resolution had not changed, so the log stopped answering "did video come
back". It now reads:

  connected to ws://... watching cam-test
  video frame 640x360 I420
  --- switching camera to 'other-cam'
  connected to ws://... watching other-cam
  --- switching camera back to 'cam-test'
  connected to ws://... watching cam-test
  video frame 640x360 I420
  status after switch-back: connected

which also confirms the whole change-settings path: each switch mints a fresh
obs:<room>:<nonce> identity and reconnects, with no crash and no stale frame.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 22:18:41 -07:00
shadowdaoandClaude Sonnet 5 0fbcb7c2f4 ci: give the macOS OBS sub-build a version-carrying SDK path
Build / macOS (macos-latest) (push) Failing after 10s
Build / Linux (ubuntu-24.04) (push) Successful in 46s
Build / Windows (windows-latest) (push) Failing after 8m14s
Second macOS failure, after the Xcode-generator one: OBS's own
cmake/macos/compilerconfig.cmake reads the macOS SDK version by regex-matching
"MacOSX<major>.<minor>.sdk" out of CMAKE_OSX_SYSROOT, and hard-fails when that
does not match --

  string sub-command REGEX, mode MATCH needs at least 5 arguments
  Your macOS SDK version () is too low. The macOS 13.1 SDK (Xcode 14.2) is
  required to build OBS.

-- with an empty version in the message, which is the tell.

With upstream's Xcode generator CMAKE_OSX_SYSROOT stays the literal string
"macosx" and Xcode resolves it late, so that regex never runs against a real
path. With Ninja, which this project now uses because the CI runner has no
Xcode, CMake resolves it eagerly to `xcrun --show-sdk-path` -- and on a
Command-Line-Tools-only install that is the UNVERSIONED symlink
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk. So swapping the generator
moved the failure rather than removing it.

_resolve_versioned_macos_sdk now hands the sub-build a path whose filename
carries the version: a versioned sibling if the toolchain ships one (the
common layout), otherwise a symlink to the same SDK created under .deps/sdk
and named MacOSX<major>.<minor>.sdk. Either way clang gets the same SDK; only
the spelling of the path changes, which is all OBS's check looks at.

Also: the source's worker thread now backs off when the source is
unconfigured, instead of re-evaluating once a second forever, and resets the
backoff whenever the settings change -- a settings change is an operator
action and should retry immediately. Filling the settings in bumps the
generation counter and wakes the worker straight away, so the longer backoff
costs no responsiveness.

Linux re-verified: ctest 6/6.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 22:10:45 -07:00
shadowdaoandClaude Sonnet 5 7146f78831 ci: fix the three failures the first real three-platform run exposed
Build / macOS (macos-latest) (push) Failing after 10s
Build / Linux (ubuntu-24.04) (push) Successful in 3m21s
Build / Windows (windows-latest) (push) Failing after 8m18s
Linux, glibc. The LiveKit SDK's "linux-x64" asset is not actually generic:
it is built on Ubuntu 24.04 and needs GLIBC_2.38 and GLIBCXX_3.4.32, so
linking it on a 22.04 runner fails outright ("undefined reference to
std::ios_base_library_init()@GLIBCXX_3.4.32", "__isoc23_strtol@GLIBC_2.38").
That is exactly what happened when this repo's CI landed on the 22.04 Linux
runner instead of the 24.04 one. LiveKitSDK.cmake now defaults Linux to the
ubuntu-22.04 asset, which needs at most GLIBC_2.35 / GLIBCXX_3.4.30 (checked
with objdump against both archives) and therefore links and runs on 22.04 and
on everything newer -- the right floor for a plugin handed to directors as a
binary.

Linux, libobs version. The Linux job is pinned to ubuntu-24.04 rather than
ubuntu-latest, which this instance's two Linux runners answer with different
releases. 24.04's libobs-dev is 30.0.2 -- exactly the OBS version
buildspec.json pins for macOS/Windows -- so all three platforms build against
the same libobs. A 22.04 runner would have given OBS 27, a different API
surface.

macOS, no Xcode. The OBS sub-build failed its configure with "No
CMAKE_C_COMPILER could be found": the template hardcodes the Xcode generator,
and the `home-mac` runner has the Command Line Tools but no xcodebuild. The
sub-build now uses Ninja (with an explicit CMAKE_BUILD_TYPE, since Ninja is
single-config) and builds a single architecture rather than upstream's forced
universal -- this plugin is single-arch anyway, because client-sdk-cpp ships
single-arch dylibs, so a universal libobs would double the slowest step in CI
for a slice nothing links against.

While in there, generator flags are built as proper CMake lists so each
becomes its own argv entry. Upstream packs several into one space-separated
string and passes it unquoted, which execute_process hands to cmake as a
single argument; it happens not to matter for the optional flags upstream
passes, but it would silently swallow -DCMAKE_BUILD_TYPE.

Also lowers the libobs API floor in the adapter: video_format_get_parameters
instead of video_format_get_parameters_for_format. The _for_format variant
only exists from libobs 30 onwards and only differs for the 10-bit formats
(I010/P010) this source never receives, so using the older entry point keeps
the module loadable on an older OBS -- the direction that matters, since OBS
refuses modules built against a NEWER libobs than the one running.

Re-verified locally on Ubuntu 24.04 with the ubuntu-22.04 SDK asset:
ctest 6/6; the real-LiveKit integration test still reports "36 video frames,
323 audio frames, 10 state changes / 32 checks passed"; and the headless
libobs harness still logs "connected to ws://127.0.0.1:7880 ... watching
cam-test" followed by "video frame 640x360 I420" with the camera dropdown
populated from the live slot list.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 22:08:03 -07:00
shadowdaoandClaude Sonnet 5 a484abec61 Make the OBS adapter real: properties UI, connect, and frame output
The stub source becomes an actual streamer-tools camera. On create it reads
server URL / room slug / read key / camera identity from obs_data_t, mints a
subscribe-only token through ApiClient, connects LiveKitSession, and pushes
decoded frames into obs_source_output_video / obs_source_output_audio. The
source is now OBS_SOURCE_ASYNC_VIDEO | OBS_SOURCE_AUDIO |
OBS_SOURCE_DO_NOT_DUPLICATE with an OBS_ICON_TYPE_CAMERA icon.

The file is C++ rather than C now: the core library's API is C++ and the C ABI
shim existed only to avoid that. obs-module.h already declares the module
entry points extern "C", so nothing is lost.

Properties UI: server URL, room slug, a masked read-key field (it is a
credential and is masked everywhere else in streamer-tools), a camera dropdown,
a "Refresh camera list" button, and a status line.

- The dropdown is built from a cache the worker keeps warm on every connect,
  so opening properties never blocks on the network. The button is the
  explicit way to force a round trip, with a shortened 5s timeout -- for which
  ApiClient's two calls gained a timeout_ms parameter.
- The currently-selected identity is always in the list, labelled "(not in
  this room)" if absent, so OBS cannot silently clear a working setting just
  because the room happens to be dark.
- The status line is the OBS_TEXT_INFO property's description (which is what
  OBS actually renders) and switches to the warning info type on a real error.

Threading: OBS's UI and graphics threads are never blocked on the network.
Each source owns a worker thread that mints, connects, and reconnects with
exponential backoff (1s -> 30s), waking early on any settings change via a
generation counter. Frames are pushed from LiveKitSession's reader threads
directly; obs_source_output_video/_audio are thread-safe.

Two details that matter operationally:
 - A null frame is pushed whenever the session leaves Connected, so a camera
   that stopped publishing clears instead of leaving its last frame on screen.
   Leaving stale media up is precisely the failure this plugin exists to avoid.
 - The SDK's own logging is routed into OBS's log file via
   livekit::setLogCallback, instead of stderr where a director would never
   see it. The adapter also logs the first frame and every later geometry
   change, so a log answers "did video ever arrive, and at what size".

Packaging: the build now stages a runnable layout into build/package/ --
the module (RPATH $ORIGIN / @loader_path, so it resolves the LiveKit
libraries from beside itself rather than from the build tree), liblivekit +
liblivekit_ffi, the locale data, and the licence files. third_party/livekit/
carries client-sdk-cpp's Apache-2.0 LICENSE and NOTICE from the pinned tag.

Its README records a correction to the design doc: the "bundled LICENSE.md
with ~28 third-party licence blocks" the doc expects DOES NOT EXIST at
v1.10.1 -- not in any of the five release archives (which contain only
include/, lib/, bin/ and build-info.json) and not in the repo at that tag,
which has only LICENSE and NOTICE. The aggregated third-party notice covering
the WebRTC/OpenH264 code inside liblivekit_ffi.so has not been located, and
that is flagged as an open licensing question rather than papered over.

Verified on Ubuntu 24.04 against real libobs 30.0.2, a real
livekit-server 1.13.6, and a stand-in API serving plugin.routes.ts's exact
shapes, using a headless libobs harness (obs_startup + obs_reset_audio +
obs_reset_video + obs_open_module + obs_source_create):

  registered=1  output_flags=0x87
  [streamer-tools-camera] connected to ws://127.0.0.1:7880 as
      obs:main-room:qY85r9D0PaPt, watching cam-test
  [streamer-tools-camera] video frame 640x360 I420
  camera dropdown has 3 items:
    [0] (no camera selected) =
    [1] Test Camera = cam-test
    [2] Dark Camera (offline) = other-cam
  status: connected (info_type=0)

and with a deliberately wrong read key:

  status: unknown room slug, or the read key is wrong or has been rotated
      (info_type=1)

with retry-and-backoff and no crash. ctest: 6/6 passed.

Still unverified, and the README says so plainly: the OBS GUI on any platform,
macOS/Windows beyond compiling, A/V sync, and end-to-end latency.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 21:54:11 -07:00
shadowdaoandClaude Sonnet 5 105f1041ab Scaffold core/OBS-adapter split, prove CMake toolchain, add 3-platform CI
Build / Windows (windows-latest) (push) Failing after 10s
Build / macOS (macos-latest) (push) Successful in 20s
Build / Linux (ubuntu-latest) (push) Successful in 31s
First pass on the streamer-tools OBS camera plugin: a minimal but real
CMake project matching the design doc's core-library/OBS-adapter split
(docs/superpowers/specs/2026-09-06-obs-camera-plugin-design.md in the
streamer-tools repo). No LiveKit FFI integration yet -- this proves the
toolchain works.

- core/: dependency-free C++17 library (no OBS dependency), unit tested
  via CTest with no external test framework.
- obs-adapter/: adapted from obsproject/obs-plugintemplate (commit
  3e7d7ac, 2025-12-09). Registers a real, stubbed OBS source type;
  builds as a genuine dynamically-linked OBS module against Ubuntu's
  system libobs-dev (confirmed via ldd/nm, not a fake stand-in).
- Simpler hand-written top-level CMakeLists.txt in place of the
  template's full buildspec-driven bootstrap (which downloads full OBS
  source + prebuilt deps) -- find_package(libobs) alone is enough on
  Linux; falls back to core-library-only when libobs isn't found
  (expected on macOS/Windows CI for now).
- .gitea/workflows/build.yml: 3-platform matrix (ubuntu-latest,
  macos-latest, windows-latest) matching the runners confirmed
  available to this repo under the CyberCoveLLC org.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 20:46:08 -07:00