-
Notifications
You must be signed in to change notification settings - Fork 35
Count object extension headers in bytes, not fields #723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This makes it possible to skip those without parsing, and easier to parse without trial decoding. Fixes moq-wg#718
draft-ietf-moq-transport.md
Outdated
* Object Extension Count: The number of Object Extensions present. A value of 0 | ||
indicates that no Object Extension Headers are present. | ||
* Object Extension Size: The total size of the Object Extension Headers block, | ||
in bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to update Canonical Field section as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the Canonical Object Fields section, though?
draft-ietf-moq-transport.md
Outdated
@@ -2680,7 +2680,7 @@ STREAM_HEADER_GROUP { | |||
} | |||
{ | |||
Object ID = 0 | |||
Extension Count = 2 | |||
Extension Headers Size = 33 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:) did you count the bytes to be 33 :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I did it in a calculator, so it may be not 33 (and of course, varint serialization is ambiguous).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Individual Review:
lgtm.
Do we need to specify an error when the length of the extensions block exceeds the Extensions Length?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This makes it possible to skip those without parsing, and easier to parse without trial decoding.
Fixes #718