Early Review of draft-ietf-cellar-ffv1-03
review-ietf-cellar-ffv1-03-genart-early-miller-2018-07-05-00

Request Review of draft-ietf-cellar-ffv1
Requested rev. no specific revision (document currently at 07)
Type Early Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2018-06-30
Requested 2018-05-29
Requested by Michael Richardson
Other Reviews Secdir Early review of -02 by Liang Xia (diff)
Comments
We are going to WGLC on this in a week.
This is an Informational document (status will be fixed in -03), of a file format that is already common.
Another document (draft-ietf-cellar-ffv1-v4) is standards track and is coming soon.
This document is from a group of open source coders, and this is their first IETF experience, so please be extra constructive.
Review State Completed
Reviewer Matthew Miller
Review review-ietf-cellar-ffv1-03-genart-early-miller-2018-07-05
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/4gDYIz2mNgxdHMSxnTV09LnQrtM
Reviewed rev. 03 (document currently at 07)
Review result On the Right Track
Draft last updated 2018-07-05
Review completed: 2018-07-05

Review
review-ietf-cellar-ffv1-03-genart-early-miller-2018-07-05

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-cellar-ffv1-03
Reviewer: Matthew A. Miller
Review Date: 2018-07-05
IETF LC End Date: N/A
IESG Telechat date: N/A

Summary:

This document is moving in the right direction, but needs
work.  Overall, the document feels unfinished.  It's clear
a lot of work has gone into this, and there's been tremendous
effort put into the details.  However, it's lacking some
clarity, that was present in older revisions that were removed;
speculatively is it due to more generic coverage that
later revisions covered?

The introduction is quite helpful in answering the inevitable
question "What is a FFV1?".  For the most part, the flow of
the document seems to make sense.

Major issues:

I can understand relying on pseudo-code for something very math-
and/or algorithm-heavy, but some prose would be quite helpful in
understanding how and why the parts relate to one another.  The
Definitions section is essentially what I would look for, but only
accounts for some of the terms used within the rest of the
document.  As written, this document depends entirely on the
reader being intimately familiar with the subject matter.

Minor issues:

* Please consider moving the text of Section 2.2.6. "Pseudo-code"
up a level to 2.2. "Conventions" or as the first sub-section under
2.2. "Conventions".  This points seems to me to warrant more
significance than it currently has.

* In reading this, I wondered if it would help improve the
understanding of this document if Sections 3. and 4. swapped
places.  I think it's worth considering, but accept this
suggestion could be rejected.

* Please consider moving Section 4.8. "Parameters" to immediately
proceed from Section 4.1. "Configuration Record".  I think it
would help with understanding the document.

* Please consider moving the ASCII diagram of a Frame from
Section 9.1.1. "Multi-threading support and independence of slices"
to Section 4.2. "Frame".

Nits/editorial comments:

* In Section 2.1. "Definitions", a short description of what a
"Frame" and "Slice" are conceptually would be very helpful.

* In Section 2.2.4. "Mathematical functions", the definition of
"ceil(a)"" appears to be a copy from "floor(a)"; I would suggest:

   """
   ceil(a) the smallest integer greater than or equal to a
   """

* In Section 2.2.6. "Pseudo-code", the word "as" ought to be
"and" in "as uses its".

* The first use of "JEP2000-RCT" (in Section 3.3.) is not
immediately followed with its reference.

* In Section 3.7.2. "RGB", there is a paragraph that is solely
""[ISO.15444-1.2016]"", almost as if it's a reference that was
meant a section heading.

* In Section 3.8.1. "Range coding mode", there appears to be some
odd formatting with "_G. Nigel_ and "N. Martin_"; is this an
attempt to italicize?

* Most of the subsection contents with Section 4. seem to have
extraneous newlines (e.g., 4.1.1. "reserved_for_future_use").
  
* In Section 4.4.9. "sar_den", the text is identical to the
preceding section.  I think this is meant to be the denominator
to complement "sar_num" as the numerator.

* In Section 4.5.1. "primary_color_count", the pseudo-code is
inline with no quotes, which is inconsistent with the rest of the
document.

* In Section 4.7.1. "slice_size", the "Note:" appears to be two
sentence fragments.  I think the following conveys the same
meaning:

   """
   Note: this allows finding the start of slices before previous
   slices have been fully decoded, and allows parallel decoding as
   well as error resilience.
   """

* Section 11. "ToDo" still as one item remaining.