fix(hls): keep a rendition when only its estimated framerate changes

[kixelated]

Problem

#4576 made the catalog estimator fill in a video rendition's framerate after about a second of media, when the publisher declared none (moq import fmp4 declares none). moq-hls still compared framerate in normalize_video, so that estimate-only republish looked like a reconfigure: renditions::Producer::sync closed the rendition and emitted Removed then Added.

Surfaced in moq-dev/moq.pro#2115: its recorder rotates a recording when a recorded rendition is removed, so every recording split after its first segment and the first recording's span stayed at one second.

Approach

Treat framerate like the other estimator fields (bitrate, jitter, delay) and clear it in normalize_video. The muxer only sizes its timescale from it, and a kept rendition keeps muxing at the timescale its init already declared. A real reconfigure still differs in codec, dimensions, or description.

estimate_only_catalog_update_keeps_the_rendition now starts with no framerate and has the estimate fill one in; it fails without the change.

Impact

• No public API or wire change.
• Live HLS: removes a one-time rendition rebuild (playlist window, media sequence, and init reset) for every importer without a declared framerate.
• DASH: a kept rendition's representation frameRate stays at the value it was built with (none, for an estimated one) until a real reconfigure.

Alternatives

• Stop estimating framerate for importers whose container carries timing: narrower, and leaves the churn for any other publisher whose framerate moves.
• Refresh the advertised framerate in place like bitrate: more code for an advisory DASH attribute; worth it only if a player needs it.

Validation

• just check passes, including the new regression test.
• moq.pro's recording-range e2e spec, which failed every run at 400b89a, passes with this patch applied to the pin.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 63d21f4a-4d71-49d2-9246-b16afc7c9f83

📥 Commits

Reviewing files that changed from the base of the PR and between e488e69 and 0e1aebe.

📒 Files selected for processing (2)

• rs/moq-hls/src/export/mod.rs
• rs/moq-hls/src/export/rendition.rs

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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]

Review: MERGE

Reviewed head: 0e1aebef3625b5bcc0baf2145205a200cd134465

Clean, targeted fix for the #4576 fallout: the catalog estimator fills framerate after ~1s when the publisher declared none (moq import fmp4), and normalize_video was still comparing it, so renditions::Producer::sync treated that as a reconfigure (Removed → Added). That is exactly what split recordings in moq.pro#2115. Clearing framerate alongside bitrate/jitter/delay matches how those advisory fields are already treated, and the muxer keeps the timescale its init already declared.

Blocking

None.

Non-blocking

1. CI not green yet on this SHA. Check / Test / Windows / macOS were still queued at review time; Auto-merge skipped. PR body says just check passes locally — worth confirming Actions before merge.
2. Doc drift (tiny). renditions.rs sync still says the publisher republishes when “measured bitrate or jitter moves,” and matches_video’s doc still names bitrate/jitter/label only. The regression test comment was updated; these two weren’t.
3. DASH @frameRate stays frozen at build time (None for an estimate-filled importer). Documented in Impact and intentional; refresh still only updates bitrate. Fine unless a player needs the advisory attribute updated in place.

Tests

estimate_only_catalog_update_keeps_the_rendition now starts with framerate = None and has the estimate fill Some(25.0) — the real hang catalog path (set_estimate only fills when supplied was none). Asserts same Arc, no cursor events. decoder_config_change_rebuilds_the_rendition still covers a real rebuild (resolution / sample-rate). Adequate for this fix.

Cross-PR

Root cause for moq.pro#2115’s recording-span flake once the moq pin includes #4576 (normalize_video at pin kept comparing framerate). Applying this on the pin is what the PR claims makes recording-range pass.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed 0e1aebe. One P2 below: avoiding rendition churn currently also freezes the fallback sample duration at the initial framerate.

Direction: keeping the rendition, playlist window, and cached init across estimator updates addresses the right cause. Separate the stable mux timebase from refreshable cadence instead of discarding cadence changes entirely.

Verification: inspected both changed files, catalog reconciliation, per-request muxer construction, missing-duration inference, and the regression assertions. Source inspection only; no local tests or moq.pro e2e rerun. CI was not verified complete.

(Written by OpenAI)

[kixelated]

[P2] Refresh the duration fallback without rebuilding the rendition

Framerate also controls sample durations: Muxer::video sets default_frame = 1 / framerate, and duration inference uses it for duration-less group tails (or all reordered samples). After an initial None becomes Some(25.0), this now keeps the old rendition, but refresh updates only bitrate and muxer() keeps using the original config. Supported Legacy/LOC groups without duration markers consequently keep getting 33.33 ms fallback durations instead of 40 ms for the rest of the broadcast; previously the replacement picked up the estimate. Preserve rendition identity and its established timescale while updating the cadence used for missing-duration synthesis, and extend the regression to inspect emitted sample durations after the estimate changes.

[kixelated]

Superseded by #4641, which fixes the same moq-hls framerate rebuild and also refreshes the framerate in place.

(Written by Claude Opus 5.5)