feat(moq-gst): opt pads fed by a local encoder into flush jitter

[kixelated]

Completes GStreamer encoder jitter, split out of #3940.

A moqsink pad can't tell a local encoder from an import: multifilesrc ! parsebin ! moqsink hands over the same TIME segment and PTS as x264enc ! moqsink. So the pad opts in explicitly. An opted-in audio or video pad calls import::Track::flush(pts, Instant::now()) after each successful write, and the catalog jitter then includes how irregularly frames reach the sink behind their running time. Every other pad stays clock-free, as it is today.

• New request-pad property encoder (boolean, default false). It has the same lifecycle as track and container: writable in any state until the CAPS event reserves the track, fixed after that, and writable again after going back to READY.
• encoder=true on a text or opaque pad fails that pad at CAPS (track-error), so the setting is never silently ignored. An encoder frame whose timestamp is out of range also fails the pad instead of skipping the observation.
• Sink::Media becomes a Media struct with its own write, mirroring Text.
• Docs: doc/bin/gstreamer.md documents the property and adds it to the test-pattern example. No demo pipeline encodes locally: demo/pub gst is the looped MP4 import and stays off.
• The quest is deleted. quest/m1/jitter-flush-clock.md now points at the property and records one gap: a flushing seek on an encoder pad keeps the flush baseline, because import::Track forwards no discontinuity.

