[Gen-art] RE: Gen ART review of draft-ietf-avt-compact-bundled-evrc-09.txt

[[e-mail]]

Qiaobing,

-- Draft Title

> Section 3.1 is essentially copied from RFC3558 word by word and dealt
> with in the same way here.
> 
> Since specifying a widely usable RTP format is NOT the focus of this
> work, we'd propose to remove section 3.1 and keep the current title.

If the authors believe that the formats defined here are essentially
only useful for the EVRC codecs, then removing Section 3.1 would be
fine.

-- Magic Number representation

> > 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" .
> 
> 
> We can either remove it as suggested, or change it as follows
(following
> the way it is dealt with in section 5.1 in RFC3267):
> 
>        "#!EVRC-B\n"
>        (or 0x2321455652432d420a in hexadecimal).

That'll work, and breaking it up into octets like this might be
easier on the eyes:

	0x2321 0x4556 0x5243 0x2d42 0x0a 

This is your choice - removing the quotes was a matter of correctness,
whether to break it up into octets or not is a matter of preference.

-- \n in the Magic Number

> >    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.
> 
> This text is modeled (almost word by word) after section 5.1 of
RFC3267.
> The visual presentation is already different ("#!EVRC-B" vs.
"#!EVRC"), 
> the Note is just meant to be a friendly reminder to an inexperienced 
> programmer, which may be unnecessary. We can remove it if you feel 
> necessary.

My concern is with: "otherwise, the EVRC-B magic number
will become indistinguishable from magic number used for EVRC."
That text appears to be incorrect, as the real issue appears to be with
the existing magic number for EVRC.  The following text would make this
clear:

    Note, the "\n" is an important part of both this magic number and
the 
    "#!EVRC\n" magic number defined in Section 11 of RFC 3558 and the
"\n"
    MUST be included in any comparison of either magic number, since,
    otherwise, a prefix of the EVRC-B magic number could be mistaken for
    the EVRC magic number.

The "MUST" in the above text modifies RFC 3558, but this draft already
has
that effect.

-- Changes to Default Values

> > 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.
> 
> A few thoughts:
> 
> 1. Due to the way the 4 parameters are used, even if the two ends
> mistakenly assume a different set of default values, they most likely
> should still be able to inter-operate, with *maybe* a bit suboptimal
> performance in some implementation.
> 
> 2. The note is meant for future-proof but maybe a bit unnecessary
since
> there is no evidence that I am aware of that 3GPP2 is going to change
> the default values soon. We can perhaps remove that note.  (Any spec
with
> default values would face the same situation if for some good reasons
> one or more default value needs to be changed in the future). There is
> nothing special for this document.
> 
> Our proposal is to remove the note.

I don't like that idea - if there are interoperability issues here, they
need to be documented, but if there aren't interoperability issues here,
the MUST was probably wrong.  I would advise consulting your WG chairs
(and possibly the responsible AD) about the importance and potential
impact of this issue, as I'm not sufficiently expert to make the call.

Off the top of my head, I would suggest:
	- using a "SHOULD" for the defaults instead of "MUST" (due to
		your point 1),
	- describe the consequences of changing the defaults (what could
		be suboptimal or broken in what ways), and
	- possibly add a "SHOULD" for explicitly negotiating the desired
		new value instead of changing the default
That way, 3GPP2 will understand the consequences of their actions if
they decide to make changes in this area.

Thanks,
--David

> -----Original Message-----
> From: Qiaobing Xie [mailto:[e-mail]] 
> Sent: Saturday, October 07, 2006 4:33 AM
> To: Black, David
> Cc: [e-mail]; [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; [e-mail]
> Subject: Re: Gen ART review of 
> draft-ietf-avt-compact-bundled-evrc-09.txt
> 
> Hi, David,
> 
> Thanks for the review and comments. Please see my response in-line.
> 
> [e-mail] wrote:
> 
> > I have been selected as the General Area Review Team 
> (Gen-ART) reviewer for this draft (for background on Gen-ART, please
see 
> http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
> > Please resolve these comments along with any other Last Call
comments you may receive.
> > Document:
> >        Enhancements to RTP Payload Formats for EVRC Family Codecs
> >                draft-ietf-avt-compact-bundled-evrc-09.txt
> > 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 3.1 is essentially copied from RFC3558 word by word and dealt
> with in the same way here.
> 
> Since specifying a widely usable RTP format is NOT the focus of this
> work, we'd propose to remove section 3.1 and keep the current title.
> 
> If you STILL feel a change to the title is necessary, we'd 
> like to propose:
> 
> "The RTP Compact Bundled Format and Enhancements to RTP Payload
Formats
> for EVRC Family Codecs"
> 
> >
> > 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" .
> 
> 
> We can either remove it as suggested, or change it as follows 
> (following
> the way it is dealt with in section 5.1 in RFC3267):
> 
>        "#!EVRC-B\n"
>        (or 0x2321455652432d420a in hexadecimal).
> 
> We prefer making the change but will be fine with either approach.
> 
> >
> >    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.
> 
> 
> This text is modeled (almost word by word) after section 5.1 of
RFC3267.
> The visual presentation is already different ("#!EVRC-B" vs.
"#!EVRC"), 
> the Note is just meant to be a friendly reminder to an inexperienced 
> programmer, which may be unnecessary. We can remove it if you feel 
> necessary.
> 
> If you STILL feel the magic number needs to be changed, we'd propose 
> "#!EVCB\n" for EVRC-B.
> 
> >    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.
> 
> Ok.
> 
> >
> > 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.
> 
> Ok.
> 
> >
> > 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.
> 
> Ok.
> 
> >
> > 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.
> 
> A few thoughts:
> 
> 1. Due to the way the 4 parameters are used, even if the two ends
> mistakenly assume a different set of default values, they most likely
> should still be able to inter-operate, with *maybe* a bit suboptimal
> performance in some implementation.
> 
> 2. The note is meant for future-proof but maybe a bit unnecessary
since
> there is no evidence that I am aware of that 3GPP2 is going to change
> the default values soon. We can perhaps remove that note.  (Any spec
with
> default values would face the same situation if for some good reasons
> one or more default value needs to be changed in the future). There is
> nothing special for this document.
> 
> Our proposal is to remove the note.
> 
> >
> > 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.
> 
> Ok.
> 
> regards,
> -Qiaobing
> 
> >
> > Thanks,
> > --David
> > ----------------------------------------------------
> > David L. Black, Senior Technologist
> > EMC Corporation, 176 South St., Hopkinton, MA  01748
> > [phone]             FAX: [phone]
> > [e-mail]        Mobile: [phone]
> > ----------------------------------------------------
> >

_______________________________________________
Gen-art mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/gen-art