Document: draft-ietf-avt-compact-bundled-evrc-09.txt 
Title:  Enhancements to RTP Payload Formats for EVRC Family Codecs
Reviewer: David L. Black
Review Date: September 29, 2006
IETF LC Date: Ends October 9

Summary: This draft is on the right track, but has open issues
described in the review.

Comments:

This draft is in good shape.  There are two relatively small open
issues (marked with *** below):
- Magic number use of \n
- Ability to change defaults
Everything else is editorial.

I think the draft should have it's title edited to better represent
what it specifies.  The title of the draft is:

       Enhancements to RTP Payload Formats for EVRC Family Codecs

but Section 3.1 says:

   Other frame-based vocoders can be carried in the packet formats
   defined in this document, as long as they possess the following
   properties:

This indicates that the new format is more widely usable.  There's
only one new format, so a better draft title would be:

  The RTP Compact Bundled Format and Additional EVRC Family Codecs

Also, since there's only one packet format defined in this draft,
change "packet formats" to "packet format" in the text quoted from
Section 3.1 above.

Section 5: Remove the quotes surrounding the hexadecimal form of
the magic number in:

	"0x23 0x21 0x45 0x56 0x52 0x43 0x2D 0x42 0x0A"

as that's not supposed to be a text string - the text string
is given earlier as "#!EVRC-B\n" .

   Note, the "\n" is an important part of the magic number and MUST be
   included in the comparison, since, otherwise, the EVRC-B magic number
   will become indistinguishable from magic number used for EVRC.

*** That's somewhat sloppy.  Can this magic number be changed to
actually have a different visual presentation, as opposed to only
differing in a non-printable character that is often stripped in
text string processing?  It would be ok (but disappointing) if this
were not possible due to the amount of installed base/running code
using that magic number.

   The ToC field is extended to one octet by setting the four
   most significant bits of the octet to zero.  For example, a
   ToC value of 4 (a full-rate frame) is stored as 0x04.

Explain where the ToC values come from.  Citing Section 5.1 of RFC
3558 should be sufficient.

Section 6: Add a note at the start of this section:

-- RFC Editor: Please replace all instances of "RFC XXXX" in
this section with the RFC number of this document prior to
IANA registration and RFC publication, and remove this note.

In the definition of silencesupp:

         If this parameter is not present, the default value specified
         in [8] MUST be assumed.

Say what that default value is, especially as [8] is a 3GPP2
document, not an IETF document.

In the definition of dtxmin:

         If this parameter is not present, the default value, 12, as
         specified in [8] MUST be assumed (note, if a later version of
         [8] specifies a new, different value, the new value will take
         precedence). 

*** That's better, but the ability of 3GPP2 to change the default looks
like it could cause an interoperability problem (applies to silencesupp,
dtxmin, dtxmax, and hangover).  How will this be addressed?  This needs
to be explained.

Sections 6.5 and 6.6 should instruct IANA to change the registrations
of EVRC and EVRC0 to reference this RFC instead of RFC 3558.