[Cellar] draft-ietf-cellar-ffv1-13 comments

["Murray S. Kucherawy" <[e-mail]>]

Hi there,

I've reviewed -13 of this document, and the results are below.  I also used
Adam's prior AD review as a checklist of sorts.

I think this needs one more round before it can be sent to the IESG.
Fortunately most of this is editorial, but there's a lot, and there are one
or two substantive things I urge you to address.

Section 2.1:
* "RCT" doesn't appear as a standalone term in this document other than as
part of the string "JPEG2000-RCT", so I don't know if it's worth including
in your glossary.
* "TBA" is not used in this document other than in the glossary.  It should
probably be removed.

Sections 2.1 and 3.5:
* "1 or more" should become "one or more".  Similarly, "1 of the 5" should
be "one of the five".

Section 2.2.2:
* The sentence starting "With" should be attached to the previous sentence
with a comma.  It's not a sentence by itself.
* "This is equivalent to, b times dividing a by 2 with rounding toward
negative infinity." -- I suggest: "This is equivalent to dividing a by 2, b
times, with rounding toward negative infinity."

Section 2.2.5:
* The layout of this table is a little hard to parse.  I would quote the
terms you're describing, or put colons after them, or something like that.

Section 3.3:
* The "Background" paragraph starts out talking in the past tense, then
switches to present ("was used", then "when the issue is discovered").
Then it switches back in a later sentence.  That should all be past tense,
right?
* It looks a little odd to quote an equation, especially where that's not
done elsewhere in the document so far.

Section 3.7.2:
* The spacing around and within the equations in this section is not
consistent with the rest of the document.
* "offsetted" should be "offset"

Section 3.8.1.1.1:
* "3 modes" should be "three modes"
* "1 byte shorter" should be "one byte shorter"
* "3 places" should be "three places"
* "Above describes the range decoding, encoding is defined as ..." -- this
should be two separate sentences

Section 3.8.1.2:
* As Adam mentioned in his review: "The exact contexts used are best
described by Figure 15, followed by some comments.", but there are no
comments after Figure 15 nor after this prose.
* Adam also pointed out that this table appears to have a "type" column
that serves no purpose.  It should either be used or removed.  Or, if as I
think I figured out much later, this layout is actually a way to indicate
the types of output values when the pseudo-code yields something, you
really should explain that either here, or maybe earlier where you're
describing conventions used in this document.
* Adam also pointed out that the put_rac() function is defined nowhere in
this document nor is a definition for it referenced.

Section 3.8.2.1:
* "2 parts" should be "two parts"
* "the prefix stores ..." should be the start of a new sentence.
* Another table with an unused "type" column.

Section 3.8.2.1.2:
* The prose in the ESC line uses too many commas.  Maybe: "the value - 11,
in MSB first order; ESC may only ..."

Section 3.8.2.1.3:
* Same issue about "value".

Section 3.8.2.2.1:
* "2 parts" -- "two parts"; "2nd' -- "second"
* "the prefix" should begin a new sentence
* another table with an unused "type" column

Section 3.8.2.2.2:
* This code uses a function defined in the next section.  They should be
swapped perhaps, or at least a forward reference would be helpful.
* The final paragraph appears to be made up of two sentences run together.

Section 3.8.2.3:
* sign_extend() is undefined.

Section 4:
* "1 or more" -- "one or more"
* This is the first time there's a reference to a "header".  Is more
context needed here?

Section 4.1:
* QuantizationTableSet() isn't defined until much later in the document.
Again, a forward reference would be helpful.

Section 4.1.1:
* "... version 2 files SHOULD NOT exist ..." -- what's an implementer
supposed to do with this normative instruction?

Section 4.1.3:
* "... there is no known implementations ..." -- change "is" to "are", or
use "implementation"

Section 4.1.5:
* The normative language at the end here is pretty crisp, but what should
an implementation do if it receives something that doesn't comply?

Section 4.1.7:
* The sentence at the end appears to be two sentences run together.

Section 4.1.8, 4.1.9:
* The spacing in the equations here isn't consistent with the rest of the
document.

Section 4.1.15:
* This conditional syntax is incomplete since the ":" part is missing.

Section 4.1.16:
* As Adam pointed out, another unexplained reference to "header" ("global"
in this case).

Section 4.2:
* First-ever reference to a "track header".

Section 4.2.2:
* Change "crc" to "CRC" (two instances).  It's correct the third time.

Section 4.2.3.1:
* "AVI " should be just "AVI" (i.e., remove the trailing space)

Section 4.4:
* Addressing normative language to authors of revisions to this document is
novel.  I suggest using non-normative prose here.
* "an other" should be "another"

Section 4.5.5:
* Rather than separating this formula away from the text as was done in
prior sections, here it's quoted inline.  I find the former much more
readable.

Section 4.6:
* Another table with an empty "type" column.  I think I see now that the
column is populated against each line that outputs something, but can it
not just be omitted when there's no output to the function?
* But does this function really output nothing?

Sections 4.6.1 through 4.6.4:
* Same comment as before about inline formulae.

Section 4.7:
* This should have indicators for output types, I think.

Section 4.7.1:
* I don't understand the second sentence.  I parse it as: "X" and "Y" value
is "Z".  Did you mean "The values of X and Y are Z"?
* Same problem on the next line, coupled with quoted inline equations again.

Sections 4.7.2 and 4.7.3:
* Same issue with quoted inline equations.

Section 7:
* Does the subtype intend to be case-sensitive?  i.e., couldn't "ffv1" work?
* Required parameters should be "N/A" if you have none to register.
* "This parameter is used to signal ..." -- to what is this referring?

Section 8:
* This can be merged with Section 7.

Section 9:
* Appendices go after the references.

-MSK