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!
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.
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
