Below one comment [SV]. -----Original Message----- From: ext Alexey Melnikov [mailto:[email protected]] Sent: 28 February, 2013 12:36 To: Veikkolainen Simo (Nokia-CTO/Espoo) Cc: [email protected]; [email protected]; [email protected] Subject: Re: Gen-ART LC review of draft-ietf-mmusic-sdp-cs-17
On 28/02/2013 07:31, [email protected] wrote: > Hello Alexey, Hi Simo, Thank you for the response. > Thank you for the review. Please find my answers inline [SV]. > > -----Original Message----- > From: ext Alexey Melnikov [mailto:[email protected]] > Sent: 12. helmikuuta 2013 15:20 > To: [email protected] > Cc: [email protected]; The IESG > Subject: Gen-ART LC review of draft-ietf-mmusic-sdp-cs-17 > > I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, > please see the FAQ at > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > > Please resolve these comments along with any other Last Call comments you may > receive. > > Document: draft-ietf-mmusic-sdp-cs-17 > Reviewer: Alexey Melnikov > Review Date: 2013-02-12 > IETF LC End Date: 2013-02-01 > IESG Telechat date: N/A > > > Summary: This draft is ready for publication as a Standard Track RFC (with > some nits/minor issues): > > Major issues: > > Minor issues: > > 5.2.2. Media Descriptions > > When "RTP/AVP" is used in the <proto> field, the <fmt> subfield > contains the RTP payload type numbers. We use the <fmt> subfield to > indicate the list of available codecs over the circuit-switched > bearer, by re-using the conventions and payload type numbers defined > for RTP/AVP. The RTP audio and video media types, which, when > applied to PSTN circuit-switched bearers, represent merely an audio > or video codec. The endpoint SHOULD only use those payload type > whose corresponding codecs is available for PSTN media streams. > > Why is this a SHOULD? What are possible reasons for violating it? > > [SV] This is clarified in the following paragraph: > > In some cases, the endpoint is not able to determine the list of > available codecs for circuit-switched media streams. In this case, > in order to be syntactically compliant with SDP [RFC4566], the > endpoint MUST include a single dash ("-") in the <fmt> subfield. > > [SV] Other option is to say that in cases where the endpoint is able to > determine the RTP payload numbers, it MUST use those, and in cases when it is > not, it MUST use the "dash". I think this would be much better! [SV] Ok, we'll change the text as: If the endpoint is able to determine the list of available codecs for circuit-switched media streams, it MUST use the corresponding payload type numbers in the <fmt> subfield. > 5.6.1. Generating the Initial Offer > > The <fmt> subfield carries the payload type number(s) the endpoint is > wishing to use. Payload type numbers in this case refer to the > codecs that the endpoint wishes to use on the PSTN media stream. For > example, if the endpoint wishes to use the GSM codec, it would add > payload type number 3 in the list of codecs. The list of payload > types SHOULD only contain those codecs the endpoint is able to use on > the PSTN bearer. > > Again, a SHOULD similar to the one above. Can you please explain the > reasoning for it? > > [SV] See above. [SV] In this paragraph, I think it is sufficient just to replace the "SHOULD" with "MUST", i.e. "The list of payload types MUST only contain..." Regards, Simo > If the Offerer is only able to become the passive party in the > circuit-switched bearer setup, it MUST add the "callerid", "uuie" > and/or "dtmf" subfields, > > This text is a bit awkward as you specify this field as extensible. So > I think you need to add a choice for extensions here. > > [SV] We changed the text to read: > > If the Offerer is only able to become the passive party in > the circuit-switched bearer setup, it MUST add at least > one of the possible correlation mechanisms, but MUST NOT > specify values for those subfields. > > Similar text in section 5.6.2 > > but MUST NOT specify values for those > subfields. Looks good. > 5.6.2. Generating the Answer > > If the Answerer becomes the active party, it MUST add any of the > "callerid", "uuie" or "dtmf" subfield values. > > [SV] Changed the text in 5.6.2 to read > > If the Answerer becomes the active party, it MUST > add a value to any of the possible subfields. > > If the Answerer becomes the passive party, it MUST NOT add > any values to the subfields in the "cs-correlation" attribute. Looks good. > > 8.3. Registration of new "addrtype" values > > > Note to IANA: The current "addrtype" sub-registry structure does not > capture the fact that a given addrtype is defined in the context of a > particular "nettype". The sub-registry structure should be to > correct that as part of this registration. > > Did you mean "should be corrected as part ..." > > [SV] We discussed this Note to IANA with the ADs, and as a result the note > will be removed (and no context will be added for addrtype values in the IANA > registry). > > Nits/editorial comments: > > 5.3.3. Considerations on correlations > > Passive endpoints should expect an incoming CS call for setting up > the audio bearer. Passive endpoints MAY suppress the incoming CS > alert during a certain time periods. Additional restrictions can be > applied, such as the passive endpoint not alerting incoming calls > originated from the number that was observed uduring the > offer/answer > > Typo: during > > [SV]. Fixed. > > negotiation. > > > 5.7. Formal Syntax > > The following is the formal Augmented Backus-Naur Form (ABNF) > [RFC5234] syntax that supports the extensions defined in this > specification. The syntax is built above the SDP [RFC4566] and the > tel URI [RFC3966] grammars. Implementations according to this > specification MUST be compliant with this syntax. > > Figure 2 shows the formal syntax of the extensions defined in this > memo. > > ; extension to the connection field originally specified > ; in RFC 4566 > > connection-field = [%x63 "=" nettype SP addrtype SP > connection-address CRLF] > ;nettype and addrtype are defined in RFC 4566 > > connection-address /= global-number-digits / "-" > ; global-number-digits specified in RFC 3966 > > This is good, but you don't specify where CRLF, HEXDIG are defined. I > can guess, but it would be better if a reader doesn't have to guess. > > [SV] Added comment lines in ABNF that CRLF and DIGIT are defined in RFC5234 > (there was already a reference to RFC534 for HEXDIG). Thanks. > > ;subrules for correlation attribute > attribute /= cs-correlation-attr > ; attribute defined in RFC 4566 > cs-correlation-attr = "cs-correlation:" corr-mechanisms > corr-mechanisms = corr-mech *(SP corr-mech) > corr-mech = caller-id-mech / uuie-mech / > dtmf-mech / ext-mech > caller-id-mech = "callerid" [":" caller-id-value] > caller-id-value = "+" 1*15DIGIT > uuie-mech = "uuie" [":" uuie-value] > uuie-value = 1*65(HEXDIG HEXDIG) > ;This represents up to 130 HEXDIG > ; (65 octets) > ;HEXDIG defined in RFC5234 > ;HEXDIG defined as 0-9, A-F > dtmf-mech = "dtmf" [":" dtmf-value] > dtmf-value = 1*32(DIGIT / %x41-44 / %x23 / %x2A ) > ;0-9, A-D, '#' and '*' > ext-mech = ext-mech-name [":" ext-mech-value] > ext-mech-name = token > ext-mech-value = token > ; token is specified in RFC4566 > > Regards, > Simo > _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
