diff --git a/quest/m0/README.md b/quest/m0/README.md index c1b44eda82..96f7400fd5 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -92,7 +92,6 @@ do not add another media abstraction or a renderer crate during stabilization. - [E2EE API](/quest/m0/e2ee-api.md) - epoch-scoped ownership replaces raw crypto, catalog helpers, and process-global claims - [Socket group](/quest/m0/sock-group.md) - complete formation and retained sockets precede usable serving handles - [uring identity](/quest/m0/uring-identity.md) - sockets and connections carry their worker and steering identity -- [Archive ranges](/quest/m0/archive-ranges.md) - one finite inclusive range convention replaces reversed integer pairs - [Archive listing](/quest/m0/archive-listing.md) - one recording-scoped query exposes only supported listing behavior - [Vulkan/CUDA surfaces](/quest/m0/video-vulkan-cuda.md) - retain producer slots and synchronize GPU access safely across Vulkan and CUDA diff --git a/quest/m0/archive-ranges.md b/quest/m0/archive-ranges.md deleted file mode 100644 index ccfa9346a4..0000000000 --- a/quest/m0/archive-ranges.md +++ /dev/null @@ -1,34 +0,0 @@ -# [S] Archive group bounds use one inclusive range - -## Goal - -Archive callers pass group bounds without remembering an argument-order -exception. `Object::bounds`, range-named keys, reads, and validation use one -finite inclusive convention, with the persisted layout unchanged. - -## Plan - -`Object::bounds` returns `(smallest, largest)`, while `Key::groups`, -`Store::get_groups`, and `Object::{decode_groups,check_bounds}` take the -reverse order. Reuse standard Rust range notation: moq-net already accepts -`RangeBounds` in `Subscription::with_groups` and reader `set_groups`. -Archive objects require two finite bounds, so use `RangeInclusive` and -reject empty, reversed, or out-of-profile ranges at the boundary. Do not -invent another public Bounds type or expose moq-net's private normalization -helper; that helper supports unbounded subscription ranges with exclusive caps. - -Keep largest-first filename serialization private to the codec. Public -enum construction must not bypass validation when a key is serialized. -An object's returned bounds should pass directly to lookup and validation. - -Cover singleton and sparse ranges, reversed bounds, both identifier limits, -and direct key construction in the crate's CI tests. Preserve the exact -existing encoded paths and bytes. Update the examples and module docs inline; -this quest adds no recording or replay orchestration. - -Public API: breaking range arguments and return values in moq-archive 0.0.1. -Wire and persisted format: unchanged. - -## Related - -- [Archive proof](/quest/m2/archive/proof.md) - storage and replay conformance beyond the API change diff --git a/quest/m1/release.md b/quest/m1/release.md index 08cac7aa1c..1339f8c149 100644 --- a/quest/m1/release.md +++ b/quest/m1/release.md @@ -88,7 +88,6 @@ Public API: none beyond the required quests. Wire: none. - [E2EE API](/quest/m0/e2ee-api.md) - expose epoch-scoped ownership and align the implemented profile - [Socket group](/quest/m0/sock-group.md) - make partial reuseport groups and early socket drops unrepresentable - [uring identity](/quest/m0/uring-identity.md) - bind sockets, connections, workers, and steering identity together -- [Archive ranges](/quest/m0/archive-ranges.md) - use one validated inclusive range contract - [Archive listing](/quest/m0/archive-listing.md) - expose one recording-scoped listing query - [Merge dev](/quest/m1/merge-dev.md) - the tree the release is cut from - [Binding audio tests](/quest/m2/binding-audio-tests.md) - every binding proves the audio config it exposes diff --git a/rs/moq-archive/src/error.rs b/rs/moq-archive/src/error.rs index 56488380e4..2b139bd5de 100644 --- a/rs/moq-archive/src/error.rs +++ b/rs/moq-archive/src/error.rs @@ -38,8 +38,8 @@ pub enum Error { #[error("group sequences are not strictly ascending")] Sequence, - /// A range object's table does not match its filename bounds. - #[error("table bounds {smallest}..={largest} do not match the key")] + /// A group range is empty, reversed, or does not match an object's table. + #[error("invalid or mismatched group bounds {smallest}..={largest}")] Bounds { smallest: u64, largest: u64 }, /// The binary table is truncated, overlapping, gapped, or out of range. diff --git a/rs/moq-archive/src/lib.rs b/rs/moq-archive/src/lib.rs index fec73a76a6..7838bd30c4 100644 --- a/rs/moq-archive/src/lib.rs +++ b/rs/moq-archive/src/lib.rs @@ -3,6 +3,13 @@ //! The crate owns the portable layout and codecs: percent-encoded track names, `.info` JSON, //! the binary segment envelope, and put/get/list/delete. Callers that need runtime dispatch //! supply `Arc`; the archive API itself stays generic. +//! +//! Group bounds are finite inclusive ranges in first-to-last order: +//! ``` +//! let key = moq_archive::Key::groups("video", 5..=7)?; +//! assert_eq!(key.track(), "video"); +//! # Ok::<(), moq_archive::Error>(()) +//! ``` pub use object_store; diff --git a/rs/moq-archive/src/path.rs b/rs/moq-archive/src/path.rs index 63f9400137..40253d15a1 100644 --- a/rs/moq-archive/src/path.rs +++ b/rs/moq-archive/src/path.rs @@ -1,3 +1,5 @@ +use std::ops::RangeInclusive; + use object_store::path::{Path, PathPart}; use percent_encoding::{AsciiSet, NON_ALPHANUMERIC, percent_decode_str, utf8_percent_encode}; @@ -58,10 +60,8 @@ pub enum Key { Groups { /// The unencoded track name. track: String, - /// Inclusive last group sequence in the object. - largest: u64, - /// Inclusive first group sequence in the object. - smallest: u64, + /// Inclusive group sequences in the object, from first to last. + range: RangeInclusive, }, /// `/segments/` Segments { @@ -80,20 +80,12 @@ impl Key { Ok(Self::Info { track }) } - /// A range-named groups object. `largest` is the last sequence, `smallest` the first. - pub fn groups(track: impl Into, largest: u64, smallest: u64) -> Result { + /// A groups object named by its inclusive first-to-last sequence range. + pub fn groups(track: impl Into, range: RangeInclusive) -> Result { let track = track.into(); encode_track(&track)?; - check_id(largest)?; - check_id(smallest)?; - if largest < smallest { - return Err(Error::Bounds { smallest, largest }); - } - Ok(Self::Groups { - track, - largest, - smallest, - }) + check_range(&range)?; + Ok(Self::Groups { track, range }) } /// A timeline object at `segments/`. @@ -116,9 +108,9 @@ impl Key { let path = push(prefix, &encode_track(self.track())?)?; match self { Self::Info { .. } => push(&path, ".info"), - Self::Groups { largest, smallest, .. } => { + Self::Groups { range, .. } => { let path = push(&path, "groups")?; - push(&path, &range_name(*largest, *smallest)?) + push(&path, &range_name(range)?) } Self::Segments { segment, .. } => { let path = push(&path, "segments")?; @@ -155,12 +147,8 @@ fn parse_parts<'a>(mut parts: impl Iterator>, location: &Pat if parts.next().is_some() { return Err(Error::Path(location.to_string())); } - let (largest, smallest) = parse_range(name.as_ref())?; - Ok(Key::Groups { - track, - largest, - smallest, - }) + let range = parse_range(name.as_ref())?; + Ok(Key::Groups { track, range }) } "segments" => { let name = parts.next().ok_or_else(|| Error::Path(location.to_string()))?; @@ -199,11 +187,13 @@ pub(crate) fn push(base: &Path, segment: &str) -> Result { Ok(base.clone().join(part)) } -fn range_name(largest: u64, smallest: u64) -> Result { +fn range_name(range: &RangeInclusive) -> Result { + check_range(range)?; + let (smallest, largest) = (*range.start(), *range.end()); Ok(format!("{}.{}", format_id(largest)?, format_id(smallest)?)) } -fn parse_range(name: &str) -> Result<(u64, u64)> { +fn parse_range(name: &str) -> Result> { let (largest, smallest) = name.split_once('.').ok_or_else(|| Error::Path(name.to_string()))?; if smallest.contains('.') { return Err(Error::Path(name.to_string())); @@ -213,7 +203,17 @@ fn parse_range(name: &str) -> Result<(u64, u64)> { if largest < smallest { return Err(Error::Bounds { smallest, largest }); } - Ok((largest, smallest)) + Ok(smallest..=largest) +} + +pub(crate) fn check_range(range: &RangeInclusive) -> Result<()> { + let (smallest, largest) = (*range.start(), *range.end()); + check_id(smallest)?; + check_id(largest)?; + if range.is_empty() { + return Err(Error::Bounds { smallest, largest }); + } + Ok(()) } #[cfg(test)] @@ -282,7 +282,7 @@ mod tests { let prefix = Path::from("rec/1"); for key in [ Key::info("catalog.json").unwrap(), - Key::groups("video", 10, 5).unwrap(), + Key::groups("video", 5..=10).unwrap(), Key::segments("timeline.z", 0).unwrap(), Key::segments("timeline.z", ID_MAX).unwrap(), ] { @@ -299,13 +299,54 @@ mod tests { #[test] fn inverted_range_is_rejected() { + let (smallest, largest) = (2, 1); + assert!(matches!( + Key::groups("v", smallest..=largest), + Err(Error::Bounds { + smallest: 2, + largest: 1 + }) + )); + } + + #[test] + fn exhausted_range_is_rejected() { + let mut range = 1..=1; + assert_eq!(range.next(), Some(1)); + assert!(matches!(Key::groups("v", range), Err(Error::Bounds { .. }))); + } + + #[test] + fn range_id_endpoints_are_valid() { + let key = Key::groups("v", 0..=ID_MAX).unwrap(); + assert_eq!( + key.path(&Path::from("rec")).unwrap().as_ref(), + "rec/v/groups/0009007199254740991.0000000000000000000" + ); + assert!(matches!(Key::groups("v", 0..=ID_MAX + 1), Err(Error::Id(_)))); + assert!(matches!(Key::groups("v", ID_MAX + 1..=ID_MAX + 1), Err(Error::Id(_)))); + } + + #[test] + fn direct_inverted_range_is_rejected_when_serialized() { + let (smallest, largest) = (2, 1); + let key = Key::Groups { + track: "v".to_string(), + range: smallest..=largest, + }; assert!(matches!( - Key::groups("v", 1, 2), + key.path(&Path::from("rec")), Err(Error::Bounds { smallest: 2, largest: 1 }) )); + + let key = Key::Groups { + track: "v".to_string(), + range: 0..=ID_MAX + 1, + }; + assert!(matches!(key.path(&Path::from("rec")), Err(Error::Id(_)))); } #[test] diff --git a/rs/moq-archive/src/segment.rs b/rs/moq-archive/src/segment.rs index 3fc981333c..c2fd68c5aa 100644 --- a/rs/moq-archive/src/segment.rs +++ b/rs/moq-archive/src/segment.rs @@ -1,7 +1,9 @@ +use std::ops::RangeInclusive; + use bytes::{Buf, BufMut, Bytes, BytesMut}; use moq_net::VarInt; -use crate::path::check_id; +use crate::path::{check_id, check_range}; use crate::{Error, Result, VERSION}; /// One complete group in a segment object, in sequence order. @@ -30,10 +32,10 @@ pub struct Object { } impl Object { - /// Inclusive first and last group sequences. - pub fn bounds(&self) -> Result<(u64, u64)> { + /// Inclusive group sequences from first to last. + pub fn bounds(&self) -> Result> { validate(&self.groups)?; - Ok((self.groups[0].sequence, self.groups.last().unwrap().sequence)) + Ok(self.groups[0].sequence..=self.groups.last().unwrap().sequence) } /// Encode the binary envelope. The first sequence is absolute; later ones are `current - previous - 1`. @@ -158,20 +160,22 @@ impl Object { Ok(Self { groups }) } - /// Decode and require the table's first and last sequences to match `smallest` and `largest`. - pub fn decode_groups(buf: impl Buf, largest: u64, smallest: u64) -> Result { + /// Decode and require the table's sequences to match `range`. + pub fn decode_groups(buf: impl Buf, range: RangeInclusive) -> Result { + check_range(&range)?; let object = Self::decode(buf)?; - object.check_bounds(largest, smallest)?; + object.check_bounds(range)?; Ok(object) } /// Require this object's sequences to match a range-named key. - pub fn check_bounds(&self, largest: u64, smallest: u64) -> Result<()> { - let (got_smallest, got_largest) = self.bounds()?; - if got_smallest != smallest || got_largest != largest { + pub fn check_bounds(&self, range: RangeInclusive) -> Result<()> { + check_range(&range)?; + let got = self.bounds()?; + if got != range { return Err(Error::Bounds { - smallest: got_smallest, - largest: got_largest, + smallest: *got.start(), + largest: *got.end(), }); } Ok(()) @@ -250,7 +254,8 @@ mod tests { ]); let bytes = original.encode().unwrap(); assert_eq!(Object::decode(&bytes[..]).unwrap(), original); - original.check_bounds(4, 0).unwrap(); + assert_eq!(original.bounds().unwrap(), 0..=4); + original.check_bounds(0..=4).unwrap(); } #[test] @@ -421,14 +426,22 @@ mod tests { }, ]); let bytes = object.encode().unwrap(); - assert!(Object::decode_groups(&bytes[..], 7, 5).is_ok()); + assert!(Object::decode_groups(&bytes[..], 5..=7).is_ok()); assert!(matches!( - Object::decode_groups(&bytes[..], 7, 6), + Object::decode_groups(&bytes[..], 6..=7), Err(Error::Bounds { smallest: 5, largest: 7 }) )); + let (smallest, largest) = (7, 5); + assert!(matches!( + Object::decode_groups(&bytes[..], smallest..=largest), + Err(Error::Bounds { + smallest: 7, + largest: 5 + }) + )); } #[test] diff --git a/rs/moq-archive/src/store.rs b/rs/moq-archive/src/store.rs index 72227e40c2..12d4a8d8ba 100644 --- a/rs/moq-archive/src/store.rs +++ b/rs/moq-archive/src/store.rs @@ -1,3 +1,5 @@ +use std::ops::RangeInclusive; + use bytes::Bytes; use futures::StreamExt; use futures::stream::BoxStream; @@ -120,16 +122,15 @@ impl Store { /// Create a range-named groups object. A collision is accepted only when the bytes match. pub async fn put_groups(&self, track: &str, object: &Object) -> Result { - let (smallest, largest) = object.bounds()?; - let key = Key::groups(track, largest, smallest)?; + let key = Key::groups(track, object.bounds()?)?; self.put_segment(&key, object.encode()?).await?; Ok(key) } /// Fetch a groups object and require its table to match the filename bounds. - pub async fn get_groups(&self, track: &str, largest: u64, smallest: u64) -> Result { - let path = self.path(&Key::groups(track, largest, smallest)?)?; - Object::decode_groups(self.get_bytes(&path).await?, largest, smallest) + pub async fn get_groups(&self, track: &str, range: RangeInclusive) -> Result { + let path = self.path(&Key::groups(track, range.clone())?)?; + Object::decode_groups(self.get_bytes(&path).await?, range) } /// Create a timeline object at `segments/`. A collision is accepted only when the bytes match. @@ -349,13 +350,14 @@ mod tests { async fn groups_put_get_and_identical_collision() { let store = memory(); let object = two_groups(); + let range = object.bounds().unwrap(); let key = store.put_groups("video", &object).await.unwrap(); - assert_eq!(key, Key::groups("video", 7, 5).unwrap()); + assert_eq!(key, Key::groups("video", range.clone()).unwrap()); assert_eq!( store.path(&key).unwrap().as_ref(), "rec/video/groups/0000000000000000007.0000000000000000005" ); - assert_eq!(store.get_groups("video", 7, 5).await.unwrap(), object); + assert_eq!(store.get_groups("video", range).await.unwrap(), object); store.put_groups("video", &object).await.unwrap(); } @@ -410,8 +412,8 @@ mod tests { vec![ Key::segments("timeline.z", 2).unwrap(), Key::info("video").unwrap(), - Key::groups("video", 1, 1).unwrap(), - Key::groups("video", 3, 3).unwrap(), + Key::groups("video", 1..=1).unwrap(), + Key::groups("video", 3..=3).unwrap(), ] ); } @@ -457,7 +459,7 @@ mod tests { let object = one_group(ID_MAX, b"z"); store.put_groups("v", &object).await.unwrap(); store.put_segments("t", ID_MAX, &object).await.unwrap(); - assert_eq!(store.get_groups("v", ID_MAX, ID_MAX).await.unwrap(), object); + assert_eq!(store.get_groups("v", ID_MAX..=ID_MAX).await.unwrap(), object); assert_eq!(store.get_segments("t", ID_MAX).await.unwrap(), object); } @@ -471,6 +473,6 @@ mod tests { let object = two_groups(); store.put_groups("audio", &object).await.unwrap(); assert_eq!(store.get_info("audio").await.unwrap(), info); - assert_eq!(store.get_groups("audio", 7, 5).await.unwrap(), object); + assert_eq!(store.get_groups("audio", 5..=7).await.unwrap(), object); } }