Re: [quicwg/base-drafts] Editorial suggestions from Benjamin Kaduk's IESG review (qpack) (#4789)

[afrind <[e-mail]>]

@afrind commented on this pull request.

Thank you for your detailed review and this PR.  I have one suggestion.

> +state memory it uses and signal a lower dynamic table size than the decoder
+allows to propagate that fact to the decoder (see {{eviction}}).

Suggestion:

 An encoder can limit the amount of state memory it uses by choosing a smaller dynamic table size than the decoder allows, and signaling this to the decoder (see {{set-dynamic-table-capacity}}).

> @@ -1738,7 +1740,7 @@ Stream: Encoder
                               ^-- acknowledged --^
                                4   0  :authority  www.example.com
                                5   0  custom-key  custom-value2
-                              Size=215
+                              Size=214

This looks right too, but it makes me want to re-run the examples through the code I wrote to verify (I made @martinthomson's changes by hand).

> @@ -1787,8 +1789,8 @@ for line in field_lines:
 
 # encode the prefix
 if requiredInsertCount == 0:
-  encodeIndexReference(prefixBuffer, 0, 0, 8)
-  encodeIndexReference(prefixBuffer, 0, 0, 7)
+  encodeInteger(prefixBuffer, 0, 0, 8)
+  encodeInteger(prefixBuffer, 0, 0, 7)

This is a good catch, thanks.  The prefix is integers, not references.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4789#pullrequestreview-574654039