Skip to content

Cmake: Cmake ffmpeg library fix for Windows#31

Draft
danrossi wants to merge 12 commits intomoq-dev:mainfrom
danrossi:win-build-fix
Draft

Cmake: Cmake ffmpeg library fix for Windows#31
danrossi wants to merge 12 commits intomoq-dev:mainfrom
danrossi:win-build-fix

Conversation

@danrossi
Copy link
Copy Markdown

Thanks for doing this plugin. Yet to begin testing it. Will this work perhaps with Cloudflare Moq relay ? I have set it up on Cloudflare for testing.

I found the Windows build broken because pkg-config is resolving system libraries. It took me a few days to understand how cmake works as its really busy. This change uses the pre-built libraries already downloaded for Windows. I found a method to automatically search the destination directory as the variables dont get set for it. It needed two extra Windows libraries. The spec downloads the latest version.

I've created a windows specific build setup doc including environment tools setup. It took a while to figure that out to get obs built first. I am using VS2026 build tools so documented that setup.

There is a way to program installation into the obs fork in the windows specific cmake files but haven't figured that out yet. The location is

../obs-studio/build_x64/rundir/RelWithDebInfo/obs-plugins

I have an obs fork where I've merged from the upstream branch and shows a menu for now.

https://github.com/danrossi/obs-studio/tree/moq-merged

Screenshot 2026-04-25 040008
  • Add windows environment setup and build instructions
  • Update build specs to the latest obs-deps versions
  • Change library resolving for windows to use the pre built ffmpeg libaries
  • Add extra required windows libraries needed

- Add windows environment setup and build instructions
- Update build specs to the latest obs-deps versions
- Change library resolving for windows to use the pre built ffmpeg libaries
- Add extra required windows libraries needed
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Walkthrough

CMake changes make FFmpeg discovery conditional: when BUILD_PLUGIN is true, Linux retains pkg_check_modules/${FFMPEG_*} usage while non-Linux hosts use per-library find_library for avcodec, avutil, swscale, and swresample (linking only if avcodec is found); on Windows the target also links Userenv and ntdll. A new WINDOWS.md provides Windows build and debug instructions. buildspec.json bumps obs-studio to 32.1.2 and updates prebuilt/qt6 versions and hashes. macOS logic patches libobs-metal/CMakeLists.txt for Swift (<= 32.1.2) and raises minimum SDK/Xcode requirements to 26.1.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing FFmpeg library resolution for Windows in CMake by switching from pkg-config to pre-built libraries.
Description check ✅ Passed The description is directly related to the changeset, explaining the Windows FFmpeg build issue and the overall changes including dependency updates, build docs, and platform-specific fixes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
CMakeLists.txt (1)

25-25: Confirm Userenv/ntdll are needed for the obs-moq target directly.

These are almost certainly pulled in transitively by libmoq (Rust std). Linking them against obs-moq is harmless, but if you can confirm they come from the Rust static lib, it may be cleaner to expose them as INTERFACE link libs from the moq target instead of the plugin. Low priority.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CMakeLists.txt` at line 25, The CMake line linking Userenv and ntdll directly
into the obs-moq target may be redundant because those libs are likely pulled in
by the Rust static library (libmoq); confirm whether the transitive dependency
originates from the moq target (or libmoq) and if so move Userenv and ntdll to
the moq target as INTERFACE link libraries instead of linking them directly to
obs-moq; check the CMake target names target_link_libraries(obs-moq ...), the
moq target, and any Rust-produced static lib exports to decide whether to change
the linkage to target_link_libraries(moq INTERFACE Userenv ntdll) and remove
them from obs-moq.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CMakeLists.txt`:
- Around line 27-32: The find_library invocations (e.g., the calls that set
AVCODEC_LIB, AVUTIL_LIB, SWSCALE_LIB, SWRESAMPLE_LIB) are missing REQUIRED and
search hints so a missing library becomes <VAR>-NOTFOUND later; update each
find_library to use REQUIRED and provide sensible PATHS/HINTS (for example
system/library install prefixes or a build/install output directory used by your
presets) so CMake fails at configure time with a clear error if a library is
absent and so discovery falls back to the intended destination directory when
CMAKE_PREFIX_PATH isn’t set; keep target_link_libraries(obs-moq PRIVATE
${AVCODEC_LIB} ...) unchanged since it will then receive only valid library
results.

