Skip to content

Commit dd8ba19

Browse files
kixelatedclaude
andcommitted
fix(moq-rtc): address review — keep ts/export parsing, docs, non_exhaustive configs
- Revert the `avcc_params`/`hvcc_params` delegation: the new typed `Avcc`/`Hvcc` parsers stay for moq-rtc, but the existing pub(crate) flatteners that MPEG-TS export relies on are restored verbatim, so ts/export output is byte-identical (the delegation had silently dropped non-VPS/SPS/PPS hvcC arrays and could reorder them). Also drops a duplicated doc comment. - Add `#[non_exhaustive]` to the public `server::Config` / `client::Config` so future fields stay additive; build them via `Default` + field assignment. - Document the public `codec::{h264,vp8,vp9,opus}::Bridge` items. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 764df8a commit dd8ba19

9 files changed

Lines changed: 55 additions & 21 deletions

File tree

rs/moq-mux/src/codec/h264/mod.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,20 @@ pub(crate) fn build_avcc(sps_nals: &[Bytes], pps_nals: &[Bytes]) -> anyhow::Resu
180180
/// Extract the parameter-set NALs (SPS then PPS) and the NALU length size from
181181
/// an AVCDecoderConfigurationRecord. The inverse of [`build_avcc`]; used to
182182
/// re-emit out-of-band avc1 parameter sets as inline Annex-B (e.g. for MPEG-TS).
183-
/// The SPS+PPS NALs (in that order) and NALU length size, flattened for callers
184-
/// that re-emit every parameter set as one Annex-B prefix (e.g. MPEG-TS export).
185183
pub(crate) fn avcc_params(avcc: &[u8]) -> anyhow::Result<(usize, Vec<Bytes>)> {
186-
let avcc = Avcc::parse(avcc)?;
187-
let mut params = avcc.sps;
188-
params.extend(avcc.pps);
189-
Ok((avcc.length_size, params))
184+
anyhow::ensure!(avcc.len() >= 6, "AVCDecoderConfigurationRecord too short");
185+
let length_size = (avcc[4] & 0x03) as usize + 1;
186+
187+
let mut params = Vec::new();
188+
let num_sps = avcc[5] & 0x1f;
189+
let mut pos = read_param_set_array(avcc, 6, num_sps as usize, &mut params)?;
190+
191+
anyhow::ensure!(avcc.len() > pos, "avcC missing PPS count");
192+
let num_pps = avcc[pos];
193+
pos += 1;
194+
read_param_set_array(avcc, pos, num_pps as usize, &mut params)?;
195+
196+
Ok((length_size, params))
190197
}
191198

192199
/// Read `count` u16-length-prefixed NALs starting at `pos`, appending each to

