feat(ffi): expose generic CMAF muxing - #2765
Conversation
Add transport-independent frame muxing with explicit timeline origins, exact sample durations, atomic initialization and fragment output, and codec reconfiguration boundaries. Expose the same API through UniFFI, C, Swift, Kotlin, Python, and Go. Co-Authored-By: Codex <codex@openai.com>
WalkthroughAdded CMAF muxing to the Rust fMP4 engine, including timestamp rebasing, codec metadata handling, configuration boundaries, and sample durations. Added C and UniFFI bindings with lifecycle and error handling. Added Go, Kotlin, Python, and Swift APIs with tests. Added documentation for CMAF packaging workflows and output behavior. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
py/moq-rs/moq/cmaf.py (1)
27-51: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExpose deterministic CMAF muxer cleanup in every wrapper.
CmafMuxerandCMAFMuxerwrap a nativeMoqCmafMuxer, but neither wrapper exposesclose. Add explicit cleanup in both APIs. Add a Python context manager so callers can reliably release the handle.
py/moq-rs/moq/cmaf.py#L27-L51: addclose()and context-manager cleanup that delegates to the FFI handle.swift/Sources/Moq/CMAF.swift#L12-L61: add a publicclose()method that delegates to the FFI handle.The Kotlin wrapper already exposes
AutoCloseable.close()for the same FFI handle.(Written by CodeRabbit)
🤖 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 `@py/moq-rs/moq/cmaf.py` around lines 27 - 51, Expose deterministic cleanup for both CMAF wrappers: in py/moq-rs/moq/cmaf.py, add CmafMuxer.close() delegating to the native _ffi handle and implement context-manager entry/exit cleanup; in swift/Sources/Moq/CMAF.swift, add the public CMAFMuxer.close() method delegating to its FFI handle. Ensure both wrappers release the underlying MoqCmafMuxer explicitly.kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt (1)
91-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftVerify the timestamp rebasing result, not only fragment presence.
Each test passes a frame timestamp equal to the configured origin. Each test then checks only for non-empty output. A wrapper that forwards
0as the origin would still pass. Decode or inspect the fragmenttfdtand assert that the first sample is rebased to zero.
kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt#L91-L117: assert the fragment decode time is zero after muxing.py/moq-rs/tests/test_local.py#L102-L127: assert the fragment decode time is zero after muxing.swift/Tests/MoqTests/SmokeTests.swift#L174-L199: assert the fragment decode time is zero after muxing.The supplied native muxer test demonstrates the required timestamp check.
(Written by CodeRabbit)
🤖 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 `@kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt` around lines 91 - 117, The muxer smoke tests only verify fragment presence and must also verify timestamp rebasing. In kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt lines 91-117, py/moq-rs/tests/test_local.py lines 102-127, and swift/Tests/MoqTests/SmokeTests.swift lines 174-199, inspect or decode the generated fragment’s tfdt and assert that the first sample decode time is zero after using the configured origin; retain the existing initialization and fragment presence assertions.rs/libmoq/src/test.rs (1)
127-174: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the rebased timestamp.
The test only verifies that a fragment exists. A muxer that ignores
origin_usalso passes this test. Decode the fragment and assert that its first timestamp is zero.As per coding guidelines, add a regression test that fails without the fix. (Written by CodeRabbit)
🤖 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 `@rs/libmoq/src/test.rs` around lines 127 - 174, The cmaf_muxer_constructs_and_rebases_frames test must verify origin rebasing, not just fragment creation. Decode output.fragment using the existing CMAF/fragment parsing utilities, assert the first decoded sample timestamp is zero, and keep the test configured with origin_us and timestamp_us both set to 10_000_000 so it fails when rebasing is ignored.Source: Coding guidelines
rs/moq-mux/src/container/fmp4/muxer.rs (1)
307-323: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the origin conversion out of the per-frame loop when the scale repeats.
fragment_rebasedrecomputesmoq_net::Timestamp::try_from(origin)?on every iteration, and converts it toframe.timestamp.scale()each time. Frames in one batch normally share a scale, so the conversion repeats without effect. Caching the converted origin per scale keeps behavior identical and removes the repeated work.♻️ Proposed refactor
pub fn fragment_rebased(&self, sequence: u32, origin: Duration, frames: &[Frame]) -> crate::Result<Bytes> { + let origin = moq_net::Timestamp::try_from(origin)?; let mut rebased = Vec::with_capacity(frames.len()); + let mut cached: Option<(moq_net::Timescale, moq_net::Timestamp)> = None; for frame in frames { - let track_origin = moq_net::Timestamp::try_from(origin)?.convert(frame.timestamp.scale())?; + let scale = frame.timestamp.scale(); + let track_origin = match cached { + Some((cached_scale, value)) if cached_scale == scale => value, + _ => { + let value = origin.convert(scale)?; + cached = Some((scale, value)); + value + } + }; let timestamp = frame.timestamp.checked_sub(track_origin)?;🤖 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 `@rs/moq-mux/src/container/fmp4/muxer.rs` around lines 307 - 323, Update fragment_rebased to cache the converted origin per timestamp scale instead of recomputing Timestamp::try_from(origin) and convert for every frame; reuse the cached value when consecutive or repeated frame.timestamp.scale() values match, while preserving the existing checked subtraction and fragment_owned behavior.
🤖 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 `@doc/lib/c/index.md`:
- Around line 279-295: After the moq_cmaf_video call, return from the example or
otherwise skip frame setup and moq_cmaf_mux when muxer is negative. Keep the
existing moq_error reporting, and allow the mux path to run only after
successful muxer creation.
In `@rs/libmoq/src/cmaf.rs`:
- Around line 205-208: Update the frame-size handling in moq_cmaf_video so a
non-zero coded_width combined with a zero coded_height, or vice versa, returns
Error::InvalidCode. Preserve assigning both values when they are non-zero and
continue treating both-zero dimensions as unknown.
In `@rs/moq-ffi/src/cmaf.rs`:
- Around line 20-28: Add a Rust-only MoqCmafConfig::new(track) constructor that
initializes track and sets origin_us to zero, while retaining the existing
#[uniffi(default = 0)] annotation for foreign callers.
In `@rs/moq-ffi/src/test.rs`:
- Around line 492-511: Strengthen the fragment metadata assertion in the muxer
test by capturing traf.trun[0].entries.len() and the inferred duration of the
second entry alongside the existing sequence, track, decode-time, and
first-duration values. Update the expected fragment_info tuple to require
exactly two samples and the expected second-entry duration, preserving the
existing checks.
---
Nitpick comments:
In `@kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt`:
- Around line 91-117: The muxer smoke tests only verify fragment presence and
must also verify timestamp rebasing. In
kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt lines 91-117,
py/moq-rs/tests/test_local.py lines 102-127, and
swift/Tests/MoqTests/SmokeTests.swift lines 174-199, inspect or decode the
generated fragment’s tfdt and assert that the first sample decode time is zero
after using the configured origin; retain the existing initialization and
fragment presence assertions.
In `@py/moq-rs/moq/cmaf.py`:
- Around line 27-51: Expose deterministic cleanup for both CMAF wrappers: in
py/moq-rs/moq/cmaf.py, add CmafMuxer.close() delegating to the native _ffi
handle and implement context-manager entry/exit cleanup; in
swift/Sources/Moq/CMAF.swift, add the public CMAFMuxer.close() method delegating
to its FFI handle. Ensure both wrappers release the underlying MoqCmafMuxer
explicitly.
In `@rs/libmoq/src/test.rs`:
- Around line 127-174: The cmaf_muxer_constructs_and_rebases_frames test must
verify origin rebasing, not just fragment creation. Decode output.fragment using
the existing CMAF/fragment parsing utilities, assert the first decoded sample
timestamp is zero, and keep the test configured with origin_us and timestamp_us
both set to 10_000_000 so it fails when rebasing is ignored.
In `@rs/moq-mux/src/container/fmp4/muxer.rs`:
- Around line 307-323: Update fragment_rebased to cache the converted origin per
timestamp scale instead of recomputing Timestamp::try_from(origin) and convert
for every frame; reuse the cached value when consecutive or repeated
frame.timestamp.scale() values match, while preserving the existing checked
subtraction and fragment_owned behavior.
🪄 Autofix
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 Plus
Run ID: 44b7ce17-6365-496f-9c25-6cdc3a2df962
📒 Files selected for processing (30)
doc/lib/c/index.mddoc/lib/go/moq.mddoc/lib/kt/moq.mddoc/lib/py/moq-rs.mddoc/lib/swift/moq.mdgo/wrapper/moq/cmaf.gogo/wrapper/moq/moq_test.gogo/wrapper/moq/types.gokt/moq/src/jvmAndAndroidMain/kotlin/dev/moq/Cmaf.ktkt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.ktpy/moq-rs/moq/__init__.pypy/moq-rs/moq/cmaf.pypy/moq-rs/tests/test_local.pyrs/libmoq/src/cmaf.rsrs/libmoq/src/lib.rsrs/libmoq/src/state.rsrs/libmoq/src/test.rsrs/moq-ffi/src/cmaf.rsrs/moq-ffi/src/consumer.rsrs/moq-ffi/src/lib.rsrs/moq-ffi/src/media.rsrs/moq-ffi/src/test.rsrs/moq-mux/src/codec/h264/mod.rsrs/moq-mux/src/codec/h265/mod.rsrs/moq-mux/src/container/fmp4/export.rsrs/moq-mux/src/container/fmp4/mod.rsrs/moq-mux/src/container/fmp4/muxer.rsrs/moq-mux/src/container/source.rsswift/Sources/Moq/CMAF.swiftswift/Tests/MoqTests/SmokeTests.swift
| int32_t muxer = moq_cmaf_video(&config); | ||
| if (muxer < 0) { | ||
| fprintf(stderr, "muxer creation failed: %s\n", moq_error()); | ||
| } | ||
|
|
||
| struct moq_cmaf_frame frame = { | ||
| .payload = payload, | ||
| .payload_size = payload_size, | ||
| .timestamp_us = timestamp_us, | ||
| .keyframe = true, | ||
| .duration_us = duration_us, | ||
| .has_duration = true, | ||
| }; | ||
| struct moq_cmaf_output output = {0}; | ||
| if (moq_cmaf_mux(muxer, sequence, &frame, 1, &output) < 0) { | ||
| fprintf(stderr, "mux failed: %s\n", moq_error()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stop after CMAF muxer creation fails.
If moq_cmaf_video returns a negative handle, Line 293 still passes that handle to moq_cmaf_mux. Exit the example or guard the mux call after reporting the creation error.
(Written by CodeRabbit)
🤖 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 `@doc/lib/c/index.md` around lines 279 - 295, After the moq_cmaf_video call,
return from the example or otherwise skip frame setup and moq_cmaf_mux when
muxer is negative. Keep the existing moq_error reporting, and allow the mux path
to run only after successful muxer creation.
| if raw.coded_width != 0 && raw.coded_height != 0 { | ||
| config.coded_width = Some(raw.coded_width); | ||
| config.coded_height = Some(raw.coded_height); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
A partially specified frame size is dropped without notice.
moq_cmaf_video applies coded_width and coded_height only when both values are non-zero. Each field is documented as "zero when unknown" on its own, so a caller that supplies only a width gets neither value, and no error. Returning Error::InvalidCode for a half-specified size makes the contract explicit.
🐛 Proposed fix
- if raw.coded_width != 0 && raw.coded_height != 0 {
- config.coded_width = Some(raw.coded_width);
- config.coded_height = Some(raw.coded_height);
- }
+ match (raw.coded_width, raw.coded_height) {
+ (0, 0) => {}
+ (width, height) if width != 0 && height != 0 => {
+ config.coded_width = Some(width);
+ config.coded_height = Some(height);
+ }
+ // A half-specified size is a caller mistake, not an unknown size.
+ _ => return Err(Error::InvalidCode),
+ }📝 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.
| if raw.coded_width != 0 && raw.coded_height != 0 { | |
| config.coded_width = Some(raw.coded_width); | |
| config.coded_height = Some(raw.coded_height); | |
| } | |
| match (raw.coded_width, raw.coded_height) { | |
| (0, 0) => {} | |
| (width, height) if width != 0 && height != 0 => { | |
| config.coded_width = Some(width); | |
| config.coded_height = Some(height); | |
| } | |
| // A half-specified size is a caller mistake, not an unknown size. | |
| _ => return Err(Error::InvalidCode), | |
| } |
🤖 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 `@rs/libmoq/src/cmaf.rs` around lines 205 - 208, Update the frame-size handling
in moq_cmaf_video so a non-zero coded_width combined with a zero coded_height,
or vice versa, returns Error::InvalidCode. Preserve assigning both values when
they are non-zero and continue treating both-zero dimensions as unknown.
| /// Options for constructing a single-rendition CMAF muxer. | ||
| #[derive(uniffi::Record)] | ||
| pub struct MoqCmafConfig { | ||
| /// The encoded rendition to package. | ||
| pub track: MoqCmafTrack, | ||
| /// Timestamp subtracted from every sample so independently fetched fragments share a timeline. | ||
| #[uniffi(default = 0)] | ||
| pub origin_us: u64, | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does UniFFI 0.31.0 support Rust #[non_exhaustive] on a type deriving uniffi::Record?
💡 Result:
UniFFI 0.31.0 does not support using the Rust #[non_exhaustive] attribute on a type deriving uniffi::Record. While UniFFI has added specific support for non-exhaustive enums (using a separate [NonExhaustive] attribute in UDL or through related proc-macro mechanisms) [1][2][3], this support does not extend to records. UniFFI Records are strictly structured data consisting of a fixed set of named fields, and the current proc-macro implementation for uniffi::Record does not accommodate the semantic flexibility or forward-compatibility requirements implied by the Rust #[non_exhaustive] attribute [4][5]. If you need to handle evolving data structures with UniFFI, the library typically recommends managing versioning through explicit field defaults [5][6] or defining wrapper types that manage compatibility between different versions of the data.
Citations:
- 1: https://mozilla.github.io/uniffi-rs/0.27/udl/enumerations.html
- 2: https://github.com/mozilla/uniffi-rs/blob/main/docs/manual/src/udl/enumerations.md
- 3: https://mozilla.github.io/uniffi-rs/latest/udl/enumerations.html
- 4: https://mozilla.github.io/uniffi-rs/0.30/types/records.html
- 5: https://mozilla.github.io/uniffi-rs/0.30/proc_macro/records.html
- 6: bendk/uniffi-rs@c66ce86
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- cmaf.rs ---'
cat -n rs/moq-ffi/src/cmaf.rs | sed -n '1,180p'
printf '%s\n' '--- moq-ffi manifests ---'
rg -n -C 3 'uniffi|version' rs/moq-ffi/Cargo.toml Cargo.toml rs -g 'Cargo.toml' | head -160
printf '%s\n' '--- comparable records and constructors ---'
rg -n -C 5 '#\[derive\(uniffi::Record\)|#\[uniffi::export\]|pub struct Moq.*Config|fn new\(' rs/moq-ffi rs/libmoq -g '*.rs' | head -240
printf '%s\n' '--- rust tool availability ---'
rustc --version 2>/dev/null || true
cargo --version 2>/dev/null || trueRepository: moq-dev/moq
Length of output: 28195
🌐 Web query:
site:github.com/mozilla/uniffi-rs 0.31.0 non_exhaustive Record Rust attribute
💡 Result:
The term "non_exhaustive" in the context of the UniFFI project refers to the Rust attribute #[non_exhaustive] [1], which is used to indicate that an enum or struct may have additional variants or fields added in future versions [1][2]. There is no specific UniFFI attribute named "non_exhaustive record". Rather, UniFFI handles non_exhaustive types in the following ways: 1. Standard Rust Attribute: UniFFI respects the standard library #[non_exhaustive] attribute applied to Rust enums or structs [1][2]. 2. UDL Support: When working with UniFFI's Interface Definition Language (UDL), there is a [NonExhaustive] UDL attribute used to inform UniFFI that a type (typically an enum) defined in an upstream crate is marked as non-exhaustive [1][2]. This is necessary because UniFFI must generate a default arm when matching against these variants to prevent compilation errors in foreign language bindings [1]. 3. Records: UniFFI provides a #[derive(uniffi::Record)] macro to expose Rust structs as records (dictionaries) [3]. While #[non_exhaustive] can be applied to Rust structs, the uniffi::Record derive macro primarily focuses on mapping named fields [3]. UniFFI v0.31.0 is a released version (as of January 2026), but it does not introduce a specific "non_exhaustive record" feature [4]. The support for non_exhaustive types has been part of UniFFI's evolution to handle remote types from other crates correctly [1][5][2]. [1][2][4]
Citations:
- 1: https://github.com/mozilla/uniffi-rs/blob/main/docs/manual/src/udl/enumerations.md
- 2: Non-exhaustive enums seem to trouble uniffi mozilla/uniffi-rs#2414
- 3: https://github.com/mozilla/uniffi-rs/blob/main/docs/manual/src/proc_macro/records.md
- 4: https://github.com/mozilla/uniffi-rs/blob/HEAD/CHANGELOG.md
- 5: Support for
#[non_exhaustive]enums mozilla/uniffi-rs#1593
🌐 Web query:
site:mozilla.github.io/uniffi-rs/0.31 records non_exhaustive uniffi::Record
💡 Result:
UniFFI does not provide a non_exhaustive attribute for the uniffi::Record derive macro. The non_exhaustive attribute is specifically supported for enumerations in UniFFI, particularly when using the Interface Definition Language (UDL) [1]. For enums defined in external crates that are marked as #[non_exhaustive], UniFFI provides a [NonExhaustive] attribute in UDL to facilitate handling them by generating a default arm for matching [1]. This is necessary because UniFFI requires exhaustive matching during the encoding and decoding process [2]. If you are working with structs using the #[derive(uniffi::Record)] macro, there is no corresponding non_exhaustive mechanism [3][4]. UniFFI records require a defined structure that must be consistent between Rust and the generated bindings [3]. If you encounter compatibility issues with external structs, it is generally recommended to use a wrapper (such as a newtype) or a custom type converter, though these approaches involve additional implementation effort [5].
Citations:
- 1: https://github.com/mozilla/uniffi-rs/blob/main/docs/manual/src/udl/enumerations.md
- 2: Non-exhaustive enums seem to trouble uniffi mozilla/uniffi-rs#2414
- 3: https://github.com/mozilla/uniffi-rs/blob/main/docs/manual/src/proc_macro/records.md
- 4: Add support for records (dictionaries / structs) to the proc-macro frontend mozilla/uniffi-rs#1360
- 5: Support for
#[non_exhaustive]enums mozilla/uniffi-rs#1593
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
import urllib.request
base = "https://api.github.com/repos/mozilla/uniffi-rs"
paths = [
"/git/trees/v0.31.0?recursive=1",
"/contents/uniffi_macros/src/record.rs?ref=v0.31.0",
"/contents/uniffi_bindgen/src?ref=v0.31.0",
]
for path in paths:
print(f"--- {path} ---")
req = urllib.request.Request(
base + path,
headers={"Accept": "application/vnd.github+json", "User-Agent": "review-inspection"},
)
try:
with urllib.request.urlopen(req, timeout=15) as response:
data = json.load(response)
if path.startswith("/git/trees"):
for item in data.get("tree", []):
name = item["path"].lower()
if "record" in name or "non_exhaustive" in name or "changelog" in name:
print(item["path"])
else:
print(json.dumps(data, indent=2)[:12000])
except Exception as exc:
print(type(exc).__name__ + ": " + str(exc))
PYRepository: moq-dev/moq
Length of output: 730
🌐 Web query:
UniFFI 0.31.0 #[non_exhaustive] #[derive(uniffi::Record)] compile struct
💡 Result:
In UniFFI 0.31.0, the #[non_exhaustive] attribute is primarily supported for enums defined in UDL, where it is handled via the [NonExhaustive] UDL attribute to allow UniFFI to generate a default arm for matching variants [1][2]. Regarding structs (records) marked with #[derive(uniffi::Record)], UniFFI does not have a corresponding #[non_exhaustive] feature for structs defined as records [3][4]. The #[non_exhaustive] attribute on a Rust struct is a language-level feature that prevents construction and pattern matching outside the crate defining the struct. Because UniFFI's #[derive(uniffi::Record)] macro requires full access to the struct's fields to generate the necessary FFI scaffolding (lift/lower implementations), using #[non_exhaustive] on a struct intended to be a UniFFI record will typically lead to compilation errors [3][5]. If you are encountering a compile error, it is likely because the derive macro cannot inspect the fields of the #[non_exhaustive] struct. To expose such a type through UniFFI, you must typically use a "wrapper" pattern: 1. Define a separate, non-exhaustive-compatible struct or type for your public API. 2. Implement a conversion (using something like a custom type or a manual mapping) between the external #[non_exhaustive] struct and a UniFFI-compatible record that you control [6]. UniFFI's internal metadata does support a non_exhaustive flag for enums [7][8], but this does not extend to providing equivalent support for non-exhaustive structs derived as records [3][5].
Citations:
- 1: https://github.com/mozilla/uniffi-rs/blob/main/docs/manual/src/udl/enumerations.md
- 2: https://mozilla.github.io/uniffi-rs/latest/udl/enumerations.html
- 3: https://mozilla.github.io/uniffi-rs/latest/proc_macro/records.html
- 4: https://mozilla.github.io/uniffi-rs/0.30/types/records.html
- 5: https://docs.rs/uniffi/latest/uniffi/derive.Record.html
- 6: Support for
#[non_exhaustive]enums mozilla/uniffi-rs#1593 - 7: https://docs.rs/uniffi_meta/latest/src/uniffi_meta/lib.rs.html
- 8: https://docs.rs/uniffi_bindgen/latest/src/uniffi_bindgen/interface/enum_.rs.html
Add a Rust constructor for MoqCmafConfig.
UniFFI records do not support #[non_exhaustive]. Add MoqCmafConfig::new(track) with origin_us set to zero, and retain the UniFFI field default for foreign callers.
(Written by CodeRabbit)
🤖 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 `@rs/moq-ffi/src/cmaf.rs` around lines 20 - 28, Add a Rust-only
MoqCmafConfig::new(track) constructor that initializes track and sets origin_us
to zero, while retaining the existing #[uniffi(default = 0)] annotation for
foreign callers.
Source: Coding guidelines
| let video_fragment = muxer.mux(12, first_video).unwrap().fragment.unwrap(); | ||
| let fragment_info = [video_fragment] | ||
| .into_iter() | ||
| .map(|fragment| { | ||
| let mut cursor = std::io::Cursor::new(fragment.as_slice()); | ||
| while let Some(atom) = mp4_atom::Any::decode_maybe(&mut cursor).unwrap() { | ||
| if let mp4_atom::Any::Moof(moof) = atom { | ||
| let traf = &moof.traf[0]; | ||
| return ( | ||
| moof.mfhd.sequence_number, | ||
| traf.tfhd.track_id, | ||
| traf.tfdt.as_ref().unwrap().base_media_decode_time, | ||
| traf.trun[0].entries[0].duration, | ||
| ); | ||
| } | ||
| } | ||
| panic!("fragment missing moof") | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| assert_eq!(fragment_info, [(12, 1, 0, Some(510))]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the sample count so a dropped or duplicated sample fails the test.
The test supplies two frames but inspects only traf.trun[0].entries[0]. If the muxer dropped the second sample, or emitted an extra one, the assertion still passes. Capturing traf.trun[0].entries.len() closes that gap. Asserting the second entry's inferred duration also pins the gap-inference behavior that the delta frame exercises.
💚 Proposed test strengthening
while let Some(atom) = mp4_atom::Any::decode_maybe(&mut cursor).unwrap() {
if let mp4_atom::Any::Moof(moof) = atom {
let traf = &moof.traf[0];
return (
moof.mfhd.sequence_number,
traf.tfhd.track_id,
traf.tfdt.as_ref().unwrap().base_media_decode_time,
+ traf.trun[0].entries.len(),
traf.trun[0].entries[0].duration,
);
}
}
panic!("fragment missing moof")
})
.collect::<Vec<_>>();
- assert_eq!(fragment_info, [(12, 1, 0, Some(510))]);
+ assert_eq!(fragment_info, [(12, 1, 0, 2, Some(510))]);📝 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.
| let video_fragment = muxer.mux(12, first_video).unwrap().fragment.unwrap(); | |
| let fragment_info = [video_fragment] | |
| .into_iter() | |
| .map(|fragment| { | |
| let mut cursor = std::io::Cursor::new(fragment.as_slice()); | |
| while let Some(atom) = mp4_atom::Any::decode_maybe(&mut cursor).unwrap() { | |
| if let mp4_atom::Any::Moof(moof) = atom { | |
| let traf = &moof.traf[0]; | |
| return ( | |
| moof.mfhd.sequence_number, | |
| traf.tfhd.track_id, | |
| traf.tfdt.as_ref().unwrap().base_media_decode_time, | |
| traf.trun[0].entries[0].duration, | |
| ); | |
| } | |
| } | |
| panic!("fragment missing moof") | |
| }) | |
| .collect::<Vec<_>>(); | |
| assert_eq!(fragment_info, [(12, 1, 0, Some(510))]); | |
| let video_fragment = muxer.mux(12, first_video).unwrap().fragment.unwrap(); | |
| let fragment_info = [video_fragment] | |
| .into_iter() | |
| .map(|fragment| { | |
| let mut cursor = std::io::Cursor::new(fragment.as_slice()); | |
| while let Some(atom) = mp4_atom::Any::decode_maybe(&mut cursor).unwrap() { | |
| if let mp4_atom::Any::Moof(moof) = atom { | |
| let traf = &moof.traf[0]; | |
| return ( | |
| moof.mfhd.sequence_number, | |
| traf.tfhd.track_id, | |
| traf.tfdt.as_ref().unwrap().base_media_decode_time, | |
| traf.trun[0].entries.len(), | |
| traf.trun[0].entries[0].duration, | |
| ); | |
| } | |
| } | |
| panic!("fragment missing moof") | |
| }) | |
| .collect::<Vec<_>>(); | |
| assert_eq!(fragment_info, [(12, 1, 0, 2, Some(510))]); |
🤖 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 `@rs/moq-ffi/src/test.rs` around lines 492 - 511, Strengthen the fragment
metadata assertion in the muxer test by capturing traf.trun[0].entries.len() and
the inferred duration of the second entry alongside the existing sequence,
track, decode-time, and first-duration values. Update the expected fragment_info
tuple to require exactly two samples and the expected second-entry duration,
preserving the existing checks.
1 participant
Summary
Public API changes
fmp4::Output,Muxer::fragment_rebased,Muxer::mux, andError::CodecConfigChangedtomoq-mux.MoqCmafTrack,MoqCmafConfig,MoqCmafOutput, andMoqCmafMuxertomoq-ffi, plus optionalduration_usmetadata onMoqMediaFrame.moq_cmaf_*configuration, frame, output, creation, muxing, initialization, and close surface tolibmoq.main.Test plan
cargo fmt --all -- --checkcargo check -p moq-mux -p moq-ffi -p libmoqcargo nextest run --all-targets -p moq-mux -p moq-ffi -p libmoq --no-fail-fast(607 passed)(Written by GPT-5)