Use typed ordered::Consumer for video/audio in moq-ffi

[Qizot]

Replace the generic subscribe_media with typed subscribe_video and subscribe_audio methods that accept catalog config and use moq_mux::ordered::Consumer<T> directly. Also fix CMAF PTS calculation to use DTS + CTS offset for correct timestamp decoding.

[coderabbitai]

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b41c45ec-1ce6-4e23-a25a-25dd574b108e

📥 Commits

Reviewing files that changed from the base of the PR and between c260956 and 38e63a9.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• rs/moq-ffi/Cargo.toml
• rs/moq-ffi/src/consumer.rs
• rs/moq-ffi/src/error.rs

✅ Files skipped from review due to trivial changes (1)

• rs/moq-ffi/Cargo.toml

🚧 Files skipped from review as they are similar to previous changes (1)

• rs/moq-ffi/src/consumer.rs

---

Walkthrough

This PR changes media consumer construction to use moq_mux::ordered::Consumer with an explicit Container parameter, updating subscribe_media to accept a Container. UniFFI-exposed types (MoqDimensions, Container, MoqVideo, MoqAudio) now derive Clone and Container gains From -> hang::catalog::Container. The Container trait impl was retargeted to hang::catalog::Container (removing VideoConfig/AudioConfig impls). CMAF decoding now computes timestamps from dts + cts (clamped). Tests and consumer usage were updated; MoqError gained a Mux variant and rs/moq-ffi package version was bumped.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Use typed ordered::Consumer for video/audio in moq-ffi' accurately describes the primary change: refactoring to use typed consumers instead of generic ones.
Description check	✅ Passed	The description mentions replacing generic subscribe_media with typed methods and fixing CMAF PTS calculation, which aligns with the actual changes made in the changeset.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

rs/moq-mux/src/cmaf/container.rs (1)

45-46: Add a direct regression test for non-zero CTS offsets.

This is the right fix, but none of the shown tests exercise a CMAF sample where cts != 0, so a future regression back to raw DTS handling would slip through. A focused decode test here with a positive CTS offset would make this change much safer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@rs/moq-mux/src/cmaf/container.rs` around lines 45 - 46, Add a regression test
that constructs/decodes a CMAF sample with a non-zero CTS offset and asserts the
resulting Timestamp equals DTS + CTS (not raw DTS); specifically exercise the
pts calculation that uses "let pts = dts as i64 + entry.cts.unwrap_or(0) as i64"
and the call to Timestamp::from_scale so future changes don't regress to using
raw dts. Create a focused unit test that sets entry.cts to a positive value,
runs the same decode/path used by the existing CMAF tests, and assert the
produced timestamp (or sample PTS) equals the expected scaled value computed
from dts + cts. Ensure the test fails if code ignores entry.cts.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@rs/moq-ffi/src/media.rs`:
- Around line 33-51: The current TryFrom implementations for MoqVideo ->
hang::catalog::VideoConfig (and the analogous MoqAudio ->
hang::catalog::AudioConfig) drop timing metadata by hardcoding jitter: None and
optimize_for_latency: None; update the FFI types MoqVideo and MoqAudio to
include jitter and optimize_for_latency fields, ensure convert_catalog()
populates those fields from the importer, and change the TryFrom implementations
to map MoqVideo.jitter -> hang::catalog::VideoConfig.jitter and
MoqVideo.optimize_for_latency -> optimize_for_latency (and the analogous
MoqAudio mappings) instead of setting them to None so the rebuilt configs
preserve computed jitter/latency preferences when fed into
moq_mux::ordered::Consumer.

---

Nitpick comments:
In `@rs/moq-mux/src/cmaf/container.rs`:
- Around line 45-46: Add a regression test that constructs/decodes a CMAF sample
with a non-zero CTS offset and asserts the resulting Timestamp equals DTS + CTS
(not raw DTS); specifically exercise the pts calculation that uses "let pts =
dts as i64 + entry.cts.unwrap_or(0) as i64" and the call to
Timestamp::from_scale so future changes don't regress to using raw dts. Create a
focused unit test that sets entry.cts to a positive value, runs the same
decode/path used by the existing CMAF tests, and assert the produced timestamp
(or sample PTS) equals the expected scaled value computed from dts + cts. Ensure
the test fails if code ignores entry.cts.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5c675377-0946-40e0-8b60-b5bc0dbdd9de

📥 Commits

Reviewing files that changed from the base of the PR and between 4ddd4b0 and 751f0a6.

📒 Files selected for processing (4)

• rs/moq-ffi/src/consumer.rs
• rs/moq-ffi/src/media.rs
• rs/moq-ffi/src/test.rs
• rs/moq-mux/src/cmaf/container.rs

[coderabbitai]

🧹 Nitpick comments (1)

py/moq-lite/moq_lite/subscribe.py (1)

63-65: Consider emitting a deprecation warning at runtime.

The docstring marks this as deprecated, but callers won't see it unless they read documentation. Consider adding warnings.warn() to surface the deprecation at runtime.

💡 Optional: Add runtime deprecation warning

+import warnings
+
 def subscribe_media(self, name: str, max_latency_ms: int = 10_000) -> MediaConsumer:
     """Deprecated: Use subscribe_video or subscribe_audio instead."""
+    warnings.warn(
+        "subscribe_media is deprecated, use subscribe_video or subscribe_audio instead",
+        DeprecationWarning,
+        stacklevel=2,
+    )
     return MediaConsumer(self._inner.subscribe_media(name, max_latency_ms))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/moq-lite/moq_lite/subscribe.py` around lines 63 - 65, The subscribe_media
method is only marked deprecated in the docstring; add a runtime deprecation
warning by calling warnings.warn(...) at the start of subscribe_media to inform
callers (e.g., warnings.warn("subscribe_media is deprecated; use subscribe_video
or subscribe_audio", DeprecationWarning, stacklevel=2)). Keep the existing
return of MediaConsumer(self._inner.subscribe_media(...)) and ensure you import
the warnings module at the top of the file if not already present.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@py/moq-lite/moq_lite/subscribe.py`:
- Around line 63-65: The subscribe_media method is only marked deprecated in the
docstring; add a runtime deprecation warning by calling warnings.warn(...) at
the start of subscribe_media to inform callers (e.g.,
warnings.warn("subscribe_media is deprecated; use subscribe_video or
subscribe_audio", DeprecationWarning, stacklevel=2)). Keep the existing return
of MediaConsumer(self._inner.subscribe_media(...)) and ensure you import the
warnings module at the top of the file if not already present.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1906d943-b9fc-4324-bdd6-64a96d83610d

📥 Commits

Reviewing files that changed from the base of the PR and between 751f0a6 and 0f07e81.

⛔ Files ignored due to path filters (1)

• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (8)

• py/moq-lite/moq_lite/subscribe.py
• py/moq-lite/tests/test_local.py
• pyproject.toml
• rs/moq-ffi/src/consumer.rs
• rs/moq-ffi/src/media.rs
• rs/moq-ffi/src/test.rs
• rs/moq-mux/src/hang/container.rs
• rs/moq-mux/src/hang/mod.rs

✅ Files skipped from review due to trivial changes (3)

• pyproject.toml
• rs/moq-mux/src/hang/mod.rs
• rs/moq-ffi/src/test.rs