Skip to content

feat(obs): vendor the OBS plugin in-tree under cpp/obs#1787

Open
kixelated wants to merge 11 commits into
mainfrom
claude/sweet-sammet-d544bc
Open

feat(obs): vendor the OBS plugin in-tree under cpp/obs#1787
kixelated wants to merge 11 commits into
mainfrom
claude/sweet-sammet-d544bc

Conversation

@kixelated

@kixelated kixelated commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Vendors the OBS Studio plugin from moq-dev/obs into the monorepo as a slim copy under a new top-level cpp/ directory (cpp/obs/), so it compiles against the in-tree libmoq instead of a downloaded release. Drops the upstream build-system overhead (~2800 lines of build-aux/ scripts and the .github/ template CI) and keeps just enough to build on each platform: src/, the CMake template, CMakePresets.json, buildspec.json, and the locale data. Also ports the merged upstream obs#42 dock cleanup.

cpp/ is a new language-dir sibling to rs/, js/, py/, go/ for C/C++ consumers of libmoq.

How it links MoQ

CMake's MOQ_LOCAL points at the repo root (set by cpp/obs/justfile), so libmoq builds from the in-tree rs/libmoq crate via cargo and links statically. No release tarball download, no version skew.

Per-platform build

Platform Approach
Linux nix dev shell provides obs-studio/qt6/ffmpeg/clang-tools/gersemi (added to flake.nix as obsDeps, Linux-only since nixpkgs marks obs-studio broken on Darwin). nix develop && just obs build.
macOS Native, not nix. buildspec.json downloads prebuilt libobs/Qt6; ffmpeg + pkg-config come from Homebrew. Needs full Xcode and must run outside the nix shell (its DEVELOPER_DIR/NIX_LDFLAGS break the Xcode generator).
Windows Native Visual Studio 2022 + buildspec. Verified: cmake --preset windows-x64 + cmake --build builds obs-moq.dll and it loads into a stock OBS Studio install.

The split (nix on Linux, native on macOS/Windows) is forced by libobs being absent in nixpkgs on Darwin, not a stylistic choice. The OBS buildspec has no Linux entry, so nix is the right call there anyway.

Changes

  • New cpp/obs/ slim plugin tree (sources + CMake + presets + buildspec + locale + GPL LICENSE). Build artifacts (build_*, .deps, cmake/.CMakeBuildNumber) gitignored.
  • cpp/obs/justfile with setup/build/run/check/fix/preset. run targets a stock OBS install (no local OBS fork build required anymore).
  • mod obs 'cpp/obs' in the root justfile.
  • flake.nix: Linux-only obsDeps in the dev shell.
  • Ported obs#42: full-width dock inputs, "Broadcast path" -> "Broadcast name", drop the Statistics box for a Disconnected/Connecting/Connected indicator, concrete stylesheet colors.
  • doc/bin/obs.md rewritten for the in-tree build; CLAUDE.md gets a cpp/ Project Structure entry and a Cross-Package Sync row (rs/libmoq C ABI -> cpp/obs/src).
  • Release: the plugin ships with libmoq. libmoq.yml (on a libmoq-v* tag) rebuilds the plugin against the libmoq release it just published (cpp/obs/build.sh --libmoq-release, CMake FetchContent — no second cargo build, ABI guaranteed) and cuts a parallel obs-moq-v<version> release. Linux x86_64/arm64 (Nix), macOS arm64 + Windows x64 (native). Unsigned archives.
  • One source edit: cpp/obs/cmake/macos/buildspec.cmake makes the xattr quarantine strip non-fatal (it errors on build outputs that carry no quarantine attribute).

Licensing

The plugin is GPL-2.0-or-later because it links OBS Studio's libobs, which is GPL-2.0-only with no linking exception. That copyleft is confined to cpp/obs/: it's a separately-distributable work, and per GPLv2 §2 its presence in this repo is mere aggregation that does not affect the MIT/Apache licensing of the rest of the project. libmoq and the other moq crates stay MIT/Apache (permissive-into-GPL is one-way; libmoq is not relicensed). To keep this legible:

  • SPDX-License-Identifier: GPL-2.0-or-later on every cpp/obs source file.
  • A licensing-exception note in the root README.md.
  • cpp/obs/LICENSE holds the full GPL text.

Public API

No changes to any pub Rust item or exported JS/TS symbol. Additive only, so this targets main.