rs/moq-mux/src/codec/h265/mod.rs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -282,15 +282,34 @@ pub(crate) fn build_hvcc(vps_nals: &[Bytes], sps_nals: &[Bytes], pps_nals: &[Byt
282282
Ok(out.freeze())
283283
}
284284

285-
/// The VPS+SPS+PPS NALs (in that order) and NALU length size, flattened for
286-
/// callers that re-emit every parameter set as one Annex-B prefix (e.g. MPEG-TS
287-
/// export).
285+
/// Extract the parameter-set NALs (VPS, SPS, PPS in array order) and the NALU
286+
/// length size from an HEVCDecoderConfigurationRecord. The inverse of
287+
/// [`build_hvcc`]; used to re-emit out-of-band hvc1 parameter sets as inline
288+
/// Annex-B (e.g. for MPEG-TS).
288289
pub(crate) fn hvcc_params(hvcc: &[u8]) -> anyhow::Result<(usize, Vec<Bytes>)> {
289-
let hvcc = Hvcc::parse(hvcc)?;
290-
let mut params = hvcc.vps;
291-
params.extend(hvcc.sps);
292-
params.extend(hvcc.pps);
293-
Ok((hvcc.length_size, params))
290+
anyhow::ensure!(hvcc.len() >= 23, "HEVCDecoderConfigurationRecord too short");
291+
let length_size = (hvcc[21] & 0x03) as usize + 1;
292+
let num_arrays = hvcc[22];
293+
294+
let mut params = Vec::new();
295+
let mut pos = 23;
296+
for _ in 0..num_arrays {
297+
// Skip the array_completeness | NAL_unit_type byte.
298+
anyhow::ensure!(hvcc.len() >= pos + 3, "truncated hvcC NAL array header");
299+
pos += 1;
300+
let num_nalus = u16::from_be_bytes([hvcc[pos], hvcc[pos + 1]]);
301+
pos += 2;
302+
for _ in 0..num_nalus {
303+
anyhow::ensure!(hvcc.len() >= pos + 2, "truncated hvcC NAL length");
304+
let len = u16::from_be_bytes([hvcc[pos], hvcc[pos + 1]]) as usize;
305+
pos += 2;
306+
anyhow::ensure!(hvcc.len() >= pos + len, "hvcC NAL exceeds buffer");
307+
params.push(Bytes::copy_from_slice(&hvcc[pos..pos + len]));
308+
pos += len;
309+
}
310+
}
311+
312+
Ok((length_size, params))
294313
}
295314

296315
/// The parameter sets carried out-of-band in an HEVCDecoderConfigurationRecord,

rs/moq-rtc/bin/moq-rtc.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,9 @@ async fn run_server(
185185
tls_key: Option<PathBuf>,
186186
direction: Direction,
187187
) -> anyhow::Result<()> {
188-
let config = moq_rtc::server::Config {
189-
ice_candidates: public_addr,
190-
udp_bind,
191-
};
188+
let mut config = moq_rtc::server::Config::default();
189+
config.ice_candidates = public_addr;
190+
config.udp_bind = udp_bind;
192191
let server = Server::new(config, publisher, subscriber);
193192

194193
let app = match direction {
@@ -209,9 +208,8 @@ async fn run_client(
209208
url: Url,
210209
direction: Direction,
211210
) -> anyhow::Result<()> {
212-
let config = moq_rtc::client::Config {
213-
ice_candidates: public_addr,
214-
};
211+
let mut config = moq_rtc::client::Config::default();
212+
config.ice_candidates = public_addr;
215213
let client = Client::new(config);
216214

217215
match direction {

rs/moq-rtc/src/client/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use url::Url;
1515

1616
/// Configuration shared by both `client publish` and `client subscribe`.
1717
#[derive(Clone, Debug, Default)]
18+
#[non_exhaustive]
1819
pub struct Config {
1920
/// Public UDP socket addresses to advertise as ICE host candidates in
2021
/// our outbound offer. Same semantics as [`crate::server::Config`].

rs/moq-rtc/src/codec/h264.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
88
use crate::{Result, codec};
99

10+
/// Feeds str0m's Annex-B H.264 access units into a moq-mux avc3 importer.
1011
pub struct Bridge {
1112
import: moq_mux::codec::h264::Import<moq_mux::catalog::hang::Extra>,
1213
}
1314

1415
impl Bridge {
16+
/// Publish an `.avc3` track on `broadcast`, registering it in `catalog`.
1517
pub fn new(broadcast: moq_net::BroadcastProducer, catalog: moq_mux::catalog::Producer) -> Result<Self> {
1618
// Pin avc3 (Annex-B, inline SPS/PPS) up front: str0m always hands us that wire shape.
1719
let import =

rs/moq-rtc/src/codec/opus.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
66
use crate::{Result, codec};
77

8+
/// Feeds str0m's Opus packets into a moq-mux Opus importer.
89
pub struct Bridge {
910
import: moq_mux::codec::opus::Import,
1011
}
1112

1213
impl Bridge {
14+
/// Publish an `.opus` track on `broadcast` at the negotiated sample rate / channel count.
1315
pub fn new(
1416
mut broadcast: moq_net::BroadcastProducer,
1517
catalog: moq_mux::catalog::Producer,

rs/moq-rtc/src/codec/vp8.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66
77
use crate::{Result, codec};
88

9+
/// Forwards str0m's VP8 frames to a `.vp8` track, detecting keyframes inline.
910
pub struct Bridge {
1011
catalog: moq_mux::catalog::Producer,
1112
track: moq_mux::container::Producer<moq_mux::catalog::hang::Container>,
1213
announced: bool,
1314
}
1415

1516
impl Bridge {
17+
/// Publish a `.vp8` track on `broadcast`; the catalog rendition is added on the first frame.
1618
pub fn new(mut broadcast: moq_net::BroadcastProducer, catalog: moq_mux::catalog::Producer) -> Result<Self> {
1719
let track = broadcast.unique_track(".vp8")?;
1820
let producer = moq_mux::container::Producer::new(track, moq_mux::catalog::hang::Container::Legacy);

rs/moq-rtc/src/codec/vp9.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
66
use crate::{Result, codec};
77

8+
/// Forwards str0m's VP9 frames to a `.vp9` track, detecting keyframes inline.
89
pub struct Bridge {
910
catalog: moq_mux::catalog::Producer,
1011
track: moq_mux::container::Producer<moq_mux::catalog::hang::Container>,
1112
announced: bool,
1213
}
1314

1415
impl Bridge {
16+
/// Publish a `.vp9` track on `broadcast`; the catalog rendition is added on the first frame.
1517
pub fn new(mut broadcast: moq_net::BroadcastProducer, catalog: moq_mux::catalog::Producer) -> Result<Self> {
1618
let track = broadcast.unique_track(".vp9")?;
1719
let producer = moq_mux::container::Producer::new(track, moq_mux::catalog::hang::Container::Legacy);

rs/moq-rtc/src/server/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ pub struct Response {
3636

3737
/// Configuration shared by both `server publish` and `server subscribe`.
3838
#[derive(Clone, Debug)]
39+
#[non_exhaustive]
3940
pub struct Config {
4041
/// Public UDP socket addresses that should be advertised as ICE host
4142
/// candidates. Each is sent as a separate `candidate` line in the SDP

0 commit comments

Comments
 (0)