-
Notifications
You must be signed in to change notification settings - Fork 10
Feat/add hevc encoder #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughVideo initialization now maps 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/moq-output.cpp (2)
212-219: Update comment to reflect multi-codec support.The comment on line 216 mentions "For H.264, this returns the SPS/PPS" but the code also supports HEVC (as indicated by
video_codecs = "h264;hevc"on line 277), which includes VPS (Video Parameter Set) in addition to SPS/PPS. Update the comment to:Suggested change
-// For H.264, this returns the SPS/PPS +// For H.264, this returns the SPS/PPS; for HEVC, this returns the VPS/SPS/PPS
251-258: Update comment to reflect Opus support.The comment on line 255 only mentions AAC but the code now supports both AAC and Opus (as shown in
audio_codecsat line 275). Update the comment to document both codecs:Suggested comment update
-// obs_encoder_get_extra_data may only return data after the first frame has been encoded. -// For AAC, this returns 2 bytes containing the profile and the sample rate. +// obs_encoder_get_extra_data may only return data after the first frame has been encoded. +// For AAC, this returns 2 bytes containing the profile and the sample rate; for Opus, this returns the identification header.
🧹 Nitpick comments (1)
src/moq-service.cpp (1)
3-3: Consider updating or removing the TODO comment.The supported codecs are now defined on lines 4-5. Consider either removing this TODO or updating it to reference future codec support (e.g., AV1, VP9 mentioned in src/moq-output.cpp).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/moq-output.cppsrc/moq-service.cpp
🔇 Additional comments (2)
src/moq-output.cpp (1)
275-277: Codec support successfully extended.The codec strings have been properly updated to include Opus and HEVC support. The semicolon-delimited format is consistent with OBS output API requirements.
Note: The TODO comment on line 276 indicates future work for AV1 and VP9 support.
However, verify that the codec identifier strings match those used in moq-service.cpp and are recognized by OBS encoders. See the verification script in the comment for src/moq-service.cpp lines 4-5.
src/moq-service.cpp (1)
4-5: No action required. Codec identifier strings are correct.The codec identifiers "opus" and "hevc" match OBS conventions. This is confirmed by:
- Web sources explicitly listing "hevc" and "opus" as correct OBS builtin codec IDs
- Consistent usage across the codebase (moq-service.cpp, moq-output.cpp, moq-source.cpp)
- Proper handling in moq-source.cpp for codec parsing
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In @src/moq-output.cpp:
- Around line 223-234: Fix the typo in the comment above the
moq_publish_media_ordered call: change "Intialize the media import module with
the codec and initialization data." to "Initialize the media import module with
the codec and initialization data." This comment is adjacent to the
moq_publish_media_ordered(broadcast, moq_codec, strlen(moq_codec), extra_data,
extra_size) invocation and references the transformed codec variable moq_codec.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakePresets.jsonsrc/moq-output.cpp
🧰 Additional context used
📓 Path-based instructions (2)
CMakePresets.json
📄 CodeRabbit inference engine (CLAUDE.md)
Specify MOQ_VERSION in
CMakePresets.jsonto control libmoq library version (platform-specific targets: aarch64-apple-darwin, x86_64-unknown-linux-gnu, x86_64-pc-windows-msvc)
Files:
CMakePresets.json
**/*.{cpp,h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{cpp,h,hpp}: Use./build-aux/run-clang-formatfor C++ code formatting
Use logging macros fromlogger.h:LOG_DEBUG(),LOG_INFO(),LOG_WARNING(),LOG_ERROR()with[obs-moq]prefix
Files:
src/moq-output.cpp
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to moq-output.cpp : Use FFI calls `moq_track_send_*()` to send encoder packets from `MoQOutput::Data()` through `VideoData()`/`AudioData()` to the MoQ library
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to CMakePresets.json : Specify MOQ_VERSION in `CMakePresets.json` to control libmoq library version (platform-specific targets: aarch64-apple-darwin, x86_64-unknown-linux-gnu, x86_64-pc-windows-msvc)
📚 Learning: 2026-01-12T01:37:10.886Z
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to CMakePresets.json : Specify MOQ_VERSION in `CMakePresets.json` to control libmoq library version (platform-specific targets: aarch64-apple-darwin, x86_64-unknown-linux-gnu, x86_64-pc-windows-msvc)
Applied to files:
CMakePresets.json
📚 Learning: 2026-01-12T01:37:10.886Z
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: For local moq library development, use `just setup ../moq` to point CMake to a local moq repository instead of downloading pre-built binaries
Applied to files:
CMakePresets.json
📚 Learning: 2026-01-12T01:37:10.886Z
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to moq-output.cpp : Use FFI calls `moq_track_send_*()` to send encoder packets from `MoQOutput::Data()` through `VideoData()`/`AudioData()` to the MoQ library
Applied to files:
src/moq-output.cpp
📚 Learning: 2026-01-12T01:37:10.886Z
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to moq-output.{cpp,h} : MoQ Output (`moq-output.cpp/h`) should handle streaming output, manage MoQ server session, and process audio/video data packets
Applied to files:
src/moq-output.cpp
📚 Learning: 2026-01-12T01:37:10.886Z
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to moq-service.{cpp,h} : MoQ Service (`moq-service.cpp/h`) should configure connection to MoQ relay: server URL, path, and supported codecs
Applied to files:
src/moq-output.cpp
📚 Learning: 2026-01-12T01:37:10.886Z
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to obs-moq.cpp : Plugin initialization must call `obs_module_load()` in `obs-moq.cpp`, configure Rust library logging via `moq_log_level()`, and register three components: service, output, and source
Applied to files:
src/moq-output.cpp
📚 Learning: 2026-01-12T01:37:10.886Z
Learnt from: CR
Repo: moq-dev/obs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T01:37:10.886Z
Learning: Applies to moq-source.{cpp,h} : MoQ Source (`moq-source.cpp/h`) is an experimental feature for receiving MoQ broadcasts
Applied to files:
src/moq-output.cpp
🔇 Additional comments (2)
src/moq-output.cpp (1)
286-288: Audio codec strings are correct as-is; no transformation needed.The codec lists are properly implemented. Unlike video codecs which require ISO FourCC transformation (h264→avc3, hevc→hev1), audio codecs pass directly to
moq_publish_media_ordered()as "aac" and "opus"—this is consistent with howmoq-service.cppdeclares supported audio codecs. The extra_data (AAC profile/sample rate info) provides the necessary initialization context for the MoQ library.CMakePresets.json (1)
16-16: Version 0.2.4 does not exist and will cause build failures.The
MOQ_VERSIONis set to "0.2.4", but this version does not exist in the moq-dev/moq repository (only v0.1.2 and v0.1.1 are released). Public registries show the maximum available version is 0.2.2. This configuration will fail when CMake attempts to fetch the non-existent release.Update
MOQ_VERSIONto an available version (e.g., "0.2.2" or "0.1.2").⛔ Skipped due to learnings
Learnt from: CR Repo: moq-dev/obs PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-12T01:37:10.886Z Learning: Applies to CMakePresets.json : Specify MOQ_VERSION in `CMakePresets.json` to control libmoq library version (platform-specific targets: aarch64-apple-darwin, x86_64-unknown-linux-gnu, x86_64-pc-windows-msvc)Learnt from: CR Repo: moq-dev/obs PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-12T01:37:10.886Z Learning: For local moq library development, use `just setup ../moq` to point CMake to a local moq repository instead of downloading pre-built binaries
| // Transform codec string for MoQ | ||
| const char *moq_codec = codec; | ||
| if (strcmp(codec, "h264") == 0) { | ||
| // H.264 with inline SPS/PPS | ||
| moq_codec = "avc3"; | ||
| } else if (strcmp(codec, "hevc") == 0) { | ||
| // H.265 with inline VPS/SPS/PPS | ||
| moq_codec = "hev1"; | ||
| } | ||
|
|
||
| // Intialize the media import module with the codec and initialization data. | ||
| video = moq_publish_media_ordered(broadcast, moq_codec, strlen(moq_codec), extra_data, extra_size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codec transformation logic is correct; minor typo on line 233.
The mapping of h264→avc3 and hevc→hev1 correctly represents the ISOBMFF codec identifiers for streams with in-band parameter sets. The fallback to the original codec string provides good forward compatibility.
📝 Fix typo
- // Intialize the media import module with the codec and initialization data.
+ // Initialize the media import module with the codec and initialization data.🤖 Prompt for AI Agents
In @src/moq-output.cpp around lines 223 - 234, Fix the typo in the comment above
the moq_publish_media_ordered call: change "Intialize the media import module
with the codec and initialization data." to "Initialize the media import module
with the codec and initialization data." This comment is adjacent to the
moq_publish_media_ordered(broadcast, moq_codec, strlen(moq_codec), extra_data,
extra_size) invocation and references the transformed codec variable moq_codec.
This PR adds HEVC to the supported video codecs for the MoQ service.
Testable with: moq-dev/moq#813