Subject: Re: [siprec] SDP Comments to draft-ietf-siprec-protocol-00

From: "Hutton, Andrew" <[email-masked]>
Date: Wed, 30 May 2012 11:21:10 +0000

Hi Miguel,

I checked whether your comments had been taken care of in the latest version of draft-ietf-siprec-protocol and found that mostly they have apart from a couple of minor omissions. See the text below for my analysis.

Regarding getting the SDP directorate to review the draft then given your previous review I think we can wait until the draft is at WGLC before we ask for a review by the SDP directorate.  This I hope will not be long probably following Vancouver.

Henry, Could you please take care of the outstanding issues below as you have the pen.

Regards
Andy

> -----Original Message-----
> From: Miguel A. Garcia [mailto:[email-masked]]
> Sent: 13 September 2011 09:55
> To: [email-masked]; [email-masked];
> [email-masked]; Hutton, Andrew; [email-masked]; mmusic
> Cc: Elwell, John; Brian Rosen
> Subject: SDP Comments to draft-ietf-siprec-protocol-00
> 
> Hi,
> 
> here are some comments to the SDP in draft-ietf-siprec-protocol-00
> 
> - Section 5.3. I found a few "must" and "should" in lowercase. I wonder
> whether these should be written normatively (uppercase) or is there a
> reason for not doing it, in which case, I would recommend to go around
> the lowercase terms to avoid confusion.

Done

> 
> - Section 5.3, third paragraph: the text is written around the format
> (a=sendonly), but I will instead put the focus on the semantics. For
> example, I would write:
> 
>     Since the SRC does not expect to receive media from the SRS, the
> SRC
> will typically set each media stream of the SDP offer to only send
> media,
> by qualifying them with the a=sendonly attribute. Conversely, as the
> SRS
> only receives RTP streams from SRCs, the SDP answer will normally set
> each media stream to only receive media, by setting them with the
> a=recvonly attribute, according to the procedures of RFC 3264
> [RFC3264].

Partly Done - The SDP answer part being set to a=recvonly is not in the current draft and could be added.

> 
> - Section 5.3, fourth paragraph. As a matter of personal taste, I
> prefer
> that actions are attributed to either the SRC or the SRS, but not to
> the
> "SDP" (which is not an entity). For example, the sentence "the SDP must
> provide a label..." should be restated in terms of the "the SRC MUST
> include a label *attribute* (a=label) in each media stream of the
> SDP"...

Done

> 
> Also, in this fourth paragraph in Section 5.3, avoid the future tense,
> unless you really want to describe an action that will take a later
> time:
> "... will be used ..."

Done

> 
> - Figure 4 in Section 5.3: the "o=" line is incorrect, due to sess-id
> and
> sess-version are set to 0. Also, the selected IP address is not being
> according to RFC 5737. Use this one instead:
> 
> 
>     o=SRS 2890844526 2890844526 IN IP4 198.51.100.1

Done

> 
> - Figure 4 in Section 5.3: the "s=" line is incorrect according to RFC
> 3264. Use an white space or dash character instead:
> 
>     s=-

Done
> 
> - Figure 4 in Section 5.3: the "c=" line is incorrect due to the IP
> address not being according to RFC 5737. Use this one instead (should
> be
> the same IP address as in the o= line).
> 
>     c=IN IP4  198.51.100.1

Done

> 
> - Figure 4 in Section 5.3: all the m= lines are incorrect. All the
> lines
> should indicate an even port number for RTP, implicitly indicating that
> the consecutive port number is used for RTCP. Additionally, two lines
> referring to the H264 codec indicate that the stream is of type
> "audio",
> which is incorrect. Use the following "m=" lines (in order).
> 
>    m=audio 12240 RTP/AVP 0 4 8
>    m=video 22456 RTP/AVP 98
>    m=audio 12242 RTP/AVP 0 4 8
>    m=video 22458 RTP/AVP 98

The 4th m-line still incorrectly indicates it is audio - Needs to be fixed.

> 
> - Figure 4 in Section 5.3. Use a correct a=fmtp line, for example:
> 
>        a=fmtp:98 profile-level-id=42A01E;
>                  sprop-parameter-sets=Z0IACpZTBYmI,aMljiA==

Done.