In `@WINDOWS.md`:
- Around line 5-11: The fenced code block in WINDOWS.md opens with two backticks
instead of three and lacks a language tag, causing rendering and MD040 lint
failures; change the opening fence to ```powershell and close with ```; also
make the Chocolatey installs non-interactive and consistent by adding -y to the
visualstudio2026buildtools line and replace the rustup install with the correct
choco package (rustup.install) and -y flag so the lines to update are the fenced
block around the PowerShell commands and the choco install invocations.
- Around line 33-37: Update the code fence in WINDOWS.md that contains the
Copy-Item example to include the powershell language identifier; replace the
current triple-backtick block around the Copy-Item command with an opening
"```powershell" and a closing "```" so the code block is annotated for MD040 and
tools that rely on language highlighting.
- Around line 19-23: The fenced code blocks containing the Windows shell
commands (the block with "cd obs-studio" and the cmake invocations including -G
"Visual Studio 18 2026" -A x64 --preset windows-x64 and cmake --build --preset
windows-x64) need explicit language identifiers (e.g., powershell or bat) to
satisfy MD040; update each of those code fences to include the appropriate
language tag so the snippets are properly highlighted while leaving the existing
cmake flags and preset usage unchanged.

---

Nitpick comments:
In `@CMakeLists.txt`:
- Line 25: The CMake line linking Userenv and ntdll directly into the obs-moq
target may be redundant because those libs are likely pulled in by the Rust
static library (libmoq); confirm whether the transitive dependency originates
from the moq target (or libmoq) and if so move Userenv and ntdll to the moq
target as INTERFACE link libraries instead of linking them directly to obs-moq;
check the CMake target names target_link_libraries(obs-moq ...), the moq target,
and any Rust-produced static lib exports to decide whether to change the linkage
to target_link_libraries(moq INTERFACE Userenv ntdll) and remove them from
obs-moq.
🪄 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: acb0e0df-f4d3-4c00-bad3-2298ac4cd646

📥 Commits

Reviewing files that changed from the base of the PR and between 8776f8e and b83a5c7.

📒 Files selected for processing (3)
  • CMakeLists.txt
  • WINDOWS.md
  • buildspec.json

Comment thread CMakeLists.txt Outdated
Comment thread WINDOWS.md Outdated
Comment thread WINDOWS.md Outdated
Comment thread WINDOWS.md Outdated
- add check for ffmpeg library path
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
CMakeLists.txt (1)

27-34: ⚠️ Potential issue | 🟠 Major

find_library calls still lack REQUIRED/search hints, and the PR-promised destination-directory fallback is missing.

The previously raised concerns still apply to this revision:

  • None of the four find_library calls use REQUIRED, so a missing component yields <VAR>-NOTFOUND. The if(AVCODEC_LIB) guard at line 32 only papers over one of the four — if avcodec is found but any of avutil/swscale/swresample is not, those NOTFOUND values still flow into target_link_libraries, producing a confusing linker error rather than a clean configure-time failure. Given that src/moq-source.cpp calls avcodec_find_decoder, avcodec_alloc_context3, sws_getContext, sws_scale, etc., none of these libraries are optional — silently skipping linkage is unsafe.
  • No PATHS/HINTS are passed, so discovery depends entirely on the caller's CMAKE_PREFIX_PATH. The PR description says this PR adds "a method to automatically search the destination directory when CMake variables are not set", but I don't see any such fallback in the diff — please confirm whether that piece was intended to land here.
🛠️ Suggested fix
-    find_library(AVCODEC_LIB avcodec)
-    find_library(AVUTIL_LIB avutil)
-    find_library(SWSCALE_LIB swscale)
-    find_library(SWRESAMPLE_LIB swresample)
-
-    if (AVCODEC_LIB)
-      target_link_libraries(obs-moq PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB})
-    endif()
+    find_library(AVCODEC_LIB avcodec REQUIRED)
+    find_library(AVUTIL_LIB avutil REQUIRED)
+    find_library(SWSCALE_LIB swscale REQUIRED)
+    find_library(SWRESAMPLE_LIB swresample REQUIRED)
+
+    target_link_libraries(
+      obs-moq
+      PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB}
+    )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CMakeLists.txt` around lines 27 - 34, The find_library invocations for
avcodec/avutil/swscale/swresample must fail at configure-time and search the
build/install destination if variables are unset: change the four find_library
calls (AVCODEC_LIB, AVUTIL_LIB, SWSCALE_LIB, SWRESAMPLE_LIB) to use REQUIRED and
add appropriate HINTS/PATHS that include the destination-directory fallback
(e.g., the project's install or output dirs or a CMake variable like
DESTDIR/CMAKE_INSTALL_PREFIX) so CMake will search those locations; then remove
the weak if(AVCODEC_LIB) guard and instead perform a single configure-time check
(or let REQUIRED enforce it) before calling target_link_libraries(obs-moq
PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB}) so no
NOTFOUND values are propagated into the linker step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@CMakeLists.txt`:
- Around line 27-34: The find_library invocations for
avcodec/avutil/swscale/swresample must fail at configure-time and search the
build/install destination if variables are unset: change the four find_library
calls (AVCODEC_LIB, AVUTIL_LIB, SWSCALE_LIB, SWRESAMPLE_LIB) to use REQUIRED and
add appropriate HINTS/PATHS that include the destination-directory fallback
(e.g., the project's install or output dirs or a CMake variable like
DESTDIR/CMAKE_INSTALL_PREFIX) so CMake will search those locations; then remove
the weak if(AVCODEC_LIB) guard and instead perform a single configure-time check
(or let REQUIRED enforce it) before calling target_link_libraries(obs-moq
PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB}) so no
NOTFOUND values are propagated into the linker step.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad8e0cea-ddad-46b3-9578-066f9ed56b93

