Make payload compression a per-hop, per-frame decision#39
Open
kixelated wants to merge 10 commits into
Open
Conversation
Rework compression in both moq-lite and the MoQ Payload Compression Extension so the publisher only signals *whether* a track is worth compressing, while the algorithm is negotiated per hop and recorded per frame/object. A single frame can opt out (e.g. a small JSON merge-patch delta that DEFLATE would enlarge), and different hops can use different algorithms. moq-lite: - Publisher Compression in TRACK_INFO becomes a boolean hint (reserved values >1 are treated as 1). - New SETUP Compression parameter: each endpoint advertises the algorithms it can decompress, negotiated per hop. - New per-frame Compression field in FRAME and datagram bodies, present only when the hint is set, naming the algorithm used (none/deflate). - New Compression section defining the algorithm IDs (shared with the extension) and relay behavior, plus compression security notes. moq-compression: - COMPRESSION track property becomes a boolean hint. - Algorithms are advertised in preference order; the hop default is the receiver's most-preferred mutually-supported algorithm. - New COMPRESSION_ALGORITHM object property overrides the hop default per object (typically none, to opt out), keeping the common case free of per-object signaling. - Relay, security, and IANA updated (new Object-scope property). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe pull request revises MoQ compression across two draft specifications to use an end-to-end algorithm identifier model with per-hop negotiation and group-scoped stream mechanics. In In 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ 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 |
kixelated
added a commit
to moq-dev/moq
that referenced
this pull request
Jun 22, 2026
Implements the moq-lite half of moq-dev/drafts#39: the publisher only signals *whether* a track is worth compressing, while the algorithm is negotiated per hop and recorded per frame. A single frame can opt out — e.g. a small JSON merge-patch delta that DEFLATE would only enlarge — instead of bloating it. Wire (moq-lite-05-wip): - TRACK_INFO `Publisher Compression` becomes a boolean hint (>1 reserved, decodes as true so it stays additive). It names no algorithm. - New SETUP `Compression` parameter (id 0x3): each endpoint advertises the algorithms it can decompress, packed varints, none(0) omitted. moq-net advertises [deflate]. Per hop, per direction; never forwarded by a relay. - New per-frame `Compression` field in FRAME, present iff the track is hinted, after the timestamp delta and before the length. Names the codec actually used (none/deflate); it doubles as the group's end-of-frames sentinel when no timestamp precedes it. Behavior: - The publisher reads the peer's advertised algorithms (PeerSetup) before serving a compress-hinted track. It forwards an already-DEFLATE cached frame verbatim when the peer can inflate it (relay passthrough, no inflate/deflate), and otherwise picks per frame whether DEFLATE actually shrinks the payload, sending `none` when it wouldn't (or the peer can't inflate). Empty payloads always use `none`. - The subscriber decodes the per-frame field and caches each frame in that codec, so the cache holds compressed bytes (billing counts the wire size) and inflates only at delivery. No datagram path exists in moq-net yet, so that part of the draft is N/A here. The IETF moqt compression extension (moq-compression) is left for a follow-up; the IETF path keeps sending verbatim, which is spec-compliant "extension not negotiated". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 tasks
Rework payload compression so a whole group is compressed as a single DEFLATE stream rather than each frame independently. Within a group the frames already share one ordered, reliable QUIC stream, so the group — not the frame — is the random-access unit; compressing across it gains cross-frame redundancy and compresses the framing too, while the first frame of a group stays independently decodable and groups remain mutually independent. - Publisher Compression in TRACK_INFO stays a boolean end-to-end hint. - SETUP Compression parameter is now a presence flag (capability), not an algorithm list. - A per-group Compression field (0 verbatim, 1 DEFLATE) is carried in GROUP, in datagram bodies, and in a reintroduced minimal FETCH_OK (present only when the hint is set). The per-frame Compression field is gone. - DEFLATE is the only scheme; others are left to future extensions, one in effect per hop. - Security note updated: the CRIME/BREACH window is now per group, not per frame. This supersedes the earlier per-frame, enum-based compression on this branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
draft-lcurley-moq-compression.md (1)
101-106: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClarify behavior when COMPRESSION_ALGORITHM override is present on an empty payload.
Line 106 states "An empty payload (size 0) MUST NOT be compressed and remains empty on the wire; it needs no override."
The specification is clear that empty payloads must not be compressed, but it does not explicitly state whether a COMPRESSION_ALGORITHM override on an empty object is an error, must be ignored, or must not be present. Consider adding:
"If a COMPRESSION_ALGORITHM override is present on an object with an empty payload, the receiver MUST ignore it and treat the object as verbatim."
💡 Proposed clarification
An empty payload (size 0) MUST NOT be compressed and remains empty on the wire; it needs no override. +If a COMPRESSION_ALGORITHM override is present on an empty object, the receiver MUST ignore it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@draft-lcurley-moq-compression.md` around lines 101 - 106, The specification currently states that empty payloads must not be compressed but does not explicitly clarify what should happen if a COMPRESSION_ALGORITHM override is present on an object with an empty payload. Add a new sentence after line 106 that explicitly states the receiver's behavior: clarify that when a COMPRESSION_ALGORITHM override is present on an object with an empty payload, the receiver MUST ignore the override and treat the object as verbatim, making the behavior unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@draft-lcurley-moq-compression.md`:
- Around line 101-106: The specification currently states that empty payloads
must not be compressed but does not explicitly clarify what should happen if a
COMPRESSION_ALGORITHM override is present on an object with an empty payload.
Add a new sentence after line 106 that explicitly states the receiver's
behavior: clarify that when a COMPRESSION_ALGORITHM override is present on an
object with an empty payload, the receiver MUST ignore the override and treat
the object as verbatim, making the behavior unambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba69ab0f-007a-45a7-b084-d2cd2119289b
📒 Files selected for processing (2)
draft-lcurley-moq-compression.mddraft-lcurley-moq-lite.md
Switch group compression from "compress the whole frame sequence" to "compress only the payloads." The payloads of a group still form one DEFLATE stream (reset per group, sliced per frame into each frame's opaque Payload), so cross-frame redundancy is retained, but the FRAME framing stays in the clear. This is for version agility and caching: with the framing uncompressed, a relay or cache can keep payloads compressed in memory, read frame metadata without inflating, and re-frame a group across transport versions (new GROUP/FRAME headers) without decompress/recompress. None of that is possible if the framing is inside the DEFLATE blob. Message Length is again the on-wire (compressed) Payload size. Also revert the moq-compression extension to its pre-PR state: its earlier enum/per-object-override design is superseded by this model and will be redone to mirror the final moq-lite design. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
With the shared per-group DEFLATE stream and the RFC 7692 trailing-bytes
trim, per-frame overhead is ~1 byte, so a per-group on/off toggle isn't
worth a wire field. Compression is now decided purely by the end-to-end
Publisher Compression signal plus per-hop negotiation.
- Publisher Compression stays an end-to-end signal ("good candidate for
compression") that fans out unchanged, so a hop that doesn't compress
still forwards it for a downstream hop to act on.
- Remove the per-group Compression field from GROUP and datagram bodies,
and remove FETCH_OK (it only carried that field). A receiver infers
compression from the signal plus its own SETUP advertisement.
- Spell out the RFC 7692 trim (strip the redundant 00 00 FF FF, since we
frame the slices ourselves) and that the decoder keeps one context per
group.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
…to match Add a second algorithm (zstd) and per-hop algorithm negotiation, now that browsers do zstd natively and its dictionary support suits MoQ's small, repetitive non-media payloads. deflate stays the mandatory baseline. Negotiation (both drafts): each endpoint advertises the algorithms it can de/compress in preference order; for a direction, the algorithm is the first in the receiver's list that the sender also lists. Both ends compute it identically from the two advertised lists, so a simultaneous SETUP can't disagree; the two directions are independent and may differ. deflate is mandatory, so a common algorithm always exists. moq-lite: the SETUP Compression parameter carries the ordered list (was a presence flag); the Compression section gains the algorithm table and per-algorithm framing trims (RFC 7692 for deflate; magicless, checksum-less frames for zstd). Still flagless and payload-only. moq-compression: rewritten to the same model — boolean COMPRESSION track Property (was an algorithm id) + COMPRESSION Setup Option carrying the ordered list + first-intersection selection + flagless inference + subgroup-scoped sliced stream. Drops the per-object override; the algorithms registry gains zstd. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
draft-lcurley-moq-lite.md (1)
1219-1220: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound decompression on the whole group stream.
Because compression here is group-scoped across multiple
FRAMEs, the limit needs to apply to the cumulative decompressed output for that group stream. As written, this can be read as a per-slice limit, which still allows an attacker to grow memory unboundedly across many frames.🔧 Proposed fix
- A receiver MUST bound the size of a decompressed payload; if the bound is exceeded it MUST reset the affected stream rather than allocate unbounded memory, and MAY close the session with a PROTOCOL_VIOLATION if it considers the peer abusive. + A receiver MUST bound the cumulative size of the decompressed output for the current group stream; if the bound is exceeded it MUST reset the affected stream rather than allocate unbounded memory, and MAY close the session with a PROTOCOL_VIOLATION if it considers the peer abusive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@draft-lcurley-moq-lite.md` around lines 1219 - 1220, The decompression bomb protection specification is ambiguous about the scope of the size bound. Clarify that the bound on decompressed payload size applies cumulatively to the entire group stream across all frames, not to individual frame decompressions, to prevent attackers from sending multiple small frames that individually stay within limits but collectively cause unbounded memory allocation. Revise the text to explicitly state that the receiver MUST track and enforce the cumulative decompressed size across all frames in a group stream and reset the stream if the total exceeds the bound.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@draft-lcurley-moq-compression.md`:
- Around line 69-70: The text describing the SETUP option currently states the
sender can "compress and decompress" which overstates the advertised capability.
Change this language to indicate that the endpoint can only *decompress* on this
hop, not both compress and decompress. Update the sentence beginning with "One
or more Algorithm identifiers" to clarify that the option lists algorithms the
endpoint can decompress, which represents the actual decompression-only
capability rather than a bidirectional compression capability.
In `@draft-lcurley-moq-lite.md`:
- Around line 634-640: The Compression Parameter description needs two
clarifications: First, change the statement that says the sender can
"decompress" to explicitly state that endpoints advertise algorithms they can
both compress and decompress on that hop. Second, add explicit guidance about
the fallback behavior while the peer's Compression Parameter list is still
unknown (before SETUP exchange is complete) to prevent implementations from
selecting algorithms they cannot emit or starting compression prematurely -
clarify that no compression should occur until both endpoints have exchanged
their lists and negotiation is complete.
---
Outside diff comments:
In `@draft-lcurley-moq-lite.md`:
- Around line 1219-1220: The decompression bomb protection specification is
ambiguous about the scope of the size bound. Clarify that the bound on
decompressed payload size applies cumulatively to the entire group stream across
all frames, not to individual frame decompressions, to prevent attackers from
sending multiple small frames that individually stay within limits but
collectively cause unbounded memory allocation. Revise the text to explicitly
state that the receiver MUST track and enforce the cumulative decompressed size
across all frames in a group stream and reset the stream if the total exceeds
the bound.
🪄 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: 4c09cba2-0632-4be6-bf8c-9d36c03f1164
📒 Files selected for processing (2)
draft-lcurley-moq-compression.mddraft-lcurley-moq-lite.md
Address review feedback on both drafts: - Decompression-bomb bound is now cumulative over the group (moq-lite) / subgroup (moq-compression), not per slice — since compression is stream-scoped, many small slices could otherwise accumulate without limit. - The advertised algorithm list denotes what an endpoint can BOTH compress and decompress, with a one-line rationale: flagless per-direction selection requires both sides to compute the algorithm from public info, so a decompress-only advertisement would force a per-message signal we deliberately don't have. (Unifies moq-lite, which had said "decompress", with moq-compression.) - A sender MUST NOT compress before it has received the peer's Compression Parameter / COMPRESSION option. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
Per implementation feedback, drop the boolean hint + first-intersection selection. The track property (Publisher Compression / COMPRESSION) now names the algorithm the publisher used (none/deflate/zstd); SETUP advertises the decoders each endpoint supports, and the publisher MUST pick an algorithm its peer advertised (deflate mandatory, so always safe). Flagless inference is now off the property: a receiver decompresses iff the property names a non-none algorithm it advertised, else verbatim. Reverts the per-direction selection and the "compress and decompress" wording (the list is decode capability again). Group/subgroup-scoped sliced-stream mechanics, deflate+zstd, and the RFC 7692 / magicless framing trims are unchanged. Relays forward the property unchanged and may recompress only with the same algorithm. Added an explicit "Open issue" note: an immutable algorithm-naming property means a relay can't transcode (e.g. zstd-> deflate) for a downstream that supports only a different algorithm without rewriting the property, which moq-transport forbids — flagged for the working group. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
Drop deflate's MUST-implement status in both drafts. A mandatory algorithm is a conformance burden (a zstd-only endpoint shouldn't be non-conformant) and doesn't actually guarantee end-to-end compression on a relayed path anyway. Now both deflate and zstd are simply defined; endpoints advertise whichever they support, and a publisher uses an algorithm its peer advertised or `none` if they share none — verbatim, the same well-defined fallback as any unsupported case. Removes the "Requirement" column and the "always a safe choice" framing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
There was a problem hiding this comment.
♻️ Duplicate comments (1)
draft-lcurley-moq-lite.md (1)
633-641: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClarify that the Compression Parameter advertises bidirectional capability.
Line 634 describes the parameter as advertising what the sender can "decompress," but line 639 constrains senders to only compress with algorithms the receiver advertised—implying the advertised list also governs the endpoint's compression capability. The PR commit message states: "advertised algorithms denote what an endpoint can both compress and decompress, since flagless per-direction selection requires both sides to compute the algorithm from public information."
For clarity, explicitly state that advertising an algorithm means the endpoint can both compress and decompress with it on that hop. Without this clarification, the semantic scope of the parameter is ambiguous.
🔧 Proposed fix
### Compression Parameter {`#compression-parameter`} -The Compression Parameter advertises the payload compression [algorithms](`#compression`) the sender can *decompress* on this hop. +The Compression Parameter advertises the payload compression [algorithms](`#compression`) the sender can compress and decompress on this hop, in preference order (most-preferred first).Alternatively, if you prefer to keep "advertises" and add a clarification sentence:
### Compression Parameter {`#compression-parameter`} The Compression Parameter advertises the payload compression [algorithms](`#compression`) the sender can *decompress* on this hop. +Because there is no per-direction flag, advertising an algorithm means the endpoint can both compress with it (for sending) and decompress with it (for receiving) on this hop, in preference order (most-preferred first).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@draft-lcurley-moq-lite.md` around lines 633 - 641, The Compression Parameter description is ambiguous about whether advertising an algorithm means the endpoint can both compress and decompress, or only decompress. Line 634 states the parameter advertises what the sender can "decompress," but line 639 constrains senders to only compress with algorithms the receiver advertised, which implies bidirectional capability. Revise the description to explicitly clarify that advertising an algorithm means the endpoint can both compress and decompress with it on that hop, making the bidirectional semantic scope unambiguous.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@draft-lcurley-moq-lite.md`:
- Around line 633-641: The Compression Parameter description is ambiguous about
whether advertising an algorithm means the endpoint can both compress and
decompress, or only decompress. Line 634 states the parameter advertises what
the sender can "decompress," but line 639 constrains senders to only compress
with algorithms the receiver advertised, which implies bidirectional capability.
Revise the description to explicitly clarify that advertising an algorithm means
the endpoint can both compress and decompress with it on that hop, making the
bidirectional semantic scope unambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5bfe1668-8800-498b-aba4-58d43c454249
📒 Files selected for processing (2)
draft-lcurley-moq-compression.mddraft-lcurley-moq-lite.md
The Compression parameter/option lists algorithms an endpoint can decompress; it does not advertise what it can produce. When sending, an endpoint compresses with an algorithm the receiver advertised. Resolves recurring ambiguity now that the publisher names the algorithm in the track property (so the receiver reads it rather than computing a mutual selection). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
5 tasks
…load Length When a group is compressed, each FRAME and datagram now carries a Decompressed Length varint (the payload's size after decompression), present only when compressed. A receiver uses it to size the output buffer in one allocation and as a precise per-frame decompression-bomb bound: reject if it exceeds the receiver's limit, and reset if the decoder produces a different number of bytes than declared. The security section is tightened to this per-frame rule (plus a SHOULD cumulative-per-group bound for accumulation). A plain absolute varint, not a delta from Payload Length: effective compression makes the two lengths diverge, so a delta would be larger than the absolute compressed size exactly when compression works. FRAME's Message Length is renamed Payload Length, which is more accurate — it has only ever delimited the Payload, not the whole message (Timestamp Delta and now Decompressed Length precede it). moq-compression is left on its cumulative-subgroup bound; a per-object declared size would be an expensive object KVP in moq-transport. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reworks payload compression in moq-lite and the MoQ Payload Compression Extension so that the publisher only signals whether a track is worth compressing, while the algorithm is negotiated per hop and recorded per frame/object. This lets a single frame opt out — e.g. a small JSON merge-patch delta that DEFLATE would only enlarge — and lets different hops use different algorithms.
Previously compression was a single per-track algorithm applied to every frame on a compressing hop. That forced a conformant publisher to sometimes ship frames larger than necessary, and an end-to-end algorithm can't be honest once different hops negotiate different techniques.
moq-lite
Publisher Compressionin TRACK_INFO becomes a boolean hint (0/1; reserved values>1are treated as1, so the hint stays additive).Compressionparameter: each endpoint advertises the algorithms it can decompress, negotiated per hop and per direction.Compressionfield in FRAME and datagram bodies, present only when the hint is set, naming the algorithm actually used (none/deflate).moq-compression
COMPRESSIONtrack property becomes a boolean hint.COMPRESSION_ALGORITHMobject property overrides the hop default per object (typicallynone, to opt out).One design call worth a look
The two layers encode the per-frame/per-object signal differently, on purpose:
Compressionfield is a cheap inline byte, present on every frame of a compressible track.Both share the same model — boolean hint + per-hop negotiation + algorithm-used-per-unit — but the encoding differs to suit each layer's cost. If you'd rather the two be byte-for-byte identical (always-present tag in moqt too), that's an easy change.
Both drafts build cleanly with
kramdown-rfc. Changelog updated formoq-lite-05; the compression extension is unreleased, so no changelog entry.🤖 Generated with Claude Code
https://claude.ai/code/session_01W8bLV6vHzucLNvDhPk3bMP
Generated by Claude Code