> 
> 
> - Section 5.3, 5th and 6th paragraphs: The text should be stated in
> terms
> of SDP offer/answer (we are in Section 5.3, which is "SDP handling"),
> and
> a reference to RFC 3264 should be added. Therefore:
> 
>     To remove a recorded media stream from the recording session, the
> SRC
>     sends a new SDP offer where the port of the media stream to be
>     removed is set to zero, according to the procedures in RFC 3264
>     [RFC3264].
> 
>     To add a new media stream to the recording session, the SRC sends a
>     new SDP offer adding a new media stream description in an m= line,
>     according to the procedures in RFC 3264 [RFC3264].

Done 

> 
> - Section 5.3, 7th paragraph should also be drafted in terms of SDP
> O/A.
> 
> - Figure 6 in Section 5.4, use this SDP instead:
> 
>       v=0
>       o=SRS 2890844526 2890844526 IN IP4 198.51.100.1
>       s=-
>       c=IN IP4 198.51.100.1
>       t=0 0
>       m=audio 12240 RTP/AVP 0 4 8
>       a=sendonly
>       a=label:1
> 

Done - The SDP example has been removed.

> - Section 6.2.1. The syntax is not correct. You should start by
> extending
> the "attribute" production in SDP, for example:
> 
>        attribute          /= record-attr
>        ; attribute defined in RFC 4566
>        record-attr        = "record:" indication
>        indication         = "on" / "off" / "paused"

Done - now section 10.2.1 - But also applies to the recordpref-attr in what is now 10.1.1 so this needs to be fixed.

> 
> - Section 6.2.2. For discussion... Do you really need two attributes,
> "record" and "recordpref"? I believe both could be integrated into a
> single attribute. For example, you could have the a=record attribute
> with
> the two values, one for the current status, and one of the current
> preference.
> 

Done - Clarified on mailing list that we do need both.

> - Section 7.1. Although this is not related to SDP, in this paragraph
> you
> need to specify the registry where you want to register the new option
> tags. I believe it is the Session Initiation Protocol Parameters
> registry.
> 
> - Section 7.5.1 and  7.5.2. The registration lacks contact name, email
> address, and telephone number, according to the requirements in RFC
> 4566.
> 

Contact names and e-mail address is now included. RFC 4566 requires a telephone number but personally I think this is not desirable due to privacy issues.

> /Miguel
> --
> Miguel A. Garcia
> +34-91-339-3608
> Ericsson Spain

---

From: "Miguel A. Garcia" <[email-masked]>
Date: Wed, 30 May 2012 14:44:30 +0200

Ok, so we will wait for the document to enter WGLC and at that time we 
will organize a review.

Please inform us when we need to kick the SDP directorate review.

BR,

      Miguel

-- 
Miguel A. Garcia
+34-91-339-3608
Ericsson Spain

---

From: "Henry Lum" <[email-masked]>
Date: Wed, 30 May 2012 09:44:39 -0700

Thanks for the detailed review. I will make the fixes along with the
issues brought up in ietf 83.

Henry

> From: Hutton, Andrew [mailto:[email-masked]]
> Sent: May-30-12 7:21 AM
> To: Miguel A. Garcia
> Cc: Brian Rosen; [email-masked]; Henry Lum;
[email-masked];
> [email-masked]; [email-masked]
> Subject: RE: SDP Comments to draft-ietf-siprec-protocol-00
> 
> Hi Miguel,
> 
> I checked whether your comments had been taken care of in the latest
version
> of draft-ietf-siprec-protocol and found that mostly they have apart
from a
> couple of minor omissions. See the text below for my analysis.
> 
> Regarding getting the SDP directorate to review the draft then given
your
> previous review I think we can wait until the draft is at WGLC before
we ask
> for a review by the SDP directorate.  This I hope will not be long
probably
> following Vancouver.
> 
> Henry, Could you please take care of the outstanding issues below as
you have
> the pen.
> Partly Done - The SDP answer part being set to a=recvonly is not in
the
> current draft and could be added.
> 
> 
> >
> Done - now section 10.2.1 - But also applies to the recordpref-attr in
what is
> now 10.1.1 so this needs to be fixed.
> 
> >
> Contact names and e-mail address is now included. RFC 4566 requires a
> telephone number but personally I think this is not desirable due to
privacy
> issues.