feat(cli): record and replay archives with export archive / import archive - #4113
Conversation
… archive` `export archive <store-url>` records one broadcast through moq_archive::Writer, enrolling catalog renditions as they appear. `import archive <store-url>` republishes a recording through moq_archive::Reader, optionally following a growing archive with `--follow`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
MERGE Positive improvement: yes. This closes the archive CLI quest (#2281) by wiring Worth the complexity: yes. The new surface is mostly thin endpoint wiring plus a small catalog diff ( Different approach: no. Recording only the catalog encoding the export is reading is the right default (not every publisher serves all three). Ending on a returning rendition is better than silently dropping its new content. Making Nits (non-blocking):
Recommendation: merge once CI is green. This is an automated review, not the maintainer's decision |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47f59984fe
ℹ️ 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".
| for change in tracks.update(&snapshot)? { | ||
| match change { | ||
| Change::Pacing(name) => control.pacing_track(&name).await?, | ||
| Change::Track(name) => control.track(&name).await?, |
There was a problem hiding this comment.
When the initial catalog contains multiple already-live renditions, each pacing_track(...).await waits for its remote subscription before the next track is enrolled, while writer.run() concurrently drains earlier tracks. A fast or backlogged first rendition can therefore commit segments before later renditions are registered, permanently omitting their groups from those timeline records even though the recorded catalog advertises them. Reserve or batch all changes from a snapshot before allowing the writer to commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Each catalog snapshot now enrolls under Control::reserve, the segmenter's existing hold, so a commit waits until every track in that snapshot is registered. an_opening_snapshot_records_every_rendition writes two renditions before export starts and checks that segment 0 names both. Without the hold it records only audio and catalog.json.
(Written by Grok 4.7)
| changes.push(match pacing { | ||
| true => Change::Pacing(name.clone()), | ||
| false => Change::Track(name.clone()), |
There was a problem hiding this comment.
When a valid catalog contains only text, JSON, or binary tracks, every content track is enrolled as non-pacing. moq-mux deliberately produces no timeline for non-pacing tracks alone, even when finishing, so this export reports success but none of those groups, including the catalog, are indexed and the subsequent import serves an empty recording. Give such broadcasts a pacing or explicit-cut policy, or reject them before recording rather than silently losing the content.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this. A text, JSON, or binary-only catalog still enrolls those tracks as non-pacing, and moq-mux will not open a timeline without a pacing track, so the export can succeed with nothing indexed. Pacing them would stall the recording whenever a sparse track goes quiet, which is why they are non-pacing. Rejecting the first snapshot would also refuse a broadcast that adds a rendition later. An explicit cut needs a boundary the catalog does not carry. That is a recording-policy choice, not something to guess at while landing this.
(Written by Grok 4.7)
`export archive` enrolled each rendition while the writer was already committing. A buffered opening rendition could close a segment before the rest of that snapshot was registered, so those groups never entered the record. `Control::reserve` withholds commits until the snapshot's tracks are enrolled. Also sort the workspace `moq-archive` dependency, which failed `cargo sort`. Co-Authored-By: Grok 4.7 <noreply@x.ai>
|
Check failed Also held segment commits across each catalog snapshot ( Left the data-only catalog case. Non-pacing tracks alone still publish no timeline, and reject versus pace versus cut is a recording policy choice. (Written by Grok 4.7) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 966e7fa797
ℹ️ 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".
| tokio::select! { | ||
| result = writer.run() => result.context("recording failed"), | ||
| Err(err) = enroll(control, catalog) => Err(err), | ||
| } |
There was a problem hiding this comment.
When the source closes while enroll is still awaiting a rendition subscription or .info PUT, such as for a short or already-finished broadcast using a remote store, writer.run() can drain the catalog track and resolve Ok(()) first. This select! then cancels enroll and reports a successful export even though the remaining renditions were never registered or recorded. The new reservation only gates segment commits and does not keep the writer alive, which is fresh evidence beyond the prior snapshot-batching finding; make successful completion conditional on enrollment finishing. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
| let credentials = std::env::vars_os() | ||
| .filter_map(|(key, value)| Some((key.into_string().ok()?, value.into_string().ok()?))) | ||
| .filter(|(key, _)| ["AWS_", "GOOGLE_", "AZURE_"].iter().any(|cloud| key.starts_with(cloud))); | ||
| let (store, prefix) = | ||
| object_store::parse_url_opts(url, credentials).with_context(|| format!("unsupported archive store {url}"))?; |
There was a problem hiding this comment.
When the process has unrelated cloud variables, for example an S3 URL in an environment that also defines GOOGLE_APPLICATION_CREDENTIALS, this iterator passes every AWS_*, GOOGLE_*, and AZURE_* pair to the selected backend. parse_url_opts parses each supplied option as that backend's configuration key, so a foreign or otherwise unrelated prefixed variable is rejected as an unknown key and prevents a valid archive store from opening. Filter to recognized options for the URL's backend instead of forwarding all three namespaces. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| let refresh = async { | ||
| loop { | ||
| tokio::time::sleep(interval).await; | ||
| reader.refresh().await?; |
There was a problem hiding this comment.
With --follow 0s, the duration parser accepts zero and tokio::time::sleep(interval) completes every iteration, causing an unbounded loop of object-store LIST requests that consumes CPU and API quota and may throttle the replay. Reject zero before entering follow mode rather than accepting this malformed polling interval. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
1 participant
Problem
Recording or replaying a broadcast with
moq-archiveneeded custom code.moq-clihad no endpoint for it.Approach
export archive <store-url>records one broadcast throughmoq_archive::Writer. It reads the broadcast's own catalog (format from the suffix or--catalog-format): video and audio renditions enroll as pacing tracks, and the catalog track plus text, JSON, and binary tracks enroll as non-pacing. Renditions added later enroll as they appear. A dropped rendition is removed so it stops pacing. The stage ends when the broadcast ends.Control::reserve, so one rendition cannot close a segment before the rest of that snapshot is registered.--retention <dur>/--retention-grace <dur>(default 30s, needs--retention) map towriter::Retention.import archive <store-url>opensmoq_archive::Readeron a fresh broadcast, announces it, and serves groups on demand. By default it replays what is stored and finishes the timeline.--follow <interval>keeps callingReader::refreshfor a growing recording.object_store::parse_url_opts, withAWS_*/GOOGLE_*/AZURE_*env vars passed as options.parse_urlalone ignores credentials. New default-on cargo featuress3,gcs,azure;file://is always available.export archive,--max-ageonimport archive, and--retention-gracewithout--retention.doc/bin/cli.mdhas an Archive section, and the multi-stage example now records. Examples were checked against--help.export archiveinto a tempdirfile://store, thenimport archiveand a FETCH of every group, checking frames and timestamps. Unit tests cover the catalog diffing.an_opening_snapshot_records_every_renditionwrites two renditions before export starts and checks that segment 0 names both.Impact
moq ... export archive <url>andmoq ... import archive <url>endpoints, with the flags above.s3,gcs,azure, all on by default.moq_archive::writer::Control::reserve()returns aReservationthat holds segment commits until dropped. Additive, on the unpublished 0.0.x crate. No wire changes.Alternatives
catalog.json,catalog.json.z,catalog) would need a presence check, because not every publisher serves all three.--followis opt-in because the store has no end marker, so a following reader never ends its timeline.parse_urlexactly as the quest says would ignore cloud credentials, so this usesparse_url_optswith the cloud env vars.Follow-ups
export ts, web watch) can't play it until the DVR/HLS quests land. The replayed catalog carries whateverarchiveentry the source had, with noreplayorstorefields.Closes #2281
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code