Early Review of draft-ietf-cellar-ffv1-03
|Requested rev.||no specific revision (document currently at 17)|
|Team||General Area Review Team (Gen-ART) (genart)|
|Requested by||Michael Richardson|
|Authors||Michael Niedermayer, Dave Rice, Jerome Martinez|
|Draft last updated||2018-07-05|
Secdir Early review of -02 by Liang Xia
Genart Early review of -03 by Matthew Miller (diff)
Secdir Last Call review of -16 by Liang Xia (diff)
Genart Last Call review of -16 by Joel Halpern (diff)
Opsdir Last Call review of -17 by Qin Wu
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.
|Reviewed rev.||03 (document currently at 17)|
|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.