Test plan

  • macOS: just obs setup && just obs build produces a 20 MB arm64 obs-moq.plugin linking libobs/Qt6/ffmpeg dynamically with libmoq static. Re-verified after the cpp/obs/ move, the obs#42 port, and the SPDX additions.
  • Linux: verified on an aarch64 Linux + Nix host (OCI A1). nix develop + just obs setup && just obs build builds the in-tree rs/libmoq and links obs-moq.so against nix obs-studio 32.1.2 / Qt6 / ffmpeg. This caught a justfile bug (MOQ_LOCAL path math wrong in module invocation) and confirmed the obs-frontend-api discovery resolves on nix.
  • Windows: native build verified on Windows 11 with VS 2022 Build Tools (MSVC 14.44) + Windows SDK 10.0.22621. cmake --preset windows-x64 && cmake --build --preset windows-x64 produces a 10.9 MB obs-moq.dll (libobs/Qt6/ffmpeg dynamic, libmoq static), and it loads into stock OBS Studio 32.1.0 (built against the 31.1.1 buildspec headers; ABI compatible). Two fixes were needed: (1) FFmpeg discovery used pkg_check_modules, but the Windows obs-deps ship ffmpeg without pkg-config .pc files, so Windows now uses find_package(FFmpeg) via a vendored copy of OBS's FindFFmpeg finder; (2) rs/libmoq did not propagate the Windows system libs Rust std links (ntdll/userenv/ws2_32/bcrypt/dbghelp/advapi32/legacy_stdio_definitions), so linking moq.lib failed on __imp_NtCreateNamedPipeFile. The CI obs.yml Windows leg is still not exercised by a real tag. Note: OBS logs a non-fatal Failed to load 'en-US' text because cpp/obs/data/locale/en-US.ini is committed empty.
  • Release: build.sh --libmoq-release 0.3.6 verified on macOS arm64 — fetches the published libmoq release and links it with no rust toolchain present (in-tree libmoq is 0.3.6, so ABI matches). Plain build.sh (in-tree libmoq) verified on macOS arm64 + Linux aarch64.

(Written by Claude)

Bring the OBS Studio plugin (github.com/moq-dev/obs) into the monorepo as a
slim copy under a new cpp/ top-level, so it builds against the in-tree libmoq
instead of a downloaded release. Drops the upstream build-aux/ scripts and
.github/ template CI; keeps src/, the CMake template, presets, buildspec, and
locale.

CMake's MOQ_LOCAL points at the repo root, so libmoq builds from rs/libmoq via
cargo and links statically (no release tarball).

Build wiring per platform:
- Linux: the nix dev shell provides obs-studio/qt6/ffmpeg/clang-tools/gersemi
  (Linux-only; nixpkgs marks obs-studio broken on Darwin). `just obs build`.
- macOS: native, not nix. buildspec.json downloads libobs/Qt6; ffmpeg and
  pkg-config come from Homebrew; needs full Xcode and must run outside the nix
  shell. Verified end to end here (arm64 obs-moq.plugin).
- Windows: native Visual Studio 2022 + buildspec. Documented, not verified.

Also makes the macOS buildspec xattr quarantine strip non-fatal (it chokes on
build outputs that carry no quarantine attribute).

The plugin is GPL-2.0, isolated under cpp/obs with its own LICENSE.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This pull request introduces cpp/obs, a comprehensive OBS Studio plugin called obs-moq that publishes video and audio to, and subscribes video from, MoQ (Media over QUIC) relays. The plugin links against the in-tree rs/libmoq C ABI and registers three OBS outputs (combined AV, video-only, audio-only), one OBS service backend, one async video source, and an optional Qt dock widget for stream control. The C++ implementation includes FFmpeg-based video decoding with libswscale RGBA conversion for the subscriber path and per-encoder MoQ media track management for the publisher. A complete cross-platform CMake build system is introduced with shared common modules (bootstrap, buildspec dependency downloader, ccache, compiler options) and platform-specific configurations for macOS (Xcode integration, code signing, installer packaging), Linux (GCC/Clang compiler flags, CPack DEB packaging), and Windows (MSVC/Clang conformance, Windows SDK validation, RC resource template). CMakePresets.json defines configure and build presets for all platforms with CI variants. Repository-level integration includes a top-level justfile module declaration, Nix flake dependency set for OBS tools, updated CLAUDE.md project structure, GPL-2.0 license, plugin README, and build instructions in doc/bin/obs.md.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: vendoring the OBS plugin into the cpp/obs directory to compile against in-tree libmoq.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description comprehensively explains the purpose, scope, implementation approach, per-platform build strategies, and licensing considerations for vendoring the OBS Studio plugin.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/sweet-sammet-d544bc

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 19

🧹 Nitpick comments (1)
cpp/obs/cmake/common/bootstrap.cmake (1)

34-41: ⚡ Quick win

Move cleanup before FATAL_ERROR to ensure it executes.

Line 40 file(REMOVE_RECURSE ...) is unreachable because message(FATAL_ERROR ...) on line 36 terminates processing immediately. Reorder to clean up before the fatal error:

