Re: [rtcweb] partial review of draft-ietf-rtcweb-sdp-01

[Paul Kyzivat <[e-mail]>]

Another general issue:

I have been *trying* to verify the consistency in use of ports, IP 
addresses, fingerprints, ice ufrag info, ice passwords, ssrcs, cnames, etc.

Of course this is excruciating to do. (For a reviewer like me, and also 
for a potential implementer trying to understand the examples.)

My preliminary take is that there are a number of errors. A lot of them 
are probably cut/paste errors. With sufficient analysis it is sometimes 
possible to figure out what was intended vs. what is written. But it is 
hard with this much stuff.

It would be easier if, instead of using random sample values, the values 
were replaced with semantically relevant and easily recognized values. 
(E.g., "ALICE-SSRC-1", "TED-CNAME-2") Unfortunately I don't think this 
can be done while also having those values by syntactically valid.

I think I will end up making such a replacement myself, in a local copy, 
simply to assist in analyzing what is there.

	Thanks,
	Paul

On 5/30/16 6:14 PM, Paul Kyzivat wrote:
> Ted asked if I would do a review of draft-ietf-rtcweb-sdp-01. I started,
> but it is taking a long time. I'm sending what I have so far to give you
> something to consider/work on.
>
> In order to simplify this review for both me and readers, I will in
> general only mention an issue once, the first time I find it, and be
> silent on future occurrences of the same issue.
>
> * In general in the examples it would be very helpful if you would
> include a blank line prior to each m-line. When reading these it is
> currently hard to find the m-lines, which is something needed when
> trying understand and compare things.
>
> * Section 3:
>
> I realize this is for a webrtc oriented reader, so I am trying to
> understand it from that perspective. Even so, I have trouble with
> classifying a=ice* as "Security Descriptions". Given the existing
> categories I would think they better belong in "Stream Description".
>
> * Section 5.1:
>
>    o  The term "Session" is used rather loosely in this document to
>       refer to either a "Communication Session" or a "RTP Session" or a
>       "RTP Stream" depending on the context.
>
> This seems like a bad idea. It may be forgivable to mix communication
> session and RTP session in examples where a single bundle is used for
> all media, or there is only one m-line. But not if multiple m-lines
> aren't bundled, and not with RTP Stream.
>
> OTOH, so far I haven't noticed any place where this is an issue.
>
> * Section 5.2.1:
>
> The SDP line-wrapping convention described in 5.1 isn't being used in
> the example.
>
> Is there a reason for including the IP address in the a=rtcp line, when
> it is the same as the address in the c= line, which is the default?
>
> Is there any expectation that a=ptime:60 only applies to opus? IIUC this
> should be interpreted ad applying to all codecs mentioned in the m-line.
> (There is no order-dependence among attributes within a single media
> section. AFAIK there is no way of specifying a separate ptime for
> individual codecs in an m-line.)
>
> Use of sha-1 in a=fingerprint is about to become incorrect according to
> draft-ietf-mmusic-4572-update-03. That work is being driven by rtcweb,
> so I would think you would want to reflect it here.
>
> The way tables are formatted in RFC format can make the examples
> confusing to readers: the label for table 1 falls exactly between the
> bottom of the table it describes and the following table. When viewed on
> a screen it is difficult to tell what each table is. I suggest tweaking
> the contents of the tables to make this clearer. E.g.,
>
>    +----------------------------------+--------------------------------+
>    | SDP Contents - Offer             | RFC#/Notes                     |
>    +----------------------------------+--------------------------------+
>
> In Table 2, the o-line has two spaces after the "-". It should only be one.
>
> * Section 5.2.3:
>
> Table 5 has ice and fingerprint attributes prior to the first m-line.
>
> The m-line and associated attributes for the data channel is
> inconsistent with draft-ietf-mmusic-sctp-sdp-16. I think it ought to be
> (more or less):
>
> m=application 56966 UDP/DTLS/SCTP webrtc-datachannel
> c=IN IP4 24.23.204.141
> a=mid:data
> a=sctp-port:5000
> a=setup:actpass
> ...
>
> If you really want to negotiate the usage of channel 1, then it also
> needs to include:
>
> a=dcmap:1 subprotocol="chat";label="channel 1"
>
> (Note: this presumes that "chat" is a registered subprotocol. It isn't!)
>
> * Section 5.2.4:
>
> This seems fine if Bob intends to send audio (e.g. MoH). If not then it
> would be better to answer a=inactive.
>
> * Section 5.2.5:
>
> Has there been any discussion of pros/cons of having DTMF on a separate
> m-line and a separate ssrc? If one or both ends actually think of DTMF
> as being part of another audio stream then this might present problems.
> This could especially be true for interoperation with telephony devices.
>
> Is there anything in rtcweb that prevents having these on the same
> m-line and ssrc?
>
> * Section 5.2.7:
>
> Note that the BAS concept has been removed from Bundle, so you at least
> shouldn't refer to it. (You can still do the O/A if you want.)
>
> In Table 14 (answer), why are ice attributes and fingerprints given
> separately (and differently) for the two m-lines? (Even though the ports
> and addresses are the same.) This is contrary to current bundle (-29)
> procedures.
>
> In the 2nd (BAS) offer there is a gratuitous change in ordering of
> several of the attributes. (Makes it difficult to see what has changed.)
> I also don't understand the other differences between the ice and
> fingerprints in audio and video, and between this and the first offer.
>
> * Section 5.2.8:
>
> This example *assumes* bundle support, and so uses the same addr/port
> for bundled m-lines. But this will fail badly if the assumption turns
> out to be wrong. When making this assumption you ought to use
> bundle-only in the initial offer to ensure nothing bad happens if the
> assumption of bundle support by answerer turns out to be wrong. AFAIK
> there is no downside to doing this.
>
> Again this doesn't follow current bundle rules.
>
> =======
>
> That's as far as I have gotten. (Slow going.) I'll try to get you more
> later.
>
>     Thanks,
>     Paul
>
> _______________________________________________
> rtcweb mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/rtcweb
>