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

"Murray S. Kucherawy" <superuser@gmail.com> Wed, 29 April 2020 05:15 UTC

Return-Path: <superuser@gmail.com>
X-Original-To: cellar@ietfa.amsl.com
Delivered-To: cellar@ietfa.amsl.com
Received: from localhost (localhost []) by ietfa.amsl.com (Postfix) with ESMTP id 26DCB3A0779 for <cellar@ietfa.amsl.com>; Tue, 28 Apr 2020 22:15:44 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.854
X-Spam-Status: No, score=-0.854 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, NORMAL_HTTP_TO_IP=0.001, NUMERIC_HTTP_ADDR=1.242, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([]) by localhost (ietfa.amsl.com []) (amavisd-new, port 10024) with ESMTP id WVnTuxfAE94M for <cellar@ietfa.amsl.com>; Tue, 28 Apr 2020 22:15:41 -0700 (PDT)
Received: from mail-vs1-xe36.google.com (mail-vs1-xe36.google.com [IPv6:2607:f8b0:4864:20::e36]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C437B3A0774 for <cellar@ietf.org>; Tue, 28 Apr 2020 22:15:40 -0700 (PDT)
Received: by mail-vs1-xe36.google.com with SMTP id g184so519681vsc.0 for <cellar@ietf.org>; Tue, 28 Apr 2020 22:15:40 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to:cc; bh=EDhGJulUDkpT2KVL4Ia8weZFt0qfh973ELtKYfRC6JA=; b=ArGVMn/WVU50l6/HHByTQt7JyKLB9q+3uQ932conucVEiKGToPKQJwRqCczb46Xr0S OFMOY1mLJ3ntJLh6LJS0rQturVb66Yy/ivg0enjILui2tzAdLzXhIISSeRPfSCCDjQjQ DeRkig8fVDtOyiua8Ky7CuFy8B/upfUNkN3LCv73jsxT/qgY3AFZK83oNLU6IkVS9StG eF3+D7OZshbooVBg2Td9XEweexXK6RqmCzft0Ck4Wrz5Uccn/yNeB07tiYnCAMDHw+rS BhOyIp3+1ECAa7G6JFkCCFPhFrVmgo3zyBZvwxQ5K6wZqGt1dfOJFYzoLHsi+pJTPkUW tuVw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=EDhGJulUDkpT2KVL4Ia8weZFt0qfh973ELtKYfRC6JA=; b=DoUa3Gk5oDBLGQ8JzSHhXyAWma+Nrqr9n1WTVfgl0xi73sHfj83vvgbZbu0MHktXIw xkOIr2UEaGl6XIKxn2uAEW8S166p6Rx2T0rtuCSlsYYbMHZuhLkhQoesKxHkGUXa2p5Y dzztygs/3hmJ2PORFNxXIPcitWARzvjOdMW4wVrBkaJJYfHRowuXVHCm/SlbhRBge4Z4 1sGX2mKUUgvxV6Aowk6dSaIIxGc77GlaE0PCmZnwchmt4i5vJvdeEsnF01jJ3LBGakhO L7P9Cu6n44laUnvslCUoq+oqbO2wlbY339Lny+9mhzS6c5DEzQIf5M6a0WYgx6sLV8EO ylNw==
X-Gm-Message-State: AGi0PubQdQvef8paGctkcEHPPZ4QyKuIocXdBn8grbGyzr0Wv4zlUk/u VclpdlfMXvCzQoZBw0Izp/6rEdgcJ6FkoH+qOzreSkw/8EY=
X-Google-Smtp-Source: APiQypIjgpMSksWX4j86s6FF0mdCpam1dr+KvAlNfbceawL99OGe6NVmpYTrv9QGQf1TIms/MKEbULzti7QajthVaf8=
X-Received: by 2002:a05:6102:208a:: with SMTP id h10mr23468622vsr.13.1588137339119; Tue, 28 Apr 2020 22:15:39 -0700 (PDT)
MIME-Version: 1.0
From: "Murray S. Kucherawy" <superuser@gmail.com>
Date: Tue, 28 Apr 2020 22:15:28 -0700
Message-ID: <CAL0qLwbaq=Xu8-3tiWeTn9EZighL2n_EHf9LsKNH+U-sN-Uqow@mail.gmail.com>
To: cellar@ietf.org
Cc: Adam Roach <adam@nostrum.com>
Content-Type: multipart/alternative; boundary="000000000000f3f03a05a4670915"
Archived-At: <https://mailarchive.ietf.org/arch/msg/cellar/K1girgkVr4AKNDzPcUPyRjla7Rc>
Subject: [Cellar] draft-ietf-cellar-ffv1-13 comments
X-BeenThere: cellar@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Codec Encoding for LossLess Archiving and Realtime transmission <cellar.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/cellar>, <mailto:cellar-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/cellar/>
List-Post: <mailto:cellar@ietf.org>
List-Help: <mailto:cellar-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/cellar>, <mailto:cellar-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 29 Apr 2020 05:15:45 -0000

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,
* 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"

* "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

* 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.

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

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

* Same issue about "value".

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

* 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.

* 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

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.

* "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

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.