📥 Commits

Reviewing files that changed from the base of the PR and between b83a5c7 and ea1c0a9.

📒 Files selected for processing (2)
  • CMakeLists.txt
  • WINDOWS.md
✅ Files skipped from review due to trivial changes (1)
  • WINDOWS.md

@danrossi
Copy link
Copy Markdown
Author

I'm not sure why the bot spelt out what I just did.

There is no "REQUIRED" for find_library so a mistake. A path check has been added for the first lib. The path variables is baked into the deps already downloaded. Same with the include path. pkg-config could have worked if there was pkg-config files in the deps. It's probably better to use what is downloaded in the built in deps than system files.

I'm not sure what this means it's an error

"only linking to the plugin if avcodec is found"

The pre-built deps are baked and already there so it will find all of them.

target_link_libraries(obs-moq PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB})

I'll test a publish to the local relay before trying other relays inc cloudflare.

@kixelated
Copy link
Copy Markdown
Collaborator

Oh sorry I forgot about this PR. I'll take a look later today.

@danrossi
Copy link
Copy Markdown
Author

I don't see an issue with my build but the plugin won't connect to either the relay in WSL or in Powershell. I was able to build a relay exe in powershell then run it directly not requiring to rebuild everytime. It consumes alot of memory and cpu when rebuilding to launch the relay.

$env:RUST_LOG="debug"; $env:RUST_BACKTRACE=1; moq\target\debug\moq-relay.exe localhost.toml

There is no way to get a rust trace log launching obs in powershell it closes stdout after launching obs. There is just the obs logger. I tried this

$env:RUST_LOG="debug"; $env:RUST_BACKTRACE=1; $env:OBS_LOG_LEVEL="debug"; .\obs64.exe --verbose

The callback error code for the connection is -5 which might need to return a string instead from the error code ?. I see no activity in the relay of a connection. And nothing in wireshark shows up. Publishing works running the demo so the rust publisher is working. I will try and run that command separate.

03:47:40.572: [obs-moq] Connecting to MoQ server: http://localhost:4443/anon
03:47:40.573: [obs-moq] Publishing broadcast: obs
03:47:40.575: Maximum interleaver batch size for 'simple_stream' calculated to be 5 packets
03:47:40.580: [obs-moq] MoQ session closed (-5): http://localhost:4443/anon

@danrossi
Copy link
Copy Markdown
Author

The web and moq-cli publish is working. To both the linux and windows relay. OBS isn't connecting for some reason and hard to debug still. I've got a mac mini to test mac.

I'll test if the rust publisher works with cloudflare not just the rust relay. I wanted to test that with OBS also.

