Thank you Alexey very much for the review, and Simo for the updates. I have balloted No-Objection.
Jari On Feb 28, 2013, at 12:48 PM, [email protected] wrote: > 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 _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
