Attached is my review of the specified document, submitted as part of
the Gen-ART process.  For background on Gen-ART, please see the FAQ at
<http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html>.

Document Tag: draft-ietf-avt-rfc2429-bis-08.txt
Document Title: RTP Payload Format for the 1998 Version of ITU-T
		    Rec. H.263 Video (H.263+)
Intended Status:  Proposed Standard

----------------  Begin review  ----------------

Summary: This draft is basically ready for publication, but has nits that
	should be fixed before publication.

Major concerns
--------------

In the beginning of Section 3 it refers to the "payload header"
definition being in Section 3.  I believe this was meant to refer to
Section 5.


Minor comments
--------------

The Title only mentions the 1998 version, the intro says both 1998 and
2000 versions are described, and 1996 is supported by subsetting.  I
suggest just shortening the title to "RTP Payload Format for ITU-T
Rec. H.263 Video" and leave the detail to the content of the document.

In the Intro it says "obsoletes RFC2429" but also says 2190 is now
only for backwards (legacy) use, so it's also obsoleting 2190, that
should be added to the statement.

Immediately after the confused reference to Section 3, that I above
note probably meant to be 5, it talks about "the usage of the RTP
fixed header" being in "this section", which I take to be a reference
to the whole of section 3, but following after the previous confusion,
the referent is just not clear.

The second bullet item in Section 4 has a "should" that probably meant
to be a "SHOULD".  A casual search through the document shows other
places where 2119 words are used in the 2119 sense, but not
capitalized.

The first paragraph of Section 5 refers to "the basic payload
header".  But that's the only occurrence of the word "basic" in the
document.  I think it meant to refer to the "General" header described
in 5.1.  If so, replace the word "basic" with "general" and add
"defined in 5.1" after it.

In section 8.1.1 discussing parameters for indicating Annex support,
"FIJT" can indicate "not supported" either by not being present or by
explicitly as (for example) "F=0".  For consistency, I would suggest
allowing "KNP" to also use "=0" to indicate "not supported".

Specifying 29.97 as the default for CPCF seems odd given that all the
rest of the spec tries so hard at being non-preferential, but that
clearly makes it prefer NTSC to PAL or film or anything else...  But,
I guess you had to pick some default. the only unbiased choice is to
make it required which you can't...

There are occurrences of both "RFCxxxx" and "RFC yyy" which are, I
expect, meant to be self referential.  There should have been a "Note
to RFC Editor" about this.  (And it would have been nice if they'd all
been the same, making the RFCed search-and-replace only once.)


----------------------------------------------------------------
   The following editorial issues are noted for the convenience
   of possible copy editors but are not part of the technical review.

Clarity
-------

Since both the 1998 and 2000 versions of H.263 are referred to,
shouldn't they both appear in the references?

Typos
-----

In Abstract: "document also describe" => "document also describes"

In 3.1: "those timing information" => "that timing information"

In 4: "in such cases in which" => "in cases in which"
   Which is still pretty stilted language and a somewhat more extended
   rewording might be called for.

In 5.2: "damage of individual frame" => "damage to an individual frame"

In 5.2 the picture for the VRC layout has a shifted header.

In 8.1.1 (six times): "Permissible value are" => "Permissible values are"
  it was also hard for me to parse the descriptions, perhaps the
  addition of parens could help, i.e.
	"maximum frame rate of 29.97/ the specified value frames per
	second." => "maximum frame rate of (29.97 / the specified
	value) frames per second."

In 8.1.1: "A system that support a" => "A system that supports a"

In 8.1.1: "specify it support" => "specify its support"

In 8.1.1: "than the stream" => "then the stream"

In 8.1.1: "a number fro 1 to 4" => "a number from 1 to 4"

In 8.1.1: 'SHALL have the values "1"' => 'SHALL have the value "1"'

In 8.1.2, the def for INTERLACE appears to either have extra words or
	missing words, or maybe just missing punctuation.  In any
	case, I couldn't parse it into meaningful English...although
	from experience I know what it means to say...

In 8.2.1: "when send in a" => "when sent in a"

In 8.2.1: "support a profile" => "supports a profile"

In 8.2.1: "is able of" => "is capable of"  (I think)

_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www1.ietf.org/mailman/listinfo/gen-art