←[2m2026-04-30T03:31:50.947800Z←[0m ←[32m INFO←[0m ←[1mconn←[0m←[1m{←[0m←[3mid←[0m←[2m=←[0m0←[1m}←[0m←[2m:←[0m ←[2mmoq_relay::connection←[0m←[2m:←[0m session accepted ←[3mtransport←[0m←[2m=←[0m"quic" ←[3mroot←[0m←[2m=←[0manon ←[3mpublish←[0m←[2m=←[0m ←[3msubscribe←[0m←[2m=←[0m
←[2m2026-04-30T03:31:50.948977Z←[0m ←[32m INFO←[0m ←[1mconn←[0m←[1m{←[0m←[3mid←[0m←[2m=←[0m0←[1m}←[0m←[2m:←[0m ←[2mmoq_relay::connection←[0m←[2m:←[0m negotiated ←[3mversion←[0m←[2m=←[0mmoq-lite-04 ←[3mtransport←[0m←[2m=←[0m"quic"

@danrossi
Copy link
Copy Markdown
Author

danrossi commented May 1, 2026

The build instructions for obs-studio has changed for macos. They require to build after config in xcode and there is a code error with a trailing comma in a shader I now have to fix for them to get it built to test the plugin on mac. Will figure it out. the build instructions for mac may need to change.

https://github.com/obsproject/obs-studio/wiki/build-instructions-for-mac

@danrossi
Copy link
Copy Markdown
Author

danrossi commented May 1, 2026

I have a functioning branch here with moq and some fixes for mac with the current master obs branch.

https://github.com/danrossi/obs-studio/tree/moq-merged

mac build of obs is

 git clone --recurse-submodules https://github.com/brianmed/obs-studio.git
cd obs-studio
 cmake --preset macos
 cd build_macos
  xcodebuild                                                 \
  -configuration RelWithDebInfo \
  -scheme obs-studio                                       \
  -parallelizeTargets                                      \
  -destination "generic/platform=macOS,name=Any Mac"

The mac build of the plugin however also suffers the same as windows looking for pkgconfig which is not installed by default. It may be trying to search for system files rather the included deps. I'll try and fix for mac also if needed.

cmake --preset macos -DMOQ_LOCAL="../moq" 
Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE)

@danrossi danrossi marked this pull request as draft May 1, 2026 18:01
@danrossi
Copy link
Copy Markdown
Author

danrossi commented May 1, 2026

Installing pkg-config on mac it's a tool check for system ffmpeg libraries. I'll make a proposal change to use the included dependencies like Windows for mac and Linux. To follow the obs-studio build ? I've converted to draft.

 The following required packages were not found:

   - libavcodec
   - libavutil
   - libswscale
   - libswresample

@danrossi danrossi marked this pull request as ready for review May 2, 2026 11:09
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

135-139: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

"Supported Build Environments" table is stale — still shows VS 17 2022 and Xcode 16.0.

After this PR's changes the minimum supported toolchain is VS 18 2026 (CMake 4.2+) on Windows and Xcode 16.3 on macOS, but the table retains the old values.

📝 Suggested fix
 | Platform  | Tool   |
 |-----------|--------|
-| Windows   | Visual Studio 17 2022 |
-| macOS     | Xcode 16.0 |
-| Windows, macOS  | CMake 3.30.5 |
+| Windows   | Visual Studio 18 2026 |
+| macOS     | Xcode 16.3 |
+| Windows   | CMake 4.2+ |
+| macOS     | CMake 3.30.5 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 135 - 139, Update the "Supported Build Environments"
table in README.md to reflect the new minimum toolchains: change the Windows
entry from "Visual Studio 17 2022" to "Visual Studio 18 2026", update the macOS
entry from "Xcode 16.0" to "Xcode 16.3", and change the CMake entry from "CMake
3.30.5" to "CMake 4.2+" so the table under the "Supported Build Environments"
heading matches the PR changes.
♻️ Duplicate comments (1)
CMakeLists.txt (1)

19-26: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

if (AVCODEC_LIB) guard only protects one of four libraries — the other three can still be *-NOTFOUND.

When AVCODEC_LIB is found but any of AVUTIL_LIB, SWSCALE_LIB, or SWRESAMPLE_LIB are not found, their *-NOTFOUND sentinel strings are silently forwarded to target_link_libraries, producing a confusing linker error instead of a clear configure-time failure. Adding REQUIRED to all four find_library calls (along with HINTS from CMAKE_PREFIX_PATH) is the idiomatic fix and eliminates the need for the manual guard entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CMakeLists.txt` around lines 19 - 26, The current CMake snippet uses
find_library for AVCODEC_LIB, AVUTIL_LIB, SWSCALE_LIB, and SWRESAMPLE_LIB but
only guards target_link_libraries with if(AVCODEC_LIB), allowing other variables
to be `*-NOTFOUND`; change each find_library(...) call to include REQUIRED and
appropriate HINTS using ${CMAKE_PREFIX_PATH} (e.g. find_library(AVCODEC_LIB
NAMES avcodec REQUIRED HINTS ${CMAKE_PREFIX_PATH})), do the same for AVUTIL_LIB,
SWSCALE_LIB, and SWRESAMPLE_LIB, and then remove the manual if(AVCODEC_LIB)
guard so target_link_libraries(obs-moq PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB}
${SWSCALE_LIB} ${SWRESAMPLE_LIB}) is always passed only after configure-time
failures if any library is missing.
🧹 Nitpick comments (1)
CMakeLists.txt (1)

24-24: 💤 Low value

Run ./build-aux/run-gersemiif (AVCODEC_LIB) has a space before ( inconsistent with if(OS_WINDOWS) on Line 29.

As per coding guidelines, CMake files should be formatted using ./build-aux/run-gersemi.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CMakeLists.txt` at line 24, The CMake conditional "if (AVCODEC_LIB)" has an
extra space before the paren and should match the project's style (e.g.
"if(OS_WINDOWS)"); update the token to remove the space so it reads consistently
with other conditionals and then run the formatter tool by executing
./build-aux/run-gersemi to reformat the CMakeLists; look for the "if
(AVCODEC_LIB)" occurrence and update it to the no-space form used by
"if(OS_WINDOWS)" and commit the formatted file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmake/common/buildspec_common.cmake`:
- Around line 65-74: Add an existence guard before calling file(READ) for
"${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt" so CMake
won't fatal-error when the file is missing; keep the surrounding if(_obs_version
VERSION_LESS_EQUAL "32.1.2") block. Update the string(REGEX REPLACE) pattern
used to insert enable_language(Swift) to escape dots in the version tokens
(e.g., use "3\\.28...3\\.30") so it matches literal dots, and after computing
NEW_CONTENTS compare it to CONTENTS and emit a message(WARNING ...) if they are
identical (indicating the replace did nothing) rather than silently proceeding;
leave the message(STATUS "Patching libobs-metal") and file(WRITE ...) logic
unchanged but only run them when the file exists and NEW_CONTENTS differs from
CONTENTS.

In `@README.md`:
- Line 21: Update the README macOS prerequisite to match the enforced SDK
version in cmake/macos/compilerconfig.cmake: change the listed "macOS 15" to
"macOS 15.4" (and if present, any combined entry like "XCode 16.3 / macOS 15" to
"XCode 16.3 / macOS 15.4") so it aligns with the obs_macos_minimum_sdk = 15.4
setting and prevents users on 15.0–15.3 from encountering a configure-time fatal
error.

---

Outside diff comments:
In `@README.md`:
- Around line 135-139: Update the "Supported Build Environments" table in
README.md to reflect the new minimum toolchains: change the Windows entry from
"Visual Studio 17 2022" to "Visual Studio 18 2026", update the macOS entry from
"Xcode 16.0" to "Xcode 16.3", and change the CMake entry from "CMake 3.30.5" to
"CMake 4.2+" so the table under the "Supported Build Environments" heading
matches the PR changes.

---

Duplicate comments:
In `@CMakeLists.txt`:
- Around line 19-26: The current CMake snippet uses find_library for
AVCODEC_LIB, AVUTIL_LIB, SWSCALE_LIB, and SWRESAMPLE_LIB but only guards
target_link_libraries with if(AVCODEC_LIB), allowing other variables to be
`*-NOTFOUND`; change each find_library(...) call to include REQUIRED and
appropriate HINTS using ${CMAKE_PREFIX_PATH} (e.g. find_library(AVCODEC_LIB
NAMES avcodec REQUIRED HINTS ${CMAKE_PREFIX_PATH})), do the same for AVUTIL_LIB,
SWSCALE_LIB, and SWRESAMPLE_LIB, and then remove the manual if(AVCODEC_LIB)
guard so target_link_libraries(obs-moq PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB}
${SWSCALE_LIB} ${SWRESAMPLE_LIB}) is always passed only after configure-time
failures if any library is missing.

---

Nitpick comments:
In `@CMakeLists.txt`:
- Line 24: The CMake conditional "if (AVCODEC_LIB)" has an extra space before
the paren and should match the project's style (e.g. "if(OS_WINDOWS)"); update
the token to remove the space so it reads consistently with other conditionals
and then run the formatter tool by executing ./build-aux/run-gersemi to reformat
the CMakeLists; look for the "if (AVCODEC_LIB)" occurrence and update it to the
no-space form used by "if(OS_WINDOWS)" and commit the formatted file.
🪄 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: 568d4fc6-66f5-4e81-a869-30f2ae368f29

📥 Commits

Reviewing files that changed from the base of the PR and between ea1c0a9 and d86e382.

📒 Files selected for processing (6)
  • CMakeLists.txt
  • README.md
  • WINDOWS.md
  • buildspec.json
  • cmake/common/buildspec_common.cmake
  • cmake/macos/compilerconfig.cmake
✅ Files skipped from review due to trivial changes (1)
  • buildspec.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • WINDOWS.md

Comment on lines +65 to +74
if (_obs_version VERSION_LESS_EQUAL "32.1.2")

file(READ "${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt" CONTENTS)

#check if Swift is not available
if(NOT "${CONTENTS}" MATCHES "enable_language\\(Swift\\)")
string(REGEX REPLACE "cmake_minimum_required\\(VERSION 3.28...3.30\\)" "cmake_minimum_required(VERSION 3.28...3.30)\nenable_language(Swift)" NEW_CONTENTS "${CONTENTS}")
message(STATUS "Patching libobs-metal")
file(WRITE "${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt" "${NEW_CONTENTS}")
endif()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add if(EXISTS ...) guard before file(READ ...) — fatal error if the path is absent.

file(READ ...) on Line 67 will abort CMake configure with an unhelpful fatal error if ${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt does not exist (e.g., partial extraction, unexpected OBS source layout, or obs-studio dependency not yet present). Add an existence check first.

🛠️ Proposed fix
-    `#Patch` libobs-metal if required
-    if (_obs_version VERSION_LESS_EQUAL "32.1.2")
-
-      file(READ "${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt" CONTENTS)
-
-      `#check` if Swift is not available
-      if(NOT "${CONTENTS}" MATCHES "enable_language\\(Swift\\)")
-        string(REGEX REPLACE "cmake_minimum_required\\(VERSION 3.28...3.30\\)" "cmake_minimum_required(VERSION 3.28...3.30)\nenable_language(Swift)" NEW_CONTENTS "${CONTENTS}")
-        message(STATUS "Patching libobs-metal")
-        file(WRITE "${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt" "${NEW_CONTENTS}")
-      endif()
-
-    endif()
+    # Patch libobs-metal if required
+    if(_obs_version VERSION_LESS_EQUAL "32.1.2")
+      set(_libobs_metal_cmake "${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt")
+      if(EXISTS "${_libobs_metal_cmake}")
+        file(READ "${_libobs_metal_cmake}" CONTENTS)
+        # Check if Swift is not already enabled
+        if(NOT "${CONTENTS}" MATCHES "enable_language\\(Swift\\)")
+          string(
+            REGEX REPLACE
+            "cmake_minimum_required\\(VERSION 3\\.28\\.\\.\\. ?3\\.30\\)"
+            "cmake_minimum_required(VERSION 3.28...3.30)\nenable_language(Swift)"
+            NEW_CONTENTS
+            "${CONTENTS}"
+          )
+          if(NOT NEW_CONTENTS STREQUAL CONTENTS)
+            message(STATUS "Patching libobs-metal")
+            file(WRITE "${_libobs_metal_cmake}" "${NEW_CONTENTS}")
+          else()
+            message(WARNING "libobs-metal patch: regex did not match; Swift may not be enabled")
+          endif()
+        endif()
+      else()
+        message(WARNING "libobs-metal/CMakeLists.txt not found at ${_libobs_metal_cmake}; skipping Swift patch")
+      endif()
+    endif()

The proposed fix also escapes the . characters in the version range regex (e.g., 3\\.28) so they match literal dots rather than any character, and adds a guard to warn if the string(REGEX REPLACE) produces no change — which would leave Swift silently un-enabled even though the enable_language(Swift) check triggered.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmake/common/buildspec_common.cmake` around lines 65 - 74, Add an existence
guard before calling file(READ) for
"${dependencies_dir}/${_obs_destination}/libobs-metal/CMakeLists.txt" so CMake
won't fatal-error when the file is missing; keep the surrounding if(_obs_version
VERSION_LESS_EQUAL "32.1.2") block. Update the string(REGEX REPLACE) pattern
used to insert enable_language(Swift) to escape dots in the version tokens
(e.g., use "3\\.28...3\\.30") so it matches literal dots, and after computing
NEW_CONTENTS compare it to CONTENTS and emit a message(WARNING ...) if they are
identical (indicating the replace did nothing) rather than silently proceeding;
leave the message(STATUS "Patching libobs-metal") and file(WRITE ...) logic
unchanged but only run them when the file exists and NEW_CONTENTS differs from
CONTENTS.

Comment thread README.md
* C++ Compiler (Clang/GCC/MSVC)
* OBS Studio development libraries (libobs)
* [Fork of OBS-Studio](https://github.com/brianmed/obs-studio) just to show MoQ in the UI.
* XCode 16.3 / macOS 15
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

macOS 15 prerequisite understates the actual minimum — cmake/macos/compilerconfig.cmake enforces SDK 15.4.

The prerequisite on Line 21 reads macOS 15, but the updated compilerconfig.cmake sets obs_macos_minimum_sdk to 15.4. Users on macOS 15.0–15.3 will hit a configure-time fatal error. Change to macOS 15.4.

📝 Suggested fix
-*   XCode 16.3 / macOS 15
+*   Xcode 16.3 / macOS 15.4 SDK
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* XCode 16.3 / macOS 15
* Xcode 16.3 / macOS 15.4 SDK
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 21, Update the README macOS prerequisite to match the
enforced SDK version in cmake/macos/compilerconfig.cmake: change the listed
"macOS 15" to "macOS 15.4" (and if present, any combined entry like "XCode 16.3
/ macOS 15" to "XCode 16.3 / macOS 15.4") so it aligns with the
obs_macos_minimum_sdk = 15.4 setting and prevents users on 15.0–15.3 from
encountering a configure-time fatal error.

@danrossi
Copy link
Copy Markdown
Author

danrossi commented May 2, 2026

There was requirements to get the mac build working. The changes required is to use the pre built libraries provided. I'll test Linux I may need to revert back to pkg-config for Linux.

mac build requires Xcode 16.3 for both obs-studio and the obs-moq plugin. The build requires to patch the obs source archive to configure correctly until a change is made to the source archive. It checks for that.

I've updated the obs-studio build docs for both Windows and mac. The repo for obs-studio won't have the libobs-metal fix. Which I have in this merged branch

https://github.com/danrossi/obs-studio/tree/moq-merged

I can test with debug flags on mac the connection issue to the relay.

Screenshot 2026-05-02 210831

@danrossi
Copy link
Copy Markdown
Author

danrossi commented May 2, 2026

On mac I see it connecting to these interfaces when using localhost. So the mac OBS client is working to the localhost relay for now. Not Windows, I'll see if there is a way to get a trace on Windows. I have IPv6 setup on the interface. It might need to bind to 0.0.0.0

2026-05-02T12:20:33.795671Z DEBUG reqwest::connect: starting new connection: http://localhost:4443/
2026-05-02T12:20:33.795969Z DEBUG hyper_util::client::legacy::connect::http: connecting to [::1]:4443
2026-05-02T12:20:33.796204Z DEBUG hyper_util::client::legacy::connect::http: connected to [::1]:4443
2026-05-02T12:20:33.801015Z DEBUG hyper_util::client::legacy::pool: pooling idle connection for ("http", localhost:4443)
2026-05-02T12:20:33.801794Z DEBUG moq_native::quinn: connecting url=https://localhost:4443/anon ip=[::1]:4443
2026-05-02T12:20:33.802554Z DEBUG rustls::client::hs: No cached session for DnsName("localhost")

A connection issue might need to trigger an error dialog window.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
CMakeLists.txt (1)

28-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast for all FFmpeg libs instead of gating on AVCODEC_LIB only.

Line 33 currently checks only AVCODEC_LIB, while Line 34 links four libraries. If any of the other three are missing, failure is delayed/confusing. Use REQUIRED on all find_library calls and remove the partial gate so configure fails clearly.

#!/bin/bash
set -euo pipefail

echo "FFmpeg find_library calls:"
rg -nP 'find_library\((AVCODEC_LIB|AVUTIL_LIB|SWSCALE_LIB|SWRESAMPLE_LIB)\b' CMakeLists.txt

echo
echo "Which of those use REQUIRED:"
rg -nP 'find_library\((AVCODEC_LIB|AVUTIL_LIB|SWSCALE_LIB|SWRESAMPLE_LIB)\b[^\)]*REQUIRED' CMakeLists.txt || true

echo
echo "Current link gate and link line:"
rg -nP 'if\s*\(\s*AVCODEC_LIB\s*\)|target_link_libraries\(obs-moq PRIVATE \$\{AVCODEC_LIB\}' CMakeLists.txt
Suggested patch
-    find_library(AVCODEC_LIB avcodec)
-    find_library(AVUTIL_LIB avutil)
-    find_library(SWSCALE_LIB swscale)
-    find_library(SWRESAMPLE_LIB swresample)
-
-    if (AVCODEC_LIB)
-      target_link_libraries(obs-moq PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB})
-    endif()
+    find_library(AVCODEC_LIB avcodec REQUIRED)
+    find_library(AVUTIL_LIB avutil REQUIRED)
+    find_library(SWSCALE_LIB swscale REQUIRED)
+    find_library(SWRESAMPLE_LIB swresample REQUIRED)
+    target_link_libraries(obs-moq PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CMakeLists.txt` around lines 28 - 35, The CMake snippet currently only gates
linking on AVCODEC_LIB but links AVCODEC_LIB, AVUTIL_LIB, SWSCALE_LIB, and
SWRESAMPLE_LIB; make all four find_library calls fail-fast by adding REQUIRED to
each find_library(AVCODEC_LIB ...), find_library(AVUTIL_LIB ...),
find_library(SWSCALE_LIB ...), and find_library(SWRESAMPLE_LIB ...) and remove
the conditional if (AVCODEC_LIB) ... endif() so target_link_libraries(obs-moq
PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB}) always
runs (configuration will now error if any FFmpeg lib is missing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmake/macos/compilerconfig.cmake`:
- Around line 19-20: The macOS minimums are set too high; update the CMake
variables obs_macos_minimum_sdk and obs_macos_minimum_xcode in
compilerconfig.cmake to match upstream OBS baselines by setting
obs_macos_minimum_sdk to 15.0 and obs_macos_minimum_xcode to 16.0 so the
FATAL_ERROR checks use the correct, less restrictive minima.

---

Duplicate comments:
In `@CMakeLists.txt`:
- Around line 28-35: The CMake snippet currently only gates linking on
AVCODEC_LIB but links AVCODEC_LIB, AVUTIL_LIB, SWSCALE_LIB, and SWRESAMPLE_LIB;
make all four find_library calls fail-fast by adding REQUIRED to each
find_library(AVCODEC_LIB ...), find_library(AVUTIL_LIB ...),
find_library(SWSCALE_LIB ...), and find_library(SWRESAMPLE_LIB ...) and remove
the conditional if (AVCODEC_LIB) ... endif() so target_link_libraries(obs-moq
PRIVATE ${AVCODEC_LIB} ${AVUTIL_LIB} ${SWSCALE_LIB} ${SWRESAMPLE_LIB}) always
runs (configuration will now error if any FFmpeg lib is missing).
🪄 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: 0c66aab1-2d4f-458a-8db4-8c88ff7ab59d

📥 Commits

Reviewing files that changed from the base of the PR and between d86e382 and 9d45afd.

📒 Files selected for processing (3)
  • CMakeLists.txt
  • buildspec.json
  • cmake/macos/compilerconfig.cmake
✅ Files skipped from review due to trivial changes (1)
  • buildspec.json

Comment on lines +19 to +20
set(obs_macos_minimum_sdk 26.1) # Keep in sync with Xcode
set(obs_macos_minimum_xcode 26.1) # Keep in sync with SDK
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Local thresholds in this PR:"
rg -n 'set\(obs_macos_minimum_(sdk|xcode)' cmake/macos/compilerconfig.cmake

echo
echo "Upstream OBS 32.1.2 thresholds:"
curl -fsSL https://raw.githubusercontent.com/obsproject/obs-studio/32.1.2/cmake/macos/compilerconfig.cmake \
  | rg -n 'set\(obs_macos_minimum_(sdk|xcode)'

Repository: moq-dev/obs

Length of output: 369


26.1 macOS minimums are stricter than upstream OBS 32.1.2 baselines

Lines 19–20 set both SDK and Xcode minimums to 26.1, which is 11.1 and 10.1 versions above upstream OBS 32.1.2 (15.0 SDK, 16.0 Xcode). These values feed hard FATAL_ERROR checks and unnecessarily restrict valid build environments. This is unrelated to the PR's primary Windows FFmpeg/CMake changes.

Align with upstream by using 15.0 for SDK and 16.0 for Xcode.

Suggested fix
-  set(obs_macos_minimum_sdk 26.1) # Keep in sync with Xcode
-  set(obs_macos_minimum_xcode 26.1) # Keep in sync with SDK
+  set(obs_macos_minimum_sdk 15.0) # Keep in sync with Xcode
+  set(obs_macos_minimum_xcode 16.0) # Keep in sync with SDK
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
set(obs_macos_minimum_sdk 26.1) # Keep in sync with Xcode
set(obs_macos_minimum_xcode 26.1) # Keep in sync with SDK
set(obs_macos_minimum_sdk 15.0) # Keep in sync with Xcode
set(obs_macos_minimum_xcode 16.0) # Keep in sync with SDK
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmake/macos/compilerconfig.cmake` around lines 19 - 20, The macOS minimums
are set too high; update the CMake variables obs_macos_minimum_sdk and
obs_macos_minimum_xcode in compilerconfig.cmake to match upstream OBS baselines
by setting obs_macos_minimum_sdk to 15.0 and obs_macos_minimum_xcode to 16.0 so
the FATAL_ERROR checks use the correct, less restrictive minima.

@danrossi danrossi marked this pull request as draft May 4, 2026 10:49
@danrossi
Copy link
Copy Markdown
Author

danrossi commented May 4, 2026

The minimum mac requirements is now Xcode / SDK 26.1 and mac OS 15.6 to reflect the obs-studio build workflow. Windows needs a post install script as the locale file needs to go into the data directory not the plugins directory.

The mac build should be working now with the changes made I've tested it with a local relay but need to get it connected to an external relay. Still working on getting the Windows build connecting.

The patch needed of the obs source archive is within the macOS platform check so the bot is not correct again. It needs to be done during the obs setup function.

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