Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Seek more clarity on Largest Reference/Base Index encoding #1403

Merged
merged 4 commits into from Jun 4, 2018
Merged

Seek more clarity on Largest Reference/Base Index encoding #1403

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5c3a6c1
Seek more clarify on Largest Reference/Base Index encoding
martinthomson May 30, 2018
43df5c3
Tweaks from review
martinthomson May 31, 2018
fcd33ba
Alan's comments
martinthomson Jun 1, 2018
dae9232
Include equal to, can't be less than itself
martinthomson Jun 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 30 additions & 11 deletions draft-ietf-quic-qpack.md
View file
Open in desktop
Expand Up @@ -550,18 +550,37 @@ the block. Blocking decoders use the Largest Reference to determine when it is
safe to process the rest of the block.

`Base Index` is used to resolve references in the dynamic table as described in
{{indexing}}. To save space, Base Index is encoded relative to Largest
Reference using a one-bit sign flag.
{{indexing}}.

baseIndex = largestReference + deltaBaseIndex
To save space, Base Index is encoded relative to Largest Reference using a
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
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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.

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.

If the encoder inserted entries to the table while the encoding the block,
Largest Reference will be greater than Base Index, so deltaBaseIndex will be
negative and encoded with S=1. If the block did not reference the most recent
entry in the table and did not insert any new entries, Largest Reference will be
less than Base Index, so deltaBaseIndex will be positive and encoded with S=0.
When Largest Reference and Base Index are equal, deltaBaseIndex is 0 and encoded
with S=0.
That is, after turning a sign bit of 1 to -1 and 0 to 1, the absolute Base Index
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the language about turning a sign bit somewhat confusing.

Is it more clear to explain using a conditional?

if sign:
  baseIndex = largestReference - deltaBaseIndex
else:
  baseIndex = largestReference + deltaBaseIndex

Or index into an array?

signMultiplier = [ 1, -1 ]
baeIndex = largestReference + signMultipler[ sign ] * deltaBaseIndex

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is always hard to explain, but the conditional is nice. Verbose FTW.

is:

~~~
baseIndex = largestReference + (sign * deltaBaseIndex)
~~~

An encoder is expected to determine the absolute value of Base Index before
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A single-pass encoder

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is vs. will be - can it be consistent?

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 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
efficient encoding.


### Instructions
Expand Down Expand Up @@ -697,7 +716,7 @@ represented as an 8-bit prefix string literal.

# Encoding Strategies

## Single pass encoding
## Single Pass Encoding

An encoder making a single pass over a list of headers must choose Base Index
before knowing Largest Reference. When trying to reference a header inserted to
Expand Down