From b6254e61c4741c26d2f614395ae5b1f8f4363f0c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 5 Oct 2026 13:34:22 -0700 Subject: [PATCH 1/2] chore: claim quest/m1/catalog-omit-empty Co-Authored-By: Grok 4.7 From b4ab22eeadbde429714e0f999d76ee96337e0879 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 5 Oct 2026 13:46:03 -0700 Subject: [PATCH 2/2] fix(hang): omit empty video and audio catalog sections Co-Authored-By: Grok 4.7 --- quest/m1/README.md | 1 - quest/m1/catalog-omit-empty.md | 23 ----------- rs/hang/src/catalog/audio/mod.rs | 5 +++ rs/hang/src/catalog/root.rs | 61 ++++++++++++++++++++++-------- rs/hang/src/catalog/video/mod.rs | 7 ++++ rs/moq-mux/src/catalog/hang/ext.rs | 6 +++ 6 files changed, 64 insertions(+), 39 deletions(-) delete mode 100644 quest/m1/catalog-omit-empty.md diff --git a/quest/m1/README.md b/quest/m1/README.md index a6e6e1a47e..7702a9f298 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -80,7 +80,6 @@ blocks. The quests that gated m0 lines moved under them. - [Plan: untimed verbatim PES](/quest/m1/plan-ts-pes-untimed.md) - decide how a verbatim TS track carries a PES that has no PTS, then write the implementation quest - [Data consumer timestamps](/quest/m1/data-consumer-timestamps.md) - json and binary consumers return each value's timestamp, in Rust and every binding; snapshots add `latest()` beside an in-order `next()` - [JS data consumer timestamps](/quest/m1/js-data-consumer-timestamps.md) - @moq/json and @moq/flate consumers return each value's timestamp, with snapshot `next()` and `latest()` -- [Omit empty catalog sections](/quest/m1/catalog-omit-empty.md) - a Rust catalog with no video or audio leaves those keys out, as JS does - [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) - a draft-20+ FETCH within one group is served from its LOCATION_FILTER, as older drafts are - [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - our FETCH_OK carries the track properties SUBSCRIBE_OK does, as draft 16+ requires - [Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md) - a relay fetches from an IETF upstream without subscribing, finished tracks included, with End of Track always reported diff --git a/quest/m1/catalog-omit-empty.md b/quest/m1/catalog-omit-empty.md deleted file mode 100644 index 93818c6114..0000000000 --- a/quest/m1/catalog-omit-empty.md +++ /dev/null @@ -1,23 +0,0 @@ -# [XS] The hang catalog omits empty video and audio sections - -## Goal - -A Rust-published hang catalog with no video or audio renditions leaves out the -`video` or `audio` key, as the JS publisher already does, instead of writing -`"video":{"renditions":{}},"audio":{"renditions":{}}`. - -## Plan - -Requested by an external consumer (OneTooMany, Discord): data-only broadcasts -fill logs with misleading empty sections. - -Give `Video` and `Audio` an `is_empty` (no renditions), as `Text` has, and add -`skip_serializing_if` on `video` and `audio` in -`rs/hang/src/catalog/root.rs`, like `text`/`json`/`binary`, and update the -tests that pin the empty output (`root.rs` and -`rs/moq-mux/src/catalog/hang/ext.rs`). - -Wire-compatible, so it lands on `main`: Rust already defaults an absent -section on read, and `@moq/hang` has accepted an absent one since #401 -(2025-06). The draft already marks both optional; decided to leave its wording -unchanged. diff --git a/rs/hang/src/catalog/audio/mod.rs b/rs/hang/src/catalog/audio/mod.rs index 7c013c94a9..6c32bdf0c3 100644 --- a/rs/hang/src/catalog/audio/mod.rs +++ b/rs/hang/src/catalog/audio/mod.rs @@ -49,6 +49,11 @@ impl Audio { pub fn remove(&mut self, name: &str) -> Option { self.renditions.remove(name) } + + /// True when there are no renditions, so the section can be omitted from the catalog. + pub fn is_empty(&self) -> bool { + self.renditions.is_empty() + } } /// Audio decoder configuration based on WebCodecs AudioDecoderConfig. diff --git a/rs/hang/src/catalog/root.rs b/rs/hang/src/catalog/root.rs index 85ed58b84d..0c68fa32c3 100644 --- a/rs/hang/src/catalog/root.rs +++ b/rs/hang/src/catalog/root.rs @@ -31,15 +31,17 @@ pub struct Catalog { /// Video track information with multiple renditions. /// /// Contains a map of video track renditions that the viewer can choose from - /// based on their preferences (resolution, bitrate, codec, etc). - #[serde(default)] + /// based on their preferences (resolution, bitrate, codec, etc). Omitted from the + /// wire when it has no renditions. + #[serde(default, skip_serializing_if = "Video::is_empty")] pub video: Video, /// Audio track information with multiple renditions. /// /// Contains a map of audio track renditions that the viewer can choose from - /// based on their preferences (codec, bitrate, language, etc). - #[serde(default)] + /// based on their preferences (codec, bitrate, language, etc). Omitted from the + /// wire when it has no renditions. + #[serde(default, skip_serializing_if = "Audio::is_empty")] pub audio: Audio, /// Each track's timeline and any durable archive, if the publisher offers them. @@ -540,13 +542,42 @@ mod test { #[test] fn empty_text_section_omitted() { - // A catalog without captions must stay byte-identical to before the text section existed: - // the empty section is skipped, unlike the always-present video/audio sections. + // A catalog without captions omits the text section, as it does for empty video and audio. let catalog = Catalog::<()>::default(); let output = catalog.to_json().expect("failed to encode"); assert!(!output.contains("text"), "empty text section leaked: {output}"); } + #[test] + fn empty_video_and_audio_sections_omitted() { + // A data-only broadcast must not advertise media it does not have. An old publisher's + // empty objects still decode, and a republish drops them. + assert_eq!(Catalog::<()>::default().to_json().expect("failed to encode"), "{}"); + + let mut audio_only = Catalog::<()>::default(); + audio_only + .audio + .insert("audio", AudioConfig::new(Opus, 48_000, 2)) + .unwrap(); + let output = audio_only.to_json().expect("failed to encode"); + assert!(output.contains("\"audio\""), "audio section missing: {output}"); + assert!(!output.contains("\"video\""), "empty video section leaked: {output}"); + + let mut video_only = Catalog::<()>::default(); + video_only.video.display = Some(crate::catalog::Display { width: 16, height: 9 }); + assert_eq!( + video_only.to_json().expect("failed to encode"), + "{}", + "display without a rendition must not keep an empty video section" + ); + + let legacy = Catalog::<()>::from_str(r#"{"video":{"renditions":{}},"audio":{"renditions":{}}}"#) + .expect("failed to decode"); + assert!(legacy.video.is_empty()); + assert!(legacy.audio.is_empty()); + assert_eq!(legacy.to_json().expect("failed to encode"), "{}"); + } + #[test] fn text_section_roundtrip() { use crate::catalog::{Text, TextConfig, TextFormat, TextRole}; @@ -612,6 +643,8 @@ mod test { encoded.contains(r#""role":"commentary""#), "unknown role was not preserved: {encoded}" ); + assert!(!encoded.contains("\"video\""), "empty video section leaked: {encoded}"); + assert!(!encoded.contains("\"audio\""), "empty audio section leaked: {encoded}"); } /// A section name is only reserved from the version that defines it, and `json` and `binary` are @@ -644,10 +677,9 @@ mod test { #[test] fn data_sections_stay_off_the_wire_when_empty() { - // A media-only catalog must serialize exactly as it did before the data sections existed, - // or every existing publisher's bytes change. + // Empty sections stay off the wire, video and audio included. let output = Catalog::<()>::default().to_json().expect("failed to encode"); - assert_eq!(output, r#"{"video":{"renditions":{}},"audio":{"renditions":{}}}"#); + assert_eq!(output, "{}"); } #[test] @@ -669,9 +701,9 @@ mod test { #[test] fn clock_stays_off_the_wire_when_absent() { - // A catalog without a clock serializes exactly as before the section existed. + // Absent clock stays off the wire. Empty video and audio are omitted as well. let output = Catalog::<()>::default().to_json().expect("failed to encode"); - assert_eq!(output, r#"{"video":{"renditions":{}},"audio":{"renditions":{}}}"#); + assert_eq!(output, "{}"); } #[test] @@ -740,8 +772,6 @@ mod test { #[test] fn data_tracks_roundtrip() { let mut encoded = r#"{ - "video": {"renditions": {}}, - "audio": {"renditions": {}}, "json": { "tracks": { "chat": { @@ -793,7 +823,7 @@ mod test { /// media. #[test] fn data_track_bitrate_and_jitter() { - let encoded = r#"{"video":{"renditions":{}},"audio":{"renditions":{}},"json":{"tracks":{"gps":{"mode":"stream","bitrate":8000,"jitter":100,"delay":250}}},"binary":{"tracks":{"frames":{"mode":"snapshot","bitrate":64000,"jitter":34}}}}"#; + let encoded = r#"{"json":{"tracks":{"gps":{"mode":"stream","bitrate":8000,"jitter":100,"delay":250}}},"binary":{"tracks":{"frames":{"mode":"snapshot","bitrate":64000,"jitter":34}}}}"#; let mut gps = JsonConfig::new(Mode::Stream); gps.bitrate = Some(8_000); @@ -842,7 +872,8 @@ mod test { mavlink: BTreeMap, } - let encoded = r#"{"video":{"renditions":{}},"audio":{"renditions":{}},"com.example.mavlink":{"telemetry":{"config":{"mode":"stream","compression":"deflate"},"sysid":1}}}"#; + let encoded = + r#"{"com.example.mavlink":{"telemetry":{"config":{"mode":"stream","compression":"deflate"},"sysid":1}}}"#; let catalog = Catalog::::from_str(encoded).unwrap(); let entry = &catalog.ext.mavlink["telemetry"]; diff --git a/rs/hang/src/catalog/video/mod.rs b/rs/hang/src/catalog/video/mod.rs index f4bba68565..5d8b796464 100644 --- a/rs/hang/src/catalog/video/mod.rs +++ b/rs/hang/src/catalog/video/mod.rs @@ -106,6 +106,13 @@ impl Video { self.renditions.remove(name) } + /// True when there are no renditions, so the section can be omitted from the catalog. + /// + /// Display, rotation, and flip ride this section, so they are omitted too when nothing is published. + pub fn is_empty(&self) -> bool { + self.renditions.is_empty() + } + /// Iterate the renditions best first: largest picture, then highest bitrate. /// /// A consumer that carries one rendition takes the first it supports, so the diff --git a/rs/moq-mux/src/catalog/hang/ext.rs b/rs/moq-mux/src/catalog/hang/ext.rs index 86ce7f36c1..90957aa130 100644 --- a/rs/moq-mux/src/catalog/hang/ext.rs +++ b/rs/moq-mux/src/catalog/hang/ext.rs @@ -141,7 +141,13 @@ mod test { let catalog: Catalog = serde_json::from_str(json).expect("legacy text section broke the catalog"); assert!(catalog.text.is_empty()); + assert!(catalog.video.is_empty()); + assert!(catalog.audio.is_empty()); assert!(catalog.ext.get("scte35").is_some(), "unrelated sections still decode"); + + // Republishing drops the empty media sections the old payload carried. + let output = serde_json::to_string(&catalog).expect("failed to encode"); + assert_eq!(output, r#"{"scte35":{"spliceId":7}}"#); } #[test]