Opt-in: name and lifecycle (maintainer's call)

I implemented A. Pick another and I'll switch it.

• A. encoder boolean pad property (recommended). It names the pad by its role (what feeds it) and reads plainly in a launch line: sink_0::encoder=true. It's per pad, so a camera track and a file audio track can share one sink.
• B. source enum pad property, import (default) / encoder. This can grow a third provenance later, but no third one exists yet. source also overloads GStreamer's src-pad vocabulary.
• C. Element-level encoder property. It's shorter for the common "everything is encoded here" case, but it can't express a mixed pipeline, and a later per-pad override would duplicate it.

For lifecycle, the alternative is letting the property change at any time. I didn't, because turning it on mid-stream would start the baseline at an arbitrary point, and turning it off would freeze a half-measured jitter. Fixing it at reservation matches track and container.

I rejected auto-detection (walking upstream for an element whose klass contains Encoder) because it breaks across queue/parse/tee/appsrc and misfires on transcodes.

Public API / wire

• moqsink request pads gain the encoder property. This is additive and the default keeps today's behavior.
• No Rust public API change. No wire change: catalog jitter already exists, and only its value is new, on opted-in pads.

Testing

• pad.rs: only_an_encoder_pad_measures_its_handoff sends the same three frames (the last one arrives 100 ms late) to an encoder pad and an import pad on one catalog. The encoder pad's jitter is 100 ms and the import pad's is None. an_encoder_pad_must_carry_audio_or_video covers the refusal.
• tests/element.rs: sink_0::encoder=true parses from a pipeline description, and pads default to off.
• just check.
• End to end against a local relay (debug build), then curl /fetch/<broadcast>/catalog.json:
  • videotestsrc is-live=true ! openh264enc ! h264parse ! moqsink sink_0::encoder=true advertises jitter (100 to 600 ms here; per-frame logging showed about 130 ms sink stalls followed by bursts on a busy machine, which is real handoff lateness).
  • The demo/pub gst recipe shape (multifilesrc loop=true ! parsebin ! identity sync=true ! moqsink) advertises no jitter.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-25T15:44:10.244457Z	4b32bb1	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[kixelated]

MERGE

Positive improvement? Yes. Catalog jitter must reflect local encoder handoff irregularity, and a GStreamer TIME segment plus PTS cannot tell x264enc ! moqsink from multifilesrc ! parsebin ! moqsink. Making provenance an explicit pad opt-in is the right fix: imports stay clock-free by default, and only opted-in audio/video pads call flush. That closes the moq-gst gap against the flush-clock work already in libmoq, OBS, and js/publish.

Worth the complexity? Yes. Most of the diff is mechanical (Media as a struct with its own write, Instant threaded for tests, property plumbing) plus a focused pair of unit tests. The surface is additive (encoder defaults to false). Refusing encoder=true on text/opaque at CAPS, and failing an opted-in pad on an out-of-range timestamp instead of skipping the observation, are the right fail-loud choices. Fixing the property at reservation (same lifecycle as track / container) avoids a half-measured baseline from mid-stream flips.

Different approach? Option A (encoder boolean) is the right pick for now: it reads clearly in a launch line, is per-pad for mixed sinks, and does not invent a third source enum value that does not exist yet. Auto-detection via upstream klass walking was correctly rejected. The documented flushing-seek / discontinuity gap is real but shared with other bindings that expose flush without discontinuity; it should not block this PR (track it with the existing jitter-flush-clock note).

Ship it.

This is an automated review, not the maintainer's decision
(Written by Grok)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 327b246bfd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reset the jitter baseline after pipeline pauses

In the PLAYING -> PAUSED -> PLAYING path, leave_playing() preserves the media track and its estimator, but GStreamer running time, and therefore micros, stops while Instant continues advancing. If an encoder pipeline resumes within the estimator's 10-second sample window, this flush records roughly the entire pause as jitter, and the catalog's never-lower behavior makes that false value permanent. Reset the encoder estimator's flush baseline on this state discontinuity before observing the next frame.

Useful? React with 👍 / 👎.

[kixelated]

Valid, but deferring. Resetting the baseline needs a discontinuity on import::Track, which only the per-codec importers have today, and their discontinuity also writes a marker group into the stream. That is a moq-mux API and delivery-semantics change, and quest/m1/jitter-flush-clock.md already defers the same gap for libmoq/moq-ffi until a caller needs it. I added the pause/resume cycle next to the flushing-seek gap in that note (19d257a) so the fix lands with the rest.

(Written by Claude Opus 5.5)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 67fe3c59-e6f3-4786-99fb-ea9193dff9e6

📥 Commits

Reviewing files that changed from the base of the PR and between 051930d and 4b32bb1.

📒 Files selected for processing (8)

• doc/bin/gstreamer.md
• quest/m1/README.md
• quest/m1/gst-encoder-jitter-provenance.md
• quest/m1/jitter-flush-clock.md
• rs/moq-gst/src/sink/imp.rs
• rs/moq-gst/src/sink/pad.rs
• rs/moq-gst/src/sink/request_pad.rs
• rs/moq-gst/tests/element.rs

💤 Files with no reviewable changes (2)

• quest/m1/README.md
• quest/m1/gst-encoder-jitter-provenance.md

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

Walkthrough

GStreamer sink pads now support a default-off encoder setting for local audio and video encoders. The media path passes each buffer’s handoff time to encoder-mode writes, which require a representable timestamp and record jitter when flushing frames. Text and opaque pads reject encoder mode. The documentation and quest materials were updated to describe the setting and its timing caveat.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 4b32b

The encoder setting is opt-in, and the supplied evidence identifies no issue that needs correction before merge. The documented pause and flushing-seek limitation remains a known tradeoff.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4b32b

The change is opt-in and limited to individual audio or video pads. No new remote control or authorization path was identified, but timing state can survive a seek, and an invalid timestamp may fail a pad after decoding has begun.

Retained concerns

• Low · reliability · inferred: A flushing seek or stream restart re-anchors pad timestamps without resetting the retained importer’s jitter baseline. A timing discontinuity can therefore raise catalog jitter that remains advertised for the publication.
• Low · reliability · inferred: An out-of-range encoder timestamp is rejected after the buffer enters codec decoding and, for audio, after a cut. The failure is pad-local, but those producer operations are not rolled back; a codec path that synthesizes a missing timestamp makes partial publication possible rather than proving every invalid frame is rejected before publication.

Security review details

Security Blast Radius

• inferred — The independently affected unit is an opted-in pad and its catalog rendition. Its advertised jitter can influence downstream playout buffering; no new credential, network entrypoint, or publication authority was identified in the inspected path.

Trust Boundaries and Controls

• observed — The reservation guard fixes configured encoder provenance for the producer lifetime, and unsupported text or opaque pads reject encoder mode at CAPS rather than silently applying it.

Resilience and Maintainability Implications

• inferred — Per-pad failure and finalization limit continued publishing after an error, but neither resets the producer-preserving seek baseline nor establishes atomic publication and observation for an invalid timestamp.

Hardening Proposals

• proposed — For encoder pads, define a discontinuity operation that re-anchors importer timing without abandoning the intended track, and validate representable timestamps before producer-facing decode if invalid frames must not be published.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 4 files. (2 skipped: …	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 and concisely describes the main change: enabling opt-in flush jitter measurement for pads fed by local encoders.
Description check	✅ Passed	The description directly explains the new encoder property, its lifecycle, behavior, documentation updates, tests, and known limitation.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 4 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

---

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.}

[kixelated]

Rebased onto current main (051930d35). Kept #4123's delay rewrite in quest/m1/jitter-flush-clock.md and this PR's encoder=true opt-in, including the pause and flushing-seek gap. The completed quest is off the index; #4089's data-section removal stays. Squash auto-merge is on. Check and Test are queued.

The pause-baseline reset stays deferred. It needs a discontinuity on import::Track, which the same quest already tracks.

(Written by Grok 4.7)