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

[Suhas Nandakumar <[e-mail]>]

Thanks Paul for reviewing the draft.  We will go through your comments and
get back to you. In the meanwhile, I suggest people to HTML rendering of
the draft for ease of readability.

Here is the link: https://tools.ietf.org/id/draft-ietf-rtcweb-sdp-01.html

Thanks
Suhas

On Tue, May 31, 2016 at 12:09 PM, Paul Kyzivat <[e-mail]>
wrote:

> 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
>>
>>
> _______________________________________________
> rtcweb mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/rtcweb
>