[Gen-art] Genart early review of draft-ietf-cellar-ffv1-03

Matthew Miller <linuxwolf+ietf@outer-planes.net> Thu, 05 July 2018 21:00 UTC

Return-Path: <linuxwolf+ietf@outer-planes.net>
X-Original-To: gen-art@ietf.org
Delivered-To: gen-art@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id BA79C130F89; Thu, 5 Jul 2018 14:00:33 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Matthew Miller <linuxwolf+ietf@outer-planes.net>
To: gen-art@ietf.org
Cc: draft-ietf-cellar-ffv1.all@ietf.org, cellar@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.81.3
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <153082443369.26281.17163340643478720739@ietfa.amsl.com>
Date: Thu, 05 Jul 2018 14:00:33 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/4gDYIz2mNgxdHMSxnTV09LnQrtM>
Subject: [Gen-art] Genart early review of draft-ietf-cellar-ffv1-03
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.26
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 05 Jul 2018 21:00:34 -0000

Reviewer: Matthew Miller
Review result: On the Right Track

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.