hi Adding these clarifications to the document itself would be helpful. Of particular concern would be the ambiguities identified in 4, 6, 10, 11, 12, 13, 14. You may not think they identify a technical problem because you can clarify them here, but if two different implementations come away with two different interpretations of the draft/RFC then you won't achieve interoperability and then that is a technical problem.
For 15 I would suggest perhaps adding it in the list of terms. DTX is talked about much earlier in the document then where it is introduced. Sharon -----Original Message----- From: Colin Perkins [mailto:[EMAIL PROTECTED] Sent: Tuesday, July 18, 2006 12:34 PM To: Chisholm, Sharon (CAR:ZZ00) Cc: [email protected]; Taylor, Tom-PT (CAR:5N00); Roni Even; Brian E Carpenter; SOLLAUD Aurelien RD-TECH-LAN; Magnus Westerlund Subject: Re: Review of draft-ietf-avt-rtp-g729-scal-wb-ext-06 Sharon, Gen-ART, On 28 Jun 2006, at 14:33, Sharon Chisholm wrote: > I am the assigned Gen-ART reviewer for: > > draft-ietf-avt-rtp-g729-scal-wb-ext-06.txt > > For background on Gen-ART, please see the FAQ at > <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. Thanks for the review - some comments inline. Colin > --------------------------------------------------------------------- > > This draft is on the right track but has open issues, described in the > review. It does not appear to be quite ready for publishing as an RFC > at this time. > > I have included below several comments and questions that should be > resolved or clarified. > > Comments: > ======== > > 0. In general, I thought the layout of the document could use some > work > to ensure that implementers more predictably knew everything that was > required of them. This version of the document required both a > thorough > read and a certain amount of inferring to piece together what I > believe > are the requirements. Concrete suggestions are always welcome, however the layout does match other RTP Payload Format specifications. > 1. In section 2, the third paragraph isn't a well-formed sentences and > leaves open some ambiguities such as who will be tuning the bandwidth. > > 2. In section 2, fourth paragraph, what is a graceful quality > improvement? > > 3. In section 2, last paragraph and again in section 6.2.1 (page 11), > there are sentences that start with "Anyways, ", which I don't think > makes sense. These all seems to be minor editorial issues in the informative background. We can fix in AUTH48, unless there is another editing round for other reasons. > 4. In section 3, second paragraph, is it suggesting that this > feature is > helpful when sharing a fixed bandwidth or suggesting that people > should > share a fixed bandwidth? It reads like the latter. The intent is to say that, if you have a fixed bandwidth link, the ability to adjust the bandwidth may help. > 5. In section 3, second paragraph, replace "It is described in the > following payload format (see > Section 5.2)" with "It is described in section 5.2." Editorial. RFC Editor will fix. > 6. In section 4, second paragraph, "The RTP timestamp clock > frequency is > the same as the default sampling frequency, that is 16 kHz" implies a > significant relationship between the RTP timestamp and the default > sampling frequency, but isn't it just a coincidence they have the same > value? No. They are defined to have the same value, to ensure accurate media playout. > I would suggest replacing this sentence with something like "The RTP > timestamp clock frequency is 16 KHz" unless there is in fact a > relationship between these two values. If there is a relationship, I > would suggest explaining what it is. The text you quote explains the relationship: the RTP timestamp clock frequency is the same as the default sampling frequency. I don't believe any change is needed here. > 7. In section 4, in fifth and sixth paragraph, if my understanding is > correct: A non-DTX implementation MUST do all zeros and DTX > implementation MAY do all zeros, but SHOULD set it to one. No. An non-DTX implementation MUST always set the M bit to zero. A DTX-aware implementation should set the M bit to one in "the first packet after a silence period during which packets have not been transmitted contiguously" and set it to zero in all other packets. > So, I guess this M bit isn't meant to tell me whether DTX is used? The M bit is set to indicate the first packet after a silent period, as is usual for RTP Payload Formas. > (In a few pages we do learn there is another field for this). There is an media type parameter to signal the ability to use DTX, but that is separate from the M bit in the RTP header. > Does the average reader know under what circumstances the DTX > implementation would chose to set the M bit to zero? Yes, since they match the usual semantics for other audio RTP payload formats. > 8. In section 4, final paragraph it says "It is expected that the RTP > profile under which this payload format is being used will ...." which > seems too weak if it is required to get all this to work. Are there > specific requirements against the RTP profile used? Should the profile > be compliant to requirements in section 6.2? This is the standard wording, used in several dozen other RTP Payload Format RFCs. It has a well understood meaning, and many independent & interoperable implementations exist. > 9. In section 5.1, first paragraph it would be helpful to indicated > that > the payload header consists of two fields - MBS and FT. The text just > indicates there is a header but the figure has no header, just the MBS > and FT fields and then the following section jumps in and starts > talking > about the details of MBS and FT. Seems like a minor editorial issue, but okay. > 10. In section 5.1, first paragraph, it indicates that the header is > "generally followed by audio data representing one or more consecutive > frames at the same bit rate." My initial read of this was that > occasionally it doesn't contain audio data, but after reading the > entire > document and briefly thinking perhaps it also could carry video (see > section 6.1, "Applications which use this media type"), I then > ending up > on audio-only in section 8, second paragraph. So, this means that > either > the 'generally' is incorrect or the variable in this sentence is > actually the "one or more consecutive frames at the same bit rate"? The diagram in section 5.1 explains that there may be zero or more frames following the header. The "generally followed" text is correct, since a packet containing zero frames (i.e. no audio data) is legal. > 11. In section 5.2, fifth paragraph we learn that we need to ignore > invalid values in an MBS but at this point have no idea what an > invalid > value is or how one might learn more. Later in the document, we did > learn that an MBS can't exceed the maxbitrate field, but I think > for the > offer-answer model, there may have been other restrictions. I may have > missed some. When to ignore a field seems important so it would be > better if this was clearer in the document. Values 12-14 are reserved, and hence invalid. This should probably be clarified. > 12. In section 5.3, final paragraph, I have a similar comment > around the > FT. It appears later on in the document that this also can't imply a > rate greater then the maxbitrate, but are there other invalid values? As above. > 13. In section 5.4, if FT=15, then there will be no audio frame in the > payload. Then what will there be? Is this an extension of my confusion > in point 10 or is this a case of sending only header information > and no > payload? It is header information only, no payload, as explained in my answer to your point 10. > 14. In section 6, second paragraph, it mentions a mapping for SDP and > what to do if it isn't SDP or MIME, but what happens if it is MIME? If it is MIME, standard MIME mechanisms are used, as defined in section 6.1. > 15. The definition of DTX in section 6.1 would have been useful > earlier > in the document. Where? The earlier document is about the RTP packet headers, this is about media type parameters. Putting it earlier would just confuse. > 16. Suddenly in section 6.1, we start referring to sections in this > document using the expected RFC number. Why? I imagine this will be > confusing to the reader. Section 5.2 and section 5.2 in RFC XXXX will > seem different. Standard procedure, since IANA may extract the MIME registration template from the rest of the document. > 17. In section 6.1, the "Person & email address to contact for further > information" is listed as "[EMAIL PROTECTED]", will > this stand the test of time? I doubt it, but that is the required information according to RFC 4288. If you think it incorrect, I suggest you fix the registration procedures. > 18. In section 6.2.1, it isn't explicitly stated how to state that you > 'prefer' G.729.1. Only through the examples do you get that it was by > including the 98 in audio field, and presumably before the 18 to > indicated the preference over G.729. Is this something well-understood > by the average reader? Yes. > 19. In section 7, first paragraph, replace "see Section 3", with > something like "see Section 3 for more detail". Trivial editorial issue. > 20. In section 7, second paragraph, although it isn't explicitly > stated, > it seems we are implying that a good method of congestion control > is to > minimize the header-overhead of a session over time? It would be > good to > explicitly state this. I don't know what is generally considered good > practice for discussions on congestion control, but this seems a bit > weak. While weaker than I would like, it matches other RTP payload format RFCs. We don't yet have a good understanding of how to do congestion control for media streams, so more concrete recommendations are difficult. Colin _______________________________________________ Gen-art mailing list [email protected] https://www1.ietf.org/mailman/listinfo/gen-art