🔧 Proposed fix
 if("${CMAKE_CURRENT_BINARY_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
+  file(REMOVE_RECURSE "${CMAKE_CURRENT_SOURCE_DIR}/CMakeCache.txt" "${CMAKE_CURRENT_SOURCE_DIR}/CMakeFiles")
   message(
     FATAL_ERROR
     "In-source builds are not supported. "
     "Specify a build directory via 'cmake -S <SOURCE DIRECTORY> -B <BUILD_DIRECTORY>' instead."
   )
-  file(REMOVE_RECURSE "${CMAKE_CURRENT_SOURCE_DIR}/CMakeCache.txt" "${CMAKE_CURRENT_SOURCE_DIR}/CMakeFiles")
 endif()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/obs/cmake/common/bootstrap.cmake` around lines 34 - 41, The file cleanup
command using REMOVE_RECURSE is placed after the FATAL_ERROR message, making it
unreachable code since FATAL_ERROR terminates cmake processing immediately. Move
the file(REMOVE_RECURSE) call that removes CMakeCache.txt and CMakeFiles to
execute before the message(FATAL_ERROR) call, ensuring the cleanup of temporary
build artifacts happens before cmake exits with a fatal error. This way the
in-source build remnants are properly cleaned up when the error is detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/obs/buildspec.json`:
- Line 4: Update all pinned dependency versions in the buildspec.json file to
their latest stable releases. Change the obs-studio version field from 31.1.1 to
32.1.2, update the prebuilt obs-deps version from 2025-07-11 to 2026-06-02, and
update the qt6 version to 6.11.1 or the latest version corresponding to the
2026-06-02 release date. Ensure all three dependencies are updated before
merging to address the security and compatibility gaps introduced by the
outdated pins.

In `@cpp/obs/cmake/common/buildspec_common.cmake`:
- Around line 61-62: The single quotes in the CMake `-D` arguments on lines
61-62 are being passed literally by execute_process instead of being interpreted
as shell quotes, which corrupts the values passed to the child cmake configure.
Remove all single quotes from the _cmake_arch variable definition (around
'arm64;x86_64') and the _cmake_extra variable definition (around the
CMAKE_OSX_DEPLOYMENT_TARGET value). Apply the same fix to the similar variable
definitions on lines 69-72 that also contain single quotes in `-D` arguments.

In `@cpp/obs/cmake/common/osconfig.cmake`:
- Around line 15-17: In the cmake configuration block for Linux/FreeBSD/OpenBSD
(identified by the condition matching "Linux|FreeBSD|OpenBSD"), the code
currently sets CMAKE_CXX_EXTENSIONS FALSE but leaves CMAKE_C_EXTENSIONS unset,
creating inconsistency with other platforms. Add a line to set
CMAKE_C_EXTENSIONS FALSE in this same block, placing it alongside or near the
CMAKE_CXX_EXTENSIONS setting to ensure both C and C++ extensions are disabled
consistently across all platforms.

In `@cpp/obs/cmake/linux/defaults.cmake`:
- Around line 51-82: The ENABLE_FRONTEND_API block is nested inside the if(NOT
TARGET OBS::libobs) condition, causing OBS::obs-frontend-api target to never be
created when OBS::libobs already exists. Move the entire if(ENABLE_FRONTEND_API)
block outside and after the if(NOT TARGET OBS::libobs) endif() so it always
executes independently. Additionally, add a fail-fast check after finding
obs-frontend-api_INCLUDE_DIR and obs-frontend-api_LIBRARY to ensure both are
found; if either is missing, use message(FATAL_ERROR) to stop the build with a
clear error message.

In `@cpp/obs/cmake/macos/defaults.cmake`:
- Around line 6-13: The initialization of CODESIGN_IDENTITY is nested inside the
if(NOT CODESIGN_TEAM) block, causing it to only run when CODESIGN_TEAM is unset.
This creates inconsistent behavior when CODESIGN_TEAM is already set but
CODESIGN_IDENTITY is not. Move the CODESIGN_IDENTITY check and its conditional
set() call outside and after the CODESIGN_TEAM block so that CODESIGN_IDENTITY
is initialized independently and always gets its default ad-hoc value when not
specified.

In `@cpp/obs/cmake/macos/helpers.cmake`:
- Around line 67-69: In the install command for the dSYM file, change the file
extension from `.dsym` to `.dSYM` to match the correct macOS debug symbol bundle
casing. The line installing `$<TARGET_BUNDLE_DIR:${target}>.dsym` should be
updated to use `.dSYM` (uppercase D and S) to ensure the symbols are correctly
found and installed on case-sensitive filesystems, preventing silent failures
during the installation process.

In `@cpp/obs/cmake/macos/resources/ccache-launcher-c.in`:
- Around line 1-5: The launcher scripts declare `#!/bin/sh` but use
bash-specific `[[ ... ]]` conditionals with `==` operator on lines 3, 18, and
23, which breaks on systems where /bin/sh is not bash-compatible. Replace all
instances of `[[ "$1" == "${CMAKE_C_COMPILER}" ]]` and similar constructs in
both ccache-launcher-c.in and ccache-launcher-cxx.in with POSIX-compatible
syntax using single brackets `[ ... ]` instead, or alternatively change the
shebang from `#!/bin/sh` to `#!/bin/bash` at the top of both files. The
POSIX-compatible approach is preferred to maintain broader shell compatibility.

In `@cpp/obs/cmake/macos/resources/ccache-launcher-cxx.in`:
- Around line 1-5: The script uses a POSIX-incompatible shebang and
bash-specific syntax. Replace the `#!/bin/sh` shebang with `#!/bin/bash` or
replace all instances of the `[[ ... ]]` conditional syntax (used in the
comparisons on lines 3, 19, and 24) with the POSIX-compatible single bracket
syntax `[ ... ]`. Additionally, fix the incorrect compiler ID variable reference
on line 19 by replacing `${CMAKE_C_COMPILER_ID}` with `${CMAKE_CXX_COMPILER_ID}`
since this is the C++ launcher script, not the C launcher.

In `@cpp/obs/cmake/windows/helpers.cmake`:
- Around line 57-58: The target_sources call is currently hardcoded to use
${CMAKE_PROJECT_NAME} as the target, which causes the RC file to be attached to
the wrong target if this helper function is reused. Replace
${CMAKE_PROJECT_NAME} with the target function argument in the target_sources
call to ensure the generated RC file is properly attached to the intended target
that was passed to this helper function.

In `@cpp/obs/CMakeLists.txt`:
- Around line 31-47: The MOQ_LOCAL option on line 31 defaults to an empty string
instead of the in-tree libmoq path, causing it to fall back to downloading the
release. Change the default value of the MOQ_LOCAL option from empty string to
point to the in-tree libmoq directory (adjust the path relative to the
CMakeLists.txt location to match where rs/libmoq is located in the repository
structure). Additionally, on line 34 in the add_subdirectory call, wrap the
${MOQ_LOCAL}/rs/libmoq path in quotes to prevent issues when the path contains
spaces.

In `@cpp/obs/CMakePresets.json`:
- Around line 23-24: The macOS preset configuration has a mismatch between its
labels and actual architecture setup. The displayName "macOS Universal" and
description mentioning "Universal binary" at lines 23-24 are misleading because
the architectures field at line 41 only specifies arm64. Either rename the
displayName and description to accurately reflect that this preset builds for
arm64-only (for example, change "Universal" to "Apple Silicon" or "ARM64"), or
alternatively add x86_64 to the architectures list to make the preset truly
universal. Apply the same fix approach consistently across all related presets
mentioned in the affected lines (lines 52-54 and 137-146).
- Around line 114-116: The CMAKE_INSTALL_LIBDIR setting is using
CMAKE_SYSTEM_PROCESSOR as a literal string instead of expanding it as a CMake
variable, which results in incorrect install directory paths. In the
CMakePresets.json file, replace the CMAKE_INSTALL_LIBDIR value on line 115 to
use proper CMake variable expansion syntax by changing
"lib/CMAKE_SYSTEM_PROCESSOR-linux-gnu" to
"lib/${CMAKE_SYSTEM_PROCESSOR}-linux-gnu" so that the actual processor
architecture value is substituted at build time.

In `@cpp/obs/justfile`:
- Around line 31-36: The build recipe comment claims it runs setup first if you
haven't, but the implementation does not actually call setup. Add a call to just
setup at the beginning of the build recipe before the cmake --build command to
match the documented behavior. This ensures the setup recipe is executed on the
first run to properly initialize the build environment.

In `@cpp/obs/src/moq-dock.cpp`:
- Around line 421-437: In the OnOutputStopped function, the raw `self` pointer
is captured in the lambda queued with Qt::QueuedConnection, creating a
use-after-free risk if MoQDock is destroyed before the lambda executes. Replace
the raw pointer capture with a QPointer by creating a QPointer<MoQDock> from
self before the QMetaObject::invokeMethod call, then modify the lambda capture
to use the QPointer instead of the raw self pointer. Inside the lambda, add a
null check on the QPointer before accessing any member functions like
StopStream() or the status widget to ensure the object still exists when the
queued lambda finally executes.

In `@cpp/obs/src/moq-output.cpp`:
- Around line 10-18: The MoQOutput class has inconsistent validity predicates
for session and handle validation that cause resource leaks. The issue is that
session and handle are initialized to 0, but Start() treats values less than 0
as failure (meaning 0 is valid), while Stop() and cleanup code only check for
values greater than 0, which leaks valid handles with value 0. Fix this by:
changing the initialization of the session member variable in the constructor
from 0 to -1 to match the pattern used in moq-source.cpp where -1 represents
invalid handles; updating the validity checks in Stop() method from checking
session > 0 to session >= 0; updating similar validity checks for handle
variables from handle > 0 to handle >= 0 in the cleanup sections; and setting
session = -1 after successfully closing the session to properly invalidate the
handle.
- Around line 22-28: The MoQOutput destructor has a race condition where
session_connect_callback captures a raw this pointer and accesses member
variables (connect_start, connect_time_ms, server_url) without synchronization.
Before the destructor closes the session resources with moq_publish_close and
moq_origin_close, add synchronization to wait for any pending callbacks to
complete (consider using a flag or condition variable to track callback
completion). Additionally, the current call to Stop() with its default parameter
emits OBS_OUTPUT_SUCCESS during shutdown which is misleading; either remove the
Stop() call from the destructor entirely or explicitly pass false to Stop() to
indicate abnormal termination. Ensure member variable access in
session_connect_callback is protected by the same synchronization mechanism used
in the destructor.
- Around line 61-72: In the Start() method, modify the video encoder validation
logic that checks for encoders starting at line 61. Before performing the
unconditional loop with obs_output_get_video_encoder2(), first retrieve the
output flags using obs_output_get_flags() to determine the output type. Only
enforce the video encoder requirement when the OBS_OUTPUT_VIDEO flag is set, and
similarly only require audio encoder validation when the OBS_OUTPUT_AUDIO flag
is set. This allows audio-only outputs (registered with OBS_OUTPUT_AUDIO flag)
to proceed without requiring a video encoder.

In `@cpp/obs/src/moq-source.cpp`:
- Around line 263-304: The error path in the on_session_status callback function
reads ctx->generation once at the beginning but does not re-validate it before
mutating ctx->session and ctx->origin, creating a race condition where a stale
callback can close handles belonging to a new generation. Add a generation check
in the error path (before the cleanup code that closes ctx->session and
ctx->origin at lines 296-301) to ensure ctx->generation still matches the
current_gen variable captured earlier, similar to how moq_source_start_consume
validates generation in the success path. Only proceed with cleanup if the
generation still matches.
- Around line 172-186: Replace the timing-based workaround at the
os_sleep_ms(100) call with a robust reference counting or callback completion
mechanism. Implement a counter that increments when entering any callback
function (particularly moq_source_decode_frame) and decrements when exiting,
then wait for this counter to reach zero before freeing ctx instead of relying
on the 100ms sleep. This accounts for the fact that FFmpeg decoding operations
in moq_source_decode_frame can exceed 100ms and create a real race condition.
Consider using std::atomic for the counter and a condition variable to signal
when all callbacks have completed.

---

Nitpick comments:
In `@cpp/obs/cmake/common/bootstrap.cmake`:
- Around line 34-41: The file cleanup command using REMOVE_RECURSE is placed
after the FATAL_ERROR message, making it unreachable code since FATAL_ERROR
terminates cmake processing immediately. Move the file(REMOVE_RECURSE) call that
removes CMakeCache.txt and CMakeFiles to execute before the message(FATAL_ERROR)
call, ensuring the cleanup of temporary build artifacts happens before cmake
exits with a fatal error. This way the in-source build remnants are properly
cleaned up when the error is detected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fbfa651c-7217-4f92-83cf-2a6bcc1e1fb7

📥 Commits

Reviewing files that changed from the base of the PR and between cdc07e0 and 570498b.

📒 Files selected for processing (49)
  • CLAUDE.md
  • cpp/obs/.clang-format
  • cpp/obs/.gersemirc
  • cpp/obs/.gitignore
  • cpp/obs/CMakeLists.txt
  • cpp/obs/CMakePresets.json
  • cpp/obs/LICENSE
  • cpp/obs/README.md
  • cpp/obs/buildspec.json
  • cpp/obs/cmake/common/bootstrap.cmake
  • cpp/obs/cmake/common/buildnumber.cmake
  • cpp/obs/cmake/common/buildspec_common.cmake
  • cpp/obs/cmake/common/ccache.cmake
  • cpp/obs/cmake/common/compiler_common.cmake
  • cpp/obs/cmake/common/helpers_common.cmake
  • cpp/obs/cmake/common/osconfig.cmake
  • cpp/obs/cmake/linux/compilerconfig.cmake
  • cpp/obs/cmake/linux/defaults.cmake
  • cpp/obs/cmake/linux/helpers.cmake
  • cpp/obs/cmake/macos/buildspec.cmake
  • cpp/obs/cmake/macos/compilerconfig.cmake
  • cpp/obs/cmake/macos/defaults.cmake
  • cpp/obs/cmake/macos/helpers.cmake
  • cpp/obs/cmake/macos/resources/ccache-launcher-c.in
  • cpp/obs/cmake/macos/resources/ccache-launcher-cxx.in
  • cpp/obs/cmake/macos/resources/create-package.cmake.in
  • cpp/obs/cmake/macos/resources/distribution.in
  • cpp/obs/cmake/macos/resources/installer-macos.pkgproj.in
  • cpp/obs/cmake/macos/xcode.cmake
  • cpp/obs/cmake/windows/buildspec.cmake
  • cpp/obs/cmake/windows/compilerconfig.cmake
  • cpp/obs/cmake/windows/defaults.cmake
  • cpp/obs/cmake/windows/helpers.cmake
  • cpp/obs/cmake/windows/resources/resource.rc.in
  • cpp/obs/data/locale/en-US.ini
  • cpp/obs/justfile
  • cpp/obs/src/logger.h
  • cpp/obs/src/moq-dock.cpp
  • cpp/obs/src/moq-dock.h
  • cpp/obs/src/moq-output.cpp
  • cpp/obs/src/moq-output.h
  • cpp/obs/src/moq-service.cpp
  • cpp/obs/src/moq-service.h
  • cpp/obs/src/moq-source.cpp
  • cpp/obs/src/moq-source.h
  • cpp/obs/src/obs-moq.cpp
  • doc/bin/obs.md
  • flake.nix
  • justfile

Comment thread cpp/obs/buildspec.json
Comment thread cpp/obs/cmake/common/buildspec_common.cmake Outdated
Comment thread cpp/obs/cmake/common/osconfig.cmake
Comment thread cpp/obs/cmake/linux/defaults.cmake
Comment thread cpp/obs/cmake/macos/defaults.cmake Outdated
Comment thread cpp/obs/src/moq-output.cpp
Comment thread cpp/obs/src/moq-output.cpp
Comment thread cpp/obs/src/moq-output.cpp
Comment on lines +172 to +186
// Give MoQ callbacks time to drain - they check shutting_down and exit early.
// This prevents use-after-free when async callbacks fire after ctx is freed.
//
// LIMITATION: This 100ms sleep is a timing-based workaround, not a synchronization
// guarantee. If a callback is mid-execution when shutting_down is set AND takes
// longer than 100ms to complete (after the mutex unlock), there is still a
// potential race condition. In practice, our callbacks are fast (< 1ms typically)
// and this delay provides sufficient margin. However, a more robust solution
// would use reference counting:
// - Increment refcount when entering a callback
// - Decrement when exiting
// - Wait for refcount to reach zero before freeing ctx
// This could be implemented using std::shared_ptr or a manual atomic refcount
// with a condition variable for waiting.
os_sleep_ms(100);

@coderabbitai coderabbitai Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify destroy path vs callback entry points for quiescence guarantees.
rg -n 'moq_source_destroy|os_sleep_ms\(100\)|on_session_status|on_catalog|on_video_frame|shutting_down' cpp/obs/src/moq-source.cpp

Repository: moq-dev/moq

Length of output: 1709


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '162,220p'

Repository: moq-dev/moq

Length of output: 2762


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '263,330p'

Repository: moq-dev/moq

Length of output: 2514


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '394,430p'

Repository: moq-dev/moq

Length of output: 1376


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '600,750p' | grep -A 50 'moq_source_disconnect_locked'

Repository: moq-dev/moq

Length of output: 37


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '311,380p'

Repository: moq-dev/moq

Length of output: 2579


🏁 Script executed:

rg -n 'moq_source_disconnect_locked' cpp/obs/src/moq-source.cpp -A 30

Repository: moq-dev/moq

Length of output: 6444


🏁 Script executed:

rg -n 'moq_source_disconnect_locked' cpp/obs/src/moq-source.cpp -B 2

Repository: moq-dev/moq

Length of output: 911


🏁 Script executed:

rg -n 'moq_source_start_consume|moq_source_init_decoder|moq_source_decode_frame|moq_source_blank_video' cpp/obs/src/moq-source.cpp | head -20

Repository: moq-dev/moq

Length of output: 1092


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '430,550p' | grep -A 30 'moq_source_decode_frame'

Repository: moq-dev/moq

Length of output: 37


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '764,850p'

Repository: moq-dev/moq

Length of output: 3471


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '508,580p'

Repository: moq-dev/moq

Length of output: 2812


🏁 Script executed:

cat -n cpp/obs/src/moq-source.cpp | sed -n '621,665p'

Repository: moq-dev/moq

Length of output: 1923


Shutdown timing-based workaround is insufficient for decoder operations

The 100ms sleep after os_sleep_ms(100) does not guarantee callbacks complete before ctx is freed. Specifically, moq_source_decode_frame() can execute FFmpeg decoding operations (avcodec_send_packet(), avcodec_receive_frame()) that routinely exceed 100ms on video frames, creating a real race condition where the decoder accesses ctx after it's freed at line 194.

The code's assumption that callbacks are "< 1ms typically" (line 178) does not account for the heavy FFmpeg operations in the decode path. A robust solution using reference counting or a callback completion counter is needed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/obs/src/moq-source.cpp` around lines 172 - 186, Replace the timing-based
workaround at the os_sleep_ms(100) call with a robust reference counting or
callback completion mechanism. Implement a counter that increments when entering
any callback function (particularly moq_source_decode_frame) and decrements when
exiting, then wait for this counter to reach zero before freeing ctx instead of
relying on the 100ms sleep. This accounts for the fact that FFmpeg decoding
operations in moq_source_decode_frame can exceed 100ms and create a real race
condition. Consider using std::atomic for the counter and a condition variable
to signal when all callbacks have completed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Acknowledged: pre-existing 100ms-sleep teardown workaround in the upstream plugin; the refcount/quiescence rework needs runtime testing. Tracking for upstream.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@kixelated, understood — good to know this is a pre-existing pattern from upstream rather than newly introduced. Would you like me to open a GitHub issue to track the refcount/quiescence rework? That way it's on record with the relevant context (callback race, FFmpeg decode path, suggested std::atomic counter + condition variable approach) for when you're ready to address it upstream.

Comment thread cpp/obs/src/moq-source.cpp
kixelated and others added 10 commits June 18, 2026 17:54
Port github.com/moq-dev/obs#42 into the vendored copy:
- Full-width URL and Broadcast-name fields (QFormLayout::AllNonFixedFieldsGrow).
- Rename "Broadcast path" to "Broadcast name".
- Drop the Statistics box (it showed OBS output counters, not libmoq data) in
  favor of a Disconnected / Connecting / Connected indicator driven by the
  session-connect state libmoq reports.
- Use concrete stylesheet colors; Qt style sheets ignore the palette() function.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The OBS plugin links libobs (GPL-2.0, no linking exception), so it must be GPL.
That copyleft is confined to cpp/obs: it's a separately-distributable work, and
per GPLv2 its presence here is mere aggregation that doesn't affect the
MIT/Apache licensing of the moq crates (libmoq stays permissive).

Make that legible to humans and license scanners:
- SPDX-License-Identifier: GPL-2.0-or-later on every cpp/obs source file.
- A licensing exception note in the root README.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Build/CMake fixes (macOS build re-verified end to end):
- CMakeLists: MOQ_LOCAL now defaults to the in-tree repo root (two levels up)
  with an EXISTS guard and a quoted add_subdirectory path, so a bare
  `cmake --preset` also links in-tree libmoq, matching the docs. Falls back to
  the release download only when rs/libmoq is absent.
- buildspec_common: drop single quotes from the -D args passed via
  execute_process (no shell, so they were passed literally), un-corrupting
  CMAKE_OSX_ARCHITECTURES and CMAKE_PREFIX_PATH for the OBS-sources sub-build.
- osconfig: set CMAKE_C_EXTENSIONS FALSE on Linux/BSD to match macOS/Windows.
- macos/defaults: initialize CODESIGN_IDENTITY independently of CODESIGN_TEAM.
- macos/helpers: fix dSYM bundle casing (.dsym -> .dSYM).
- windows/helpers: attach the generated .rc to the function's ${target}.
- ccache launchers: POSIX `[ ]` tests (scripts are #!/bin/sh), and use
  CMAKE_CXX_COMPILER_ID in the C++ launcher (was a C copy-paste).
- CMakePresets: macOS preset labels say arm64, not "Universal" (arch is arm64).
- justfile: correct the build recipe comment (it does not auto-run setup).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`just obs setup` (module invocation from the repo root) resolves
`justfile_directory()` to the ROOT justfile's directory, not cpp/obs, so
`justfile_directory()/../..` overshot the repo root and MOQ_LOCAL pointed
outside the tree. CMake then fell back to FetchContent and downloaded the
x86_64 libmoq release, which fails to link on an aarch64 host
("Relocations in generic ELF (EM: 62) ... file in wrong format").

Drop the path computation and let CMakeLists.txt default MOQ_LOCAL to
${CMAKE_CURRENT_SOURCE_DIR}/../.. (reliable regardless of how the recipe is
invoked). `setup` only passes -DMOQ_LOCAL when given an explicit override.

Verified on an aarch64 Linux + Nix host: builds the in-tree rs/libmoq and
links obs-moq.so against nix obs-studio/Qt6/ffmpeg.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirrors libmoq.yml / moq-gst.yml: pushing an `obs-moq-v*` tag builds the
plugin per platform and attaches archives to a GitHub release via the shared
.github/scripts/release.sh helper.

- cpp/obs/build.sh: configures + builds via the platform CMake preset and
  packages the result. macOS ships the .plugin bundle (zip); Linux/Windows
  ship the OBS portable layout obs-moq/bin/64bit/<lib> + data (tar.gz / zip).
  Uses `cmake -E tar` so no zip/gtar dependency on the Windows runner.
- .github/workflows/obs.yml: matrix of linux x86_64/aarch64 (Nix devshell),
  macOS arm64 and Windows x64 (native Xcode/VS + buildspec, no Nix).

Archives are unsigned (no signing secrets); release notes/docs say so.

Verified end to end: macOS arm64 (this build) and Linux aarch64 (OCI A1 via
nix develop) produce loadable archives. Windows leg is written to the same
pattern but not yet exercised by a real tag.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Windows build was documented but never run. Two CMake gaps blocked it:

- FFmpeg discovery used pkg_check_modules, but the Windows obs-deps ship
  FFmpeg without pkg-config .pc files. Use find_package(FFmpeg) on Windows
  via a vendored copy of OBS's FindFFmpeg finder, which searches the
  obs-deps prefix directly. pkg-config stays the path on macOS/Linux.
- libmoq's static lib did not propagate the Windows system libraries Rust
  std pulls in (ntdll, userenv, ws2_32, bcrypt, dbghelp, advapi32,
  legacy_stdio_definitions), so linking moq.lib into the C++ target failed
  with unresolved __imp_NtCreateNamedPipeFile.

Verified: builds with VS 2022 Build Tools + Windows SDK 10.0.22621 and
loads into a stock OBS Studio install.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The OBS plugin statically links libmoq, so its meaningful version is the
libmoq version and it should ship with every libmoq release rather than on a
separate manual tag that drifts out of date.

Fold the plugin build into libmoq.yml: after the libmoq `release` job
publishes the per-target archives, `obs-build` rebuilds the plugin against
that just-published release (CMake's FetchContent path) and `obs-release`
cuts a parallel obs-moq-v<version> release. This reuses the exact libmoq.a
that was released (guaranteed-matching ABI), needs no second cargo build,
and drops the rust toolchain requirement from the macOS/Windows plugin jobs.

- cpp/obs/build.sh: add --libmoq-release VERSION (forces CMake's release
  download via empty MOQ_LOCAL + MOQ_VERSION/MOQ_TARGET; versions the plugin
  to match). Default/local builds still use the in-tree rs/libmoq.
- Remove the standalone obs.yml; obs-build/obs-release live in libmoq.yml.

Verified: build.sh --libmoq-release 0.3.6 on macOS arm64 fetches the
published libmoq release and links it with no rust in the environment
(in-tree libmoq is 0.3.6, so the C ABI matches).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The plugin's compiled-in version came from buildspec.json's placeholder
0.0.1. Since it statically links libmoq and now releases per libmoq version,
stamp that version into the plugin metadata too.

- bootstrap.cmake: honor a PLUGIN_VERSION_OVERRIDE cache var (applied before
  PLUGIN_VERSION_* are derived, so project version, macOS Info.plist and the
  Windows resource all pick it up).
- build.sh: pass -DPLUGIN_VERSION_OVERRIDE=$VERSION. In --libmoq-release mode
  that's the libmoq version. Also fix the buildspec version auto-default,
  which matched the first "version" (nested obs-studio dep) instead of the
  top-level plugin version.

Verified on macOS arm64: --libmoq-release 0.3.6 yields a .plugin with
CFBundleShortVersionString=0.3.6.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…iew)

- libmoq.yml obs-release: the prev-tag step was missing, and release.sh
  prev-tag can't help (the trigger is a libmoq-v* tag, not obs-moq-v*), so
  release notes would span from the libmoq tag. Compute the newest existing
  obs-moq tag inline and pass it as RELEASE_PREV_TAG.
- Align license to GPL-2.0-or-later (matches the SPDX headers + root README)
  in cpp/obs/README.md and CLAUDE.md.
- flake.nix: fix stale comment path obs/ -> cpp/obs/.

Co-Authored-By: Claude Opus 4.8 <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.

1 participant