Add MSF catalog format support with format negotiation#1330
Conversation
Adds a `catalog` signal (and matching `catalog` attribute on <moq-watch>) that selects between hang (`catalog.json`) and msf (`catalog`) catalog formats. Default is `hang`. - New `@moq/msf` package with Zod schemas for the MSF catalog format - MSF→Hang converter in `@moq/watch/msf.ts` (parses CMAF init segments via `Cmaf.decodeInitSegment` to extract timescale/trackId) - `Broadcast` subscribes to both tracks when catalog is `auto`; hang gets a 100ms headstart and the loser track is closed https://claude.ai/code/session_012gjc9wFMcfGj83fazaJ5ij
- `catalogFormats: Signal<CatalogFormat[]>` → `catalogFormat: Signal<CatalogFormat>` - Drop hang/msf race and HANG_HEADSTART_MS; subscribe to one track based on the configured format - Drop "auto" attribute value; the app picks hang or msf explicitly - `@moq/msf` now uses `zod/mini` to match the other packages - `@moq/watch` no longer depends on zod directly https://claude.ai/code/session_012gjc9wFMcfGj83fazaJ5ij
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new package at js/msf with package.json, tsconfig, README, and src files: a Zod-based MSF catalog schema (src/catalog.ts) with encode/decode/fetch helpers and src/index.ts re-export. Registers the workspace and adds the package as a dependency of js/watch. Extends js/watch to support selectable catalog formats ("hang" | "msf"), exposes a catalogFormat signal and element attribute/accessors, updates catalog fetching to handle both formats, and adds a converter (js/watch/src/msf.ts) that maps MSF catalogs into the existing hang catalog shape. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
js/watch/src/element.ts (1)
259-268: Avoid exposing the mutable catalog format array.
catalogreturns and stores theSignalarray by reference, so callers can mutate it without triggeringSignal.set(); e.g.el.catalog.push("msf")changes internal state but may not restart negotiation. Return/store a copy, and consider validating array values before setting.Proposed defensive copy
get catalog(): CatalogFormat[] { - return this.broadcast.catalogFormats.peek(); + return [...this.broadcast.catalogFormats.peek()]; } set catalog(value: CatalogAttr | CatalogFormat[]) { if (typeof value === "string") { this.broadcast.catalogFormats.set(parseCatalogAttr(value)); } else { - this.broadcast.catalogFormats.set(value); + this.broadcast.catalogFormats.set([...value]); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/watch/src/element.ts` around lines 259 - 268, The catalog getter currently returns the internal Signal array by reference (via broadcast.catalogFormats.peek()), and the setter stores arrays directly, allowing external mutation to bypass Signal.set(); change the getter (catalog) to return a shallow copy of the array so callers cannot mutate internal state, and update the setter to validate/normalize entries (when given CatalogAttr or CatalogFormat[]) and pass a new array instance to broadcast.catalogFormats.set() (use parseCatalogAttr for string inputs, then validate each CatalogFormat item) so all mutations go through Signal.set() and invalid values are rejected.js/msf/src/catalog.ts (1)
4-61: Document the new public MSF API surface.These exported schemas and helpers define the
@moq/msfpackage contract. Please add brief comments for expected catalog shape,decodethrowing behavior, andfetchreturningundefinedwhen the track closes without a frame. As per coding guidelines, “Document public APIs with clear docstrings or comments.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/msf/src/catalog.ts` around lines 4 - 61, Add brief doc comments for the public MSF API exports to document the package contract: annotate PackagingSchema and RoleSchema with allowed values summary, describe TrackSchema/CatalogSchema shape and the expected catalog version=1 structure, and add docstrings for encode (serializes Catalog to Uint8Array), decode (parses JSON and documents that it will throw on invalid data or malformed JSON), and fetch (documents it reads one frame from Moq.Track, decodes it, and returns undefined if the track closes without a frame). Use the exported symbol names PackagingSchema, RoleSchema, TrackSchema, CatalogSchema, encode, decode, and fetch so the comments sit directly above those declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/msf/src/catalog.ts`:
- Around line 45-55: The decode function currently uses TextDecoder in
replacement mode and logs the entire decoded string; change it to create the
TextDecoder with { fatal: true } so invalid UTF-8 throws before JSON.parse
(refer to decode and TextDecoder usage), and update the catch block around
JSON.parse / CatalogSchema.parse to avoid printing the full str — log only a
minimal safe message (e.g., "invalid MSF catalog" plus a small fixed-size
hex/byte-length or prefix) or rethrow the error without the full payload to
prevent exposing large or sensitive initData.
- Around line 20-28: TrackSchema currently allows negative, fractional and
unsafe numeric values and accepts any string for initData; update TrackSchema so
numeric fields (width, height, framerate, samplerate, bitrate, renderGroup,
altGroup) use integer, non-negative, 53-bit-safe validation (e.g.
z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER)) to prevent
fractional/unsafe values being passed to u53(), and tighten initData to require
valid base64 (e.g. z.string().regex(/^[A-Za-z0-9+/]+={0,2}$/)). Locate and
modify the TrackSchema declaration and replace the loose z.number().optional() /
z.string().optional() entries for those symbols with the stronger validators.
In `@js/watch/src/broadcast.ts`:
- Around line 132-134: The current .then((c) => { if (c) return { kind: "msf" as
const, root: toHang(c) }; ... }) returns an MSF win even when toHang(c) produces
an empty root ({}); change the callback to validate the converted root from
toHang(c) and reject (throw) when it's empty. Specifically, call const root =
toHang(c) and check (e.g., Object.keys(root).length === 0) before returning {
kind: "msf", root }, throwing a descriptive Error like "msf catalog empty after
conversion" if the root is empty so MSF cannot win negotiation with no playable
renditions.
In `@js/watch/src/msf.ts`:
- Around line 92-99: The loop over msf.tracks currently ignores tracks with no
role set; update the logic in the block that iterates msf.tracks so tracks with
undefined role are not dropped but instead are inspected with the same
heuristics used by toVideoConfig and toAudioConfig (e.g., check codec, mime,
resolution/width/height or channel count) and routed into
videoRenditions[track.name] via toVideoConfig or audioRenditions[track.name] via
toAudioConfig as appropriate; keep the existing behavior when role === "video"
or "audio" but add a fallback branch that calls toVideoConfig first (or
toAudioConfig first) based on the track metadata and assigns the resulting
config if truthy.
---
Nitpick comments:
In `@js/msf/src/catalog.ts`:
- Around line 4-61: Add brief doc comments for the public MSF API exports to
document the package contract: annotate PackagingSchema and RoleSchema with
allowed values summary, describe TrackSchema/CatalogSchema shape and the
expected catalog version=1 structure, and add docstrings for encode (serializes
Catalog to Uint8Array), decode (parses JSON and documents that it will throw on
invalid data or malformed JSON), and fetch (documents it reads one frame from
Moq.Track, decodes it, and returns undefined if the track closes without a
frame). Use the exported symbol names PackagingSchema, RoleSchema, TrackSchema,
CatalogSchema, encode, decode, and fetch so the comments sit directly above
those declarations.
In `@js/watch/src/element.ts`:
- Around line 259-268: The catalog getter currently returns the internal Signal
array by reference (via broadcast.catalogFormats.peek()), and the setter stores
arrays directly, allowing external mutation to bypass Signal.set(); change the
getter (catalog) to return a shallow copy of the array so callers cannot mutate
internal state, and update the setter to validate/normalize entries (when given
CatalogAttr or CatalogFormat[]) and pass a new array instance to
broadcast.catalogFormats.set() (use parseCatalogAttr for string inputs, then
validate each CatalogFormat item) so all mutations go through Signal.set() and
invalid values are rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb5f9c11-924d-4a75-942f-34f77c6c954f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
js/msf/package.jsonjs/msf/src/catalog.tsjs/msf/src/index.tsjs/msf/tsconfig.jsonjs/watch/package.jsonjs/watch/src/broadcast.tsjs/watch/src/element.tsjs/watch/src/msf.tspackage.json
| width: z.number().optional(), | ||
| height: z.number().optional(), | ||
| framerate: z.number().optional(), | ||
| samplerate: z.number().optional(), | ||
| channelConfig: z.string().optional(), | ||
| bitrate: z.number().optional(), | ||
| initData: z.string().optional(), | ||
| renderGroup: z.number().optional(), | ||
| altGroup: z.number().optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the Zod version and inspect downstream assumptions for MSF numeric/base64 fields.
fd -i '^package\.json$' js/msf -x sh -c 'echo "== $1 =="; sed -n "/\"zod\"/p" "$1"' sh {}
rg -n -C3 '\bu53\s*\(|base64ToBytes\s*\(|\b(width|height|framerate|samplerate|bitrate|renderGroup|altGroup|initData)\b' --type=tsRepository: moq-dev/moq
Length of output: 50368
🌐 Web query:
Zod 4.1.5 base64 validator z.base64
💡 Result:
In Zod 4.1.5, the base64 validator is z.base64, a top-level string format validator that checks if a string is valid Base64 (RFC 4648 standard, including padding with =). It validates the character set (A-Z, a-z, 0-9, +, /, =), proper length (multiple of 4), and padding. Invalid examples like "should" throw "invalid base64 format". Usage: import { z } from "zod"; const schema = z.base64; schema.parse("SGVsbG8gd29ybGQ="); // valid: "Hello world" schema.parse("should"); // throws ZodError: invalid base64 format It differs from z.base64url, which is URL-safe (uses - and _ instead of + and /, no padding allowed in Zod 4). This feature was added in Zod 3.23 via PR #3047 and remains unchanged in 4.x, including 4.1.5. For decoding, use z.codec(z.base64, z.instanceof(Uint8Array)) with z.util.base64ToUint8Array.
Citations:
- 1: https://zod.dev/api
- 2: https://zod.dev/v4
- 3: https://www.zod.dev/v4/changelog?id=no-padding-in-base64url
- 4: https://v4.zod.dev/api?id=codecs
- 5: How to validate base64? colinhacks/zod#1446
- 6: Adds base64 string validation colinhacks/zod#3047
- 7: https://v4.zod.dev/codecs?id=base64urltobytes
Tighten validation for MSF track metadata to prevent downstream failures.
The TrackSchema currently accepts negative, fractional, and unsafe numeric values. Additionally, initData accepts any string even though js/watch/src/msf.ts decodes it as base64. Without validation here, decoded Catalog objects will fail when numeric fields are passed to u53() for 53-bit safe integer conversion, or when initData is decoded via atob().
🛡️ Proposed schema hardening
+const U53Schema = z.number().int().nonnegative();
+const NonNegativeNumberSchema = z.number().nonnegative();
+
export const TrackSchema = z.object({
name: z.string(),
packaging: PackagingSchema,
isLive: z.boolean(),
role: RoleSchema.optional(),
codec: z.string().optional(),
- width: z.number().optional(),
- height: z.number().optional(),
- framerate: z.number().optional(),
- samplerate: z.number().optional(),
+ width: U53Schema.optional(),
+ height: U53Schema.optional(),
+ framerate: NonNegativeNumberSchema.optional(),
+ samplerate: U53Schema.optional(),
channelConfig: z.string().optional(),
- bitrate: z.number().optional(),
- initData: z.string().optional(),
- renderGroup: z.number().optional(),
- altGroup: z.number().optional(),
+ bitrate: U53Schema.optional(),
+ initData: z.base64().optional(),
+ renderGroup: U53Schema.optional(),
+ altGroup: U53Schema.optional(),
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/msf/src/catalog.ts` around lines 20 - 28, TrackSchema currently allows
negative, fractional and unsafe numeric values and accepts any string for
initData; update TrackSchema so numeric fields (width, height, framerate,
samplerate, bitrate, renderGroup, altGroup) use integer, non-negative,
53-bit-safe validation (e.g.
z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER)) to prevent
fractional/unsafe values being passed to u53(), and tighten initData to require
valid base64 (e.g. z.string().regex(/^[A-Za-z0-9+/]+={0,2}$/)). Locate and
modify the TrackSchema declaration and replace the loose z.number().optional() /
z.string().optional() entries for those symbols with the stronger validators.
| export function decode(raw: Uint8Array): Catalog { | ||
| const decoder = new TextDecoder(); | ||
| const str = decoder.decode(raw); | ||
| try { | ||
| const json = JSON.parse(str); | ||
| return CatalogSchema.parse(json); | ||
| } catch (error) { | ||
| console.warn("invalid MSF catalog", str); | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject malformed UTF-8 and avoid logging the full catalog payload.
TextDecoder defaults to replacement mode, so invalid UTF-8 can be silently converted before JSON.parse. Also, logging str can expose user-provided metadata or large initData payloads.
🛡️ Proposed decode hardening
export function decode(raw: Uint8Array): Catalog {
- const decoder = new TextDecoder();
- const str = decoder.decode(raw);
try {
+ const decoder = new TextDecoder("utf-8", { fatal: true });
+ const str = decoder.decode(raw);
const json = JSON.parse(str);
return CatalogSchema.parse(json);
} catch (error) {
- console.warn("invalid MSF catalog", str);
+ const message = error instanceof Error ? error.message : String(error);
+ console.warn("invalid MSF catalog", { byteLength: raw.byteLength, error: message });
throw error;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/msf/src/catalog.ts` around lines 45 - 55, The decode function currently
uses TextDecoder in replacement mode and logs the entire decoded string; change
it to create the TextDecoder with { fatal: true } so invalid UTF-8 throws before
JSON.parse (refer to decode and TextDecoder usage), and update the catch block
around JSON.parse / CatalogSchema.parse to avoid printing the full str — log
only a minimal safe message (e.g., "invalid MSF catalog" plus a small fixed-size
hex/byte-length or prefix) or rethrow the error without the full payload to
prevent exposing large or sensitive initData.
| .then((c) => { | ||
| if (c) return { kind: "msf" as const, root: toHang(c) }; | ||
| throw new Error("msf catalog empty"); |
There was a problem hiding this comment.
Reject empty MSF conversions before letting MSF win negotiation.
Msf.fetch(msfTrack) can return a catalog that toHang(c) converts to {} when no usable renditions are found. Because this checks only c, MSF can win with an empty root and set the broadcast live with nothing playable.
Validate the converted root
.then((c) => {
- if (c) return { kind: "msf" as const, root: toHang(c) };
+ if (c) {
+ const root = toHang(c);
+ if (root.video || root.audio) return { kind: "msf" as const, root };
+ }
throw new Error("msf catalog empty");
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .then((c) => { | |
| if (c) return { kind: "msf" as const, root: toHang(c) }; | |
| throw new Error("msf catalog empty"); | |
| .then((c) => { | |
| if (c) { | |
| const root = toHang(c); | |
| if (root.video || root.audio) return { kind: "msf" as const, root }; | |
| } | |
| throw new Error("msf catalog empty"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/watch/src/broadcast.ts` around lines 132 - 134, The current .then((c) => {
if (c) return { kind: "msf" as const, root: toHang(c) }; ... }) returns an MSF
win even when toHang(c) produces an empty root ({}); change the callback to
validate the converted root from toHang(c) and reject (throw) when it's empty.
Specifically, call const root = toHang(c) and check (e.g.,
Object.keys(root).length === 0) before returning { kind: "msf", root }, throwing
a descriptive Error like "msf catalog empty after conversion" if the root is
empty so MSF cannot win negotiation with no playable renditions.
| for (const track of msf.tracks) { | ||
| if (track.role === "video") { | ||
| const config = toVideoConfig(track); | ||
| if (config) videoRenditions[track.name] = config; | ||
| } else if (track.role === "audio") { | ||
| const config = toAudioConfig(track); | ||
| if (config) audioRenditions[track.name] = config; | ||
| } |
There was a problem hiding this comment.
Handle MSF tracks without an explicit role.
Msf.Track.role is optional, but this loop drops any role-less track even when it has usable video/audio metadata. That can turn a valid MSF catalog into an empty hang catalog and prevent playback.
Possible fallback inference
for (const track of msf.tracks) {
- if (track.role === "video") {
+ const looksVideo = track.width != null || track.height != null || track.framerate != null;
+ const looksAudio = track.samplerate != null || track.channelConfig != null;
+
+ if (track.role === "video" || (track.role == null && looksVideo)) {
const config = toVideoConfig(track);
if (config) videoRenditions[track.name] = config;
- } else if (track.role === "audio") {
+ } else if (track.role === "audio" || (track.role == null && looksAudio)) {
const config = toAudioConfig(track);
if (config) audioRenditions[track.name] = config;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const track of msf.tracks) { | |
| if (track.role === "video") { | |
| const config = toVideoConfig(track); | |
| if (config) videoRenditions[track.name] = config; | |
| } else if (track.role === "audio") { | |
| const config = toAudioConfig(track); | |
| if (config) audioRenditions[track.name] = config; | |
| } | |
| for (const track of msf.tracks) { | |
| const looksVideo = track.width != null || track.height != null || track.framerate != null; | |
| const looksAudio = track.samplerate != null || track.channelConfig != null; | |
| if (track.role === "video" || (track.role == null && looksVideo)) { | |
| const config = toVideoConfig(track); | |
| if (config) videoRenditions[track.name] = config; | |
| } else if (track.role === "audio" || (track.role == null && looksAudio)) { | |
| const config = toAudioConfig(track); | |
| if (config) audioRenditions[track.name] = config; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/watch/src/msf.ts` around lines 92 - 99, The loop over msf.tracks currently
ignores tracks with no role set; update the logic in the block that iterates
msf.tracks so tracks with undefined role are not dropped but instead are
inspected with the same heuristics used by toVideoConfig and toAudioConfig
(e.g., check codec, mime, resolution/width/height or channel count) and routed
into videoRenditions[track.name] via toVideoConfig or
audioRenditions[track.name] via toAudioConfig as appropriate; keep the existing
behavior when role === "video" or "audio" but add a fallback branch that calls
toVideoConfig first (or toAudioConfig first) based on the track metadata and
assigns the resulting config if truthy.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
js/watch/src/broadcast.ts (1)
108-111:⚠️ Potential issue | 🟡 Minor
toHang()can produce an empty{}root on MSF — status still flips to"live".
toHang(update)returns{}when the MSF catalog has novideo/audiorenditions (seejs/watch/src/msf.tslines 87-113: onlytrack.role === "video"|"audio"are mapped, everything else is dropped). The loop then treats{}as a valid update, sets#catalogto an empty root and transitionsstatusto"live"with nothing playable. Consider treating an empty converted root as "no update" (skip the iteration) or as stream end, so downstream consumers ofstatus/catalogdon't see a spurious live state.: async () => { const update = await Msf.fetch(track); - return update ? toHang(update) : undefined; + if (!update) return undefined; + const root = toHang(update); + return root.video || root.audio ? root : undefined; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/watch/src/broadcast.ts` around lines 108 - 111, The converted root from toHang(update) can be an empty object which currently counts as a valid update and flips status to "live"; modify the async updater (the lambda that calls Msf.fetch(track) in broadcast.ts) to inspect the result of toHang(update) and treat an empty root as no update: call const root = update ? toHang(update) : undefined; then if root is an object with no own keys (Object.keys(root).length === 0) return undefined (or otherwise signal end), otherwise return root; this prevents setting `#catalog` to an empty root and avoids spurious "live" transitions.
🧹 Nitpick comments (2)
js/watch/src/broadcast.ts (2)
101-111: Extract the track-name and fetcher mapping out of#runCatalog.The
"catalog.json"/"catalog"track names and the format→fetcher switch are format-policy details buried inside the effect. Extracting a small map (or two tiny helpers) would remove the inline ternaries, avoid magic string literals, and make it trivial to add future formats.+const CATALOG_TRACK: Record<CatalogFormat, string> = { + hang: "catalog.json", + msf: "catalog", +};Then
const track = broadcast.subscribe(CATALOG_TRACK[format], Catalog.PRIORITY.catalog);and afetchCatalog(format, track)helper.As per coding guidelines: "Avoid using magic numbers; use named constants instead" — the same applies to these magic track-name strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/watch/src/broadcast.ts` around lines 101 - 111, The runCatalog effect currently inlines format-specific track names and a ternary fetcher; extract those into named mappings and helpers: create a CATALOG_TRACK map keyed by format values that maps to "catalog.json" or "catalog" and a fetchCatalog(format, track) function that returns the appropriate async fetcher (delegating to Catalog.fetch for "hang" and to Msf.fetch + toHang for other formats). Replace uses of trackName, fetchNext and the inline ternaries in runCatalog with broadcast.subscribe(CATALOG_TRACK[format], Catalog.PRIORITY.catalog) and fetchCatalog(format, track) to remove magic strings and simplify the effect.
28-29: API naming:catalogprop vscataloggetter collide.
BroadcastProps.catalognow means “catalog format” (aCatalogFormat), while theBroadcastclass already exposescatalog: Getter<Catalog.Root | undefined>(the decoded catalog) and stores the format ascatalogFormat. Two different "catalog" meanings on the same API is confusing for consumers. Consider renaming the prop tocatalogFormatto match the field, keeping one consistent noun across prop/field/getter.- // Which catalog format to use. Default: "hang" - catalog?: CatalogFormat | Signal<CatalogFormat>; + // Which catalog format to use. Default: "hang" + catalogFormat?: CatalogFormat | Signal<CatalogFormat>;- this.catalogFormat = Signal.from(props?.catalog ?? "hang"); + this.catalogFormat = Signal.from(props?.catalogFormat ?? "hang");Note:
js/watch/src/element.tsalready names the attribute/gettercatalogon the web component, which would also benefit from being updated for consistency.Also applies to: 41-41, 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/watch/src/broadcast.ts` around lines 28 - 29, Rename the prop in BroadcastProps from catalog to catalogFormat to avoid colliding with the Broadcast class getter catalog (Getter<Catalog.Root | undefined>) and to match the existing field name catalogFormat; update all usages and typings where BroadcastProps.catalog is referenced (including the web component attribute/getter in element.ts) to use catalogFormat instead so the prop/field/getter terminology is consistent across Broadcast, BroadcastProps, and the web component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@js/watch/src/broadcast.ts`:
- Around line 108-111: The converted root from toHang(update) can be an empty
object which currently counts as a valid update and flips status to "live";
modify the async updater (the lambda that calls Msf.fetch(track) in
broadcast.ts) to inspect the result of toHang(update) and treat an empty root as
no update: call const root = update ? toHang(update) : undefined; then if root
is an object with no own keys (Object.keys(root).length === 0) return undefined
(or otherwise signal end), otherwise return root; this prevents setting `#catalog`
to an empty root and avoids spurious "live" transitions.
---
Nitpick comments:
In `@js/watch/src/broadcast.ts`:
- Around line 101-111: The runCatalog effect currently inlines format-specific
track names and a ternary fetcher; extract those into named mappings and
helpers: create a CATALOG_TRACK map keyed by format values that maps to
"catalog.json" or "catalog" and a fetchCatalog(format, track) function that
returns the appropriate async fetcher (delegating to Catalog.fetch for "hang"
and to Msf.fetch + toHang for other formats). Replace uses of trackName,
fetchNext and the inline ternaries in runCatalog with
broadcast.subscribe(CATALOG_TRACK[format], Catalog.PRIORITY.catalog) and
fetchCatalog(format, track) to remove magic strings and simplify the effect.
- Around line 28-29: Rename the prop in BroadcastProps from catalog to
catalogFormat to avoid colliding with the Broadcast class getter catalog
(Getter<Catalog.Root | undefined>) and to match the existing field name
catalogFormat; update all usages and typings where BroadcastProps.catalog is
referenced (including the web component attribute/getter in element.ts) to use
catalogFormat instead so the prop/field/getter terminology is consistent across
Broadcast, BroadcastProps, and the web component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 24c77dcb-e2e0-4041-b43c-ea03c77c1027
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
js/msf/src/catalog.tsjs/watch/package.jsonjs/watch/src/broadcast.tsjs/watch/src/element.ts
✅ Files skipped from review due to trivial changes (1)
- js/watch/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- js/watch/src/element.ts
`BroadcastProps.catalog` was ambiguous with `Broadcast.catalog` (the resolved `Getter<Catalog.Root>`). Rename the prop/attr/getter to `catalogFormat` / `catalog-format` so the format selector is consistent across Broadcast, BroadcastProps, and <moq-watch>. https://claude.ai/code/session_012gjc9wFMcfGj83fazaJ5ij
The common build script copies README.md into dist/, so the package must have one or CI's build step fails with ENOENT. https://claude.ai/code/session_012gjc9wFMcfGj83fazaJ5ij
There was a problem hiding this comment.
♻️ Duplicate comments (1)
js/watch/src/broadcast.ts (1)
105-123:⚠️ Potential issue | 🟠 MajorMSF can publish an empty catalog as "live".
toHang(update)returns{}when the MSF catalog has no usable video/audio renditions (seetoHanginjs/watch/src/msf.ts:87-113). In that caseupdateis truthy, so the loop still callsthis.#catalog.set({})and transitionsstatusto"live"with nothing playable. Consider rejecting/skipping when the converted root has neithervideonoraudio:♻️ Proposed guard
: async () => { const update = await Msf.fetch(track); - return update ? toHang(update) : undefined; + if (!update) return undefined; + const root = toHang(update); + if (!root.video && !root.audio) return undefined; + return root; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/watch/src/broadcast.ts` around lines 105 - 123, The loop currently treats any truthy MSF fetch result as a valid update, but toHang(update) can return an empty object (no `video` or `audio`) causing this.#catalog.set({}) and status.set("live") with nothing playable; change the fetch/consume logic (the fetchNext branch using Msf.fetch and the consumer in effect.spawn) to detect and skip empty converted catalogs (i.e., where the object from toHang has neither `video` nor `audio`) — when detected, continue the loop without calling this.#catalog.set or this.status.set so empty MSF publications are ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@js/watch/src/broadcast.ts`:
- Around line 105-123: The loop currently treats any truthy MSF fetch result as
a valid update, but toHang(update) can return an empty object (no `video` or
`audio`) causing this.#catalog.set({}) and status.set("live") with nothing
playable; change the fetch/consume logic (the fetchNext branch using Msf.fetch
and the consumer in effect.spawn) to detect and skip empty converted catalogs
(i.e., where the object from toHang has neither `video` nor `audio`) — when
detected, continue the loop without calling this.#catalog.set or this.status.set
so empty MSF publications are ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf1a598d-c03e-4876-9d8b-361f155d983d
📒 Files selected for processing (2)
js/watch/src/broadcast.tsjs/watch/src/element.ts
Mirrors the hang catalog's `jitter` field: maximum delay (ms) between consecutive frames on a track, which the player must buffer for to avoid underruns. Not in the MSF spec, but needed to drive the jitter buffer from MSF-only publishers. Wired through `toHang()` so MSF tracks producing a `jitter` value flow straight into hang's existing `jitter` field on VideoConfig/AudioConfig. https://claude.ai/code/session_012gjc9wFMcfGj83fazaJ5ij
There was a problem hiding this comment.
♻️ Duplicate comments (2)
js/msf/src/catalog.ts (2)
54-63:⚠️ Potential issue | 🟡 MinorReject malformed UTF-8 and avoid logging the catalog payload.
TextDecoderdefaults to replacement mode, andconsole.warn(..., str)can leak user-provided metadata or largeinitDatavalues.🛡️ Proposed decode hardening
export function decode(raw: Uint8Array): Catalog { - const decoder = new TextDecoder(); - const str = decoder.decode(raw); try { + const decoder = new TextDecoder("utf-8", { fatal: true }); + const str = decoder.decode(raw); const json = JSON.parse(str); return CatalogSchema.parse(json); } catch (error) { - console.warn("invalid MSF catalog", str); + const message = error instanceof Error ? error.message : String(error); + console.warn("invalid MSF catalog", { byteLength: raw.byteLength, error: message }); throw error; } }Please verify the runtime behavior for your supported JS environments:
MDN TextDecoder fatal option invalid UTF-8 replacement behavior🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/msf/src/catalog.ts` around lines 54 - 63, The decode function currently uses TextDecoder in replacement mode and logs the decoded payload, risking silent acceptance of malformed UTF-8 and leaking sensitive data; update decode to construct TextDecoder with the fatal option (new TextDecoder(undefined, { fatal: true })) so invalid UTF-8 raises on decode, remove any logging of the decoded catalog payload (do not console.warn(str)), and in the catch block log only a generic message (e.g., "invalid MSF catalog") and rethrow or wrap the error; ensure CatalogSchema.parse remains used for JSON validation but do not expose the raw str in logs or exceptions.
24-37:⚠️ Potential issue | 🟠 MajorTighten MSF metadata validation before downstream
u53()conversion.
width,height,samplerate,bitrate,renderGroup,altGroup, and the newjitterfield can still be negative, fractional, or unsafe.js/watch/src/msf.tsconverts these values withu53(), so malformed catalogs can fail afterdecode()succeeds.initDatashould also be validated as base64 before the CMAF parser sees it.🛡️ Proposed schema hardening
+const U53Schema = z.int().check(z.nonnegative()); +const NonNegativeNumberSchema = z.number().check(z.nonnegative()); + export const TrackSchema = z.object({ name: z.string(), packaging: PackagingSchema, isLive: z.boolean(), role: z.optional(RoleSchema), codec: z.optional(z.string()), - width: z.optional(z.number()), - height: z.optional(z.number()), - framerate: z.optional(z.number()), - samplerate: z.optional(z.number()), + width: z.optional(U53Schema), + height: z.optional(U53Schema), + framerate: z.optional(NonNegativeNumberSchema), + samplerate: z.optional(U53Schema), channelConfig: z.optional(z.string()), - bitrate: z.optional(z.number()), - initData: z.optional(z.string()), - renderGroup: z.optional(z.number()), - altGroup: z.optional(z.number()), + bitrate: z.optional(U53Schema), + initData: z.optional(z.base64()), + renderGroup: z.optional(U53Schema), + altGroup: z.optional(U53Schema), // Non-standard: maximum delay (ms) between consecutive frames on this track. // The player's buffer must be at least this large to avoid underruns. // Mirrors the `jitter` field in the hang catalog. - jitter: z.optional(z.number()), + jitter: z.optional(U53Schema), });Please verify these Zod Mini helpers against the pinned Zod version before applying:
For zod@4.1.5 with import from "zod/mini", confirm that z.int().check(z.nonnegative()), z.number().check(z.nonnegative()), and z.base64() are supported APIs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/msf/src/catalog.ts` around lines 24 - 37, The schema in catalog.ts allows negative, fractional, NaN/Infinity and unsafe integers which later break u53() in js/watch/src/msf.ts during decode(); update the Zod validators for the fields width, height, samplerate, bitrate, renderGroup, altGroup, and jitter to require nonnegative integers within safe integer range (use z.int()/z.number() with checks for nonnegative, finite, and <= Number.MAX_SAFE_INTEGER as supported by your zod/mini version) and validate initData with a base64 check (z.base64() or equivalent) so malformed values are rejected early; confirm the zod/mini APIs z.int().check(z.nonnegative()), z.number().check(z.nonnegative()), and z.base64() exist for the pinned zod version before applying.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@js/msf/src/catalog.ts`:
- Around line 54-63: The decode function currently uses TextDecoder in
replacement mode and logs the decoded payload, risking silent acceptance of
malformed UTF-8 and leaking sensitive data; update decode to construct
TextDecoder with the fatal option (new TextDecoder(undefined, { fatal: true }))
so invalid UTF-8 raises on decode, remove any logging of the decoded catalog
payload (do not console.warn(str)), and in the catch block log only a generic
message (e.g., "invalid MSF catalog") and rethrow or wrap the error; ensure
CatalogSchema.parse remains used for JSON validation but do not expose the raw
str in logs or exceptions.
- Around line 24-37: The schema in catalog.ts allows negative, fractional,
NaN/Infinity and unsafe integers which later break u53() in js/watch/src/msf.ts
during decode(); update the Zod validators for the fields width, height,
samplerate, bitrate, renderGroup, altGroup, and jitter to require nonnegative
integers within safe integer range (use z.int()/z.number() with checks for
nonnegative, finite, and <= Number.MAX_SAFE_INTEGER as supported by your
zod/mini version) and validate initData with a base64 check (z.base64() or
equivalent) so malformed values are rejected early; confirm the zod/mini APIs
z.int().check(z.nonnegative()), z.number().check(z.nonnegative()), and
z.base64() exist for the pinned zod version before applying.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1cc48209-d011-4ace-b3e8-de8fe27243b1
📒 Files selected for processing (2)
js/msf/src/catalog.tsjs/watch/src/msf.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- js/watch/src/msf.ts
2 participants
Summary
This PR adds support for the MSF (MOQT Streaming Format) catalog format alongside the existing hang format, with intelligent format negotiation that races both formats and selects the first successful response.
Key Changes
New
@moq/msfpackage: Created a new package with MSF catalog types and utilitiescatalog.ts: Defines MSF catalog schema with Zod validation and encode/decode functionsMSF to hang conversion: Added
js/watch/src/msf.tswith conversion utilitiestoHang(): Converts MSF catalogs to hang format for unified internal representationtoVideoConfig()andtoAudioConfig(): Extract and normalize track configurationstoContainer(): Parses CMAF init segments with fallback to legacy formatCatalog format negotiation in Broadcast:
catalogFormatssignal to allow configurable format preferencesPromise.any()HTML element API: Extended
<moq-watch>elementcatalogattribute: accepts "hang", "msf", or "auto" (tries both)catalogproperty: getter/setter for programmatic controlparseCatalogAttr()utilityImplementation Details
Promise.race()for the initial fetch (with MSF delayed by 100ms) andPromise.any()to ensure only successful fetches competeu53()for safe integer handlinghttps://claude.ai/code/session_012gjc9wFMcfGj83fazaJ5ij