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
Seek more clarity on Largest Reference/Base Index encoding #1403
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -557,26 +557,28 @@ one-bit sign and the `Delta Base Index` value. A sign bit of 0 indicates that | |
the Base Index has a greater absolute index than the Largest Reference; the | ||
value of Delta Base Index is added to the Largest Reference to determine the | ||
absolute value of the Base Index. A sign bit of 1 indicates that the Base Index | ||
is smaller than the Base Index. | ||
|
||
That is, after turning a sign bit of 1 to -1 and 0 to 1, the absolute Base Index | ||
is: | ||
is smaller than the Base Index. That is: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Base Index is smaller than the Largest Reference |
||
|
||
~~~ | ||
baseIndex = largestReference + (sign * deltaBaseIndex) | ||
if sign == 0: | ||
baseIndex = largestReference + deltaBaseIndex | ||
else: | ||
baseIndex = largestReference - deltaBaseIndex | ||
~~~ | ||
|
||
An encoder is expected to determine the absolute value of Base Index before | ||
encoding a header block. If the encoder inserted entries in the dynamic table | ||
while encoding the header block, Largest Reference will be greater than Base | ||
Index, so the encoded difference is negative and the sign bit will be 1. If the | ||
header block did not reference the most recent entry in the table and did not | ||
insert any new entries, Base Index will be greater than the Largest Reference, | ||
so the delta will be positive and the sign bit will be 0. | ||
|
||
When Largest Reference and Base Index are equal, the Delta Base Index is encoded | ||
with a zero sign bit. A sign bit set to 1 when the Delta Base Index is 0 MUST | ||
be treated as a decoder error. | ||
A single-pass encoder is expected to determine the absolute value of Base Index | ||
before encoding a header block. If the encoder inserted entries in the dynamic | ||
table while encoding the header block, Largest Reference will be greater than | ||
Base Index, so the encoded difference is negative and the sign bit is set to 1. | ||
If the header block did not reference the most recent entry in the table and did | ||
not insert any new entries, Base Index will be greater than the Largest | ||
Reference, so the delta will be positive and the sign bit is set to 0. | ||
|
||
An encoder that produces table updates before encoding a header block might set | ||
Largest Reference and Base Index to the same value. When Largest Reference and | ||
Base Index are equal, the Delta Base Index is encoded with a zero sign bit. A | ||
sign bit set to 1 when the Delta Base Index is 0 MUST be treated as a decoder | ||
error. | ||
|
||
A header block that does not reference the dynamic table can use any value for | ||
Base Index; setting both Largest Reference and Base Index to zero is the most | ||
|
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.
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.
greater than or equal to, technically. We don't mention the zero case at all until the bottom.
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.
Well the sentence says that the encoder added entries. I don't think that it's a problem to not mention the zero case.
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.
I don't see where this sentence says it added entries - are you referring to somewhere else? I think Mike's right here - if the Base Index is equal to LR then the sign bit will be 0. The original text had that, and it's included on lines 578-9 below. Is it clearer if we bring it back up here?
I agree we don't need to mention the case where they are both 0 here.