Re: [rtcweb] Working Group Last Call: JSEP

[Magnus Westerlund <[e-mail]>]

Thanks Ted,

I think these two comments actually can be added as comments to Colin's 
earlier issue.
https://github.com/rtcweb-wg/jsep/issues/364

I will do that.

Cheers

Magnus


Den 2016-11-14 kl. 10:16, skrev Ted Hardie:
> Hi Magnus,
>
> I've added github issues for most of these in the range 387 to 408.  The
> two final issues here (numbers 31 and 32), both related to section 6,
> weren't really clear enough for me to provide a useful summary.  Can you
> try to rephrase those?  It would be great if you can do that by adding
> them to the issues list, but I'll try to add them if you update them here.
>
> thanks,
>
> Ted
>
> On Sun, Nov 13, 2016 at 4:13 PM, Magnus Westerlund
> <[e-mail] <mailto:[e-mail]>>
> wrote:
>
>     Hi,
>
>     As I am late, and we are having the discussion tomorrow I am taking
>     the easiest route for me to get my feedback out to the WG and
>     authors, thus an email rather than Github issues.
>
>     So here are my comments on the -17 version of JSEP.
>
>     1. Terminology: m= section. I would propose that the document
>     creates a terminology section and are explicit that m= section and
>     SDP Media Description are the same thing.
>
>     2. There are a couple of terms that would really need a definition
>     and also possibly some reflection of how they relate to the RTCWeb
>     terms, like WebRTC Endpoint. So the terms I really like defined are:
>
>     JSEP endpoint
>     JSEP implementation
>
>     3. Section 3.6:
>
>     JSEP
>        implementations will not transmit non-square pixels, but SHOULD
>        receive and render such video with the correct aspect ratio.
>
>     I have two issues with this. First of all, shouldn't JSEP
>     implementations be WebRTC endpoints? The second is these statement
>     that says that a JSEP/WebRTC foo will not do a thing. In most cases
>     that is not necessary, as there would be no issues if a particular
>     implementation supports this functionality. So, I think this should
>     be written as:
>
>     WebRTC endpoints does not need to support transmission of non-square
>     pixels, but SHOULD
>        receive and render such video with the correct aspect ratio.
>
>     4. Section 3.6.2:
>
>        When communicating with a non-JSEP endpoint, multiple relevant
>        "a=imageattr recv" attributes may be received.  If this occurs,
>        attributes other than the one with the highest "q=" value MUST be
>        ignored.
>
>     There is to my knowledge nothing preventing multiple parameters sets to
>     have the same preference value (q). Thus, the formulation of this
>     sentence needs to be addressed.
>
>     5. Section 3.6.2:
>
>        If an "a=imageattr recv" attribute references a different video codec
>        than what has been selected for the MediaStreamTrack, it MUST be
>        ignored.
>
>     So, to my understanding there can be multiple video codecs and
>     multiple RTP PTs that have been negotiated as possible to use. Is
>     selected just the single currently used one, one or all the possible
>     ones here?
>
>     6. Section 3.6.2:
>
>        If the attribute includes a "sar=" (sample aspect ratio) value set to
>        something other than "1.0", indicating the receiver wants to receive
>        non-square pixels, this cannot be satisfied and the sender MUST NOT
>        transmit the track.
>
>     Another instances where the language should allow for flexibility if
>     an implementation goes beyond the defined mandatory support.
>     Changing "this cannot be" to "if this cannot be" may resolve it.
>     Also, isn't the relevant "MUST NOT" is to not accept this in the
>     negotiation?
>
>     7.  Section 3.7:
>
>        JSEP supports simulcast of a MediaStreamTrack, where multiple
>        encodings of the source media can be transmitted within the context
>        of a single m= section.
>
>     I think this should be explcit that it is "simulcast transmission of
>     a MediaStreamTrack".
>
>     8. Section 5.1.1:
>
>        This list of mandatory-to-implement specifications is derived from
>        the requirements outlined in [I-D.ietf-rtcweb-rtp-usage].
>
>        R-5   [RFC4568] MUST NOT be implemented to signal SDES SRTP keying
>              information.
>
>     So, the list of mandatory-to-implement includes also some to not
>     implement. That is fine, but probably should be moved to its own
>     list, or rename the list so that it can contain both types.
>
>     Secondly, this specific requirement I think comes from the security
>     docs, rather than rtp-usage.
>
>     9. Section 5.1.1:
>        R-2   [RFC5764] MUST be supported for signaling the UDP/TLS/RTP/SAVPF
>              [RFC5764], TCP/DTLS/RTP/SAVPF
>              [I-D.nandakumar-mmusic-proto-iana-registration], "UDP/DTLS/
>              SCTP" [I-D.ietf-mmusic-sctp-sdp], and "TCP/DTLS/SCTP"
>              [I-D.ietf-mmusic-sctp-sdp] RTP profiles.
>
>     I would note that [I-D.nandakumar-mmusic-proto-iana-registration] is
>     actually published as RFC 7850.
>
>     10. Section 5.1.1:
>        R-12  [RFC5761] MUST be implemented to signal multiplexing of RTP and
>              RTCP.
>
>     This should include the updated as ref also.
>
>
>     11. Section 5.1.1:
>
>       R-15  [RFC3556] with bandwidth modifiers MAY be supported for
>              specifying RTCP bandwidth as a fraction of the media bandwidth,
>              RTCP fraction allocated to the senders and setting maximum
>              media bit-rate boundaries.
>
>     So, why wasn't support of RR and RS bandwidth modifiers made a MUST?
>
>     Secondly, the RR and RS attributes do not specify the bandwidth as
>     fraction of the media bandwidth. The specify RTP session level
>     limits for RTCP bandwidth.
>
>     12. Section 5.1.1:
>
>        R-16  TODO: any others?
>
>     Please remove this.
>
>     13. Section 5.1.3:
>
>        For media m= sections, JSEP endpoints MUST support both the "UDP/TLS/
>        RTP/SAVPF" and "TCP/DTLS/RTP/SAVPF" profiles and MUST indicate one of
>        these two profiles for each media m= line they produce in an offer.
>
>     Do I understand this correct, we are requiring support for the
>     "TCP/DTLS/RTP/SAVPF" proto so that in cases an endpoint supports the
>     optional to support ICE TCP, they can indicate it, and any WebRTC
>     endpoint will accept it, even if that is just one option? I do know
>     that different profiles are a negotiation issue. But, wouldn't it be
>     more reasonable in this case to use UDP/TLS/RTP/SAVPF in all cases
>     where one offer any candidates that also use UDP? And only use
>     TCP/DTLS/RTP/SAVPF in cases only TCP candidates are signalled, thus
>     not forcing TCP/DTLS/RTP/SAVPF onto clients that doesn't support it?
>
>     14. Section 5.1.3:
>     Otherwise, assume that AVPF is being used in an AVP
>           compatible mode and use AVP timing, i.e., "trr-int=4".
>
>     I find this sentence confusing. I think this should be clarified to say:
>
>     Otherwise, assume that AVPF is being used in an AVP
>           compatible mode, i.e. use AVPF timing configured with a
>     "trr-int=4".
>
>     15. Section 5.1.3:
>
>     Spurious space in:
>
>     For data m= sections, JSEP endpoints MUST support receiving the
>           "UDP/ DTLS/SCTP", "TCP/DTLS/SCTP", or "DTLS/SCTP" (for backwards
>           compatibility) profiles.
>
>     16. Section 5.2.1:
>        o  Session Information ("i="), URI ("u="), Email Address ("e="),
>           Phone Number ("p="), Bandwidth ("b="), Repeat Times ("r="), and
>           Time Zones ("z=") lines are not useful in this context and SHOULD
>           NOT be included.
>
>     Considering that b=CT is not useless, I am a bit surprised to see b=
>     on this list. Yes, b=CT: is only useful is one like to provide a
>     upper total bandwidth limitation. So, maybe some discussion of the
>     possible choice?
>
>     17. Section 5.2.1:
>
>     This is done in the order
>        that their associated RtpTransceivers were added to the
>        PeerConnection and excludes RtpTransceivers that are stopped and not
>        associated with an m= section (either due to an m= section being
>        recycled or an RtpTransceiver having been stopped before being
>        associated with an m= section) .
>
>     I am bit surprised to see "recycled" on a list of things that may
>     occur in generating an initial offer? Can that really occur?
>
>     18. Section 5.2.1:
>
>        o  An "a=mid" line, as specified in [RFC5888], Section 4.  When
>           generating mid values, it is RECOMMENDED that the values be 3
>           bytes or less, to allow them to efficiently fit into the RTP
>           header extension defined in
>           [I-D.ietf-mmusic-sdp-bundle-negotiation], Section 11.
>
>
>     Just to note that to my understanding we are having discussion if
>     this should be referencing a more specific generation algorithm.
>
>     Such a change is likely to affect text on RID also:
>
>     19. Section 5.2.1:
>     The list of
>           RTCP feedback mechanisms that SHOULD/MUST be supported is
>           specified in [I-D.ietf-rtcweb-rtp-usage], Section 5.1.
>
>     I would request that we don't use the quite confusing "SHOULD/MUST"
>     and instead write something like:
>
>     mechanisms that are recommended or mandated to be supported
>
>     20. Section 5.3.1:
>
>        o  If this m= section is for media with configurable frame sizes,
>           e.g. audio, an "a=maxptime" line, indicating the smallest of the
>           maximum supported frame sizes out of all codecs included above, as
>           specified in [RFC4566], Section 6.
>
>     In most cases the maxptime attribute doesn't affect the frame size
>     of the encoder, instead it affects the maximum amount of media that
>     are packetized into a RTP payload, i.e. how many frames that are
>     included.
>
>     21. Section 5.3.1:
>
>        Each m= section which is not bundled into another m= section, MUST
>        contain the following attributes (which are of category IDENTICAL or
>        TRANSPORT):
>
>     I react to "bundle into another m= section". I though they where
>     "bundled with"?
>
>     22. Section 5.7.2:
>        o  If present, a single "a=rtcp" attribute MUST be parsed as
>           specified in [RFC3605], Section 2.1, but its value is ignored, as
>           this information is superfluous when using ICE.
>
>     This is another case, where there is an exception to what is written
>     if one has supports the non RTP/RTCP mux usage. Thus, this should be
>     ammended to included such an exception.
>
>     23. Section 5.7.2:
>
>     What about additional a= attributes? Any attribute supported by the
>     implementation MAY be parsed?
>
>     24. Section 5.8:
>
>              +  Find the RtpTransceiver that corresponds to the m= section
>                 with the same MID in the created offer.
>
>              +  Set the value of the RtpTransceiver's mid attribute to the
>                 MID of the m= section.
>
>     I find this text strange. If you already know the MID, why is it
>     being set in the second sub-bullet?
>
>     25. Section 5.8:
>
>          *  If RTCP mux is indicated, prepare to demux RTP and RTCP from
>              the RTP ICE component, as specified in [RFC5761],
>              Section 5.1.1.  If RTCP mux is not indicated, but was indicated
>              in a previous description, this MUST result in an error.
>
>     I note that there can be difference here between an offer and an
>     answer if the RTCP mux indication may have changed or not between
>     these two description.
>
>     26. Section 5.9:
>       o  For any specified "CT" bandwidth value, set this as the limit for
>           the maximum total bitrate for all m= sections, as specified in
>           Section 5.8 of [RFC4566].  The implementation can decide how to
>           allocate the available bandwidth between m= sections to
>           simultaneously meet any limits on individual m= sections, as well
>           as this overall session limit.
>
>     I think this text can be improved to make it clearer that CT doesn't
>     imply a fixed over time divide of the bandwidth between RTP streams.
>     As long as individual media sources streams stay within other
>     requirements such as b=AS/TIAS they can be changed dynamically.
>
>     27. Section 5.10:
>
>           *  If the media section has RTCP mux enabled, discard any RTCP
>              component, and begin or continue muxing RTCP over the RTP
>              component, as specified in [RFC5761], Section 5.1.3.
>              Otherwise, prepare to transmit RTCP over the RTCP component; if
>              no RTCP component exists, because RTCP mux was previously
>              enabled, this MUST result in an error.
>
>     I think this should say "any RTCP ICE component"..
>
>     28. Section 5.10:
>     When sending RTCP feedback, use the
>              SSRC of an outgoing Source RTP Stream as the RTCP sender SSRC;
>              if no outgoing Source RTP Stream exists, choose a random one.
>
>     This is okay, but could be slightly more open in regards to sending
>     feedback packets efficiently. Section 5.4.1 in
>     https://datatracker.ietf.org/doc/draft-ietf-avtcore-rtp-multi-stream/?include_text=1
>     <https://datatracker.ietf.org/doc/draft-ietf-avtcore-rtp-multi-stream/?include_text=1>
>     is providing more detailed rules for things to consider.
>
>     29. Section 6:
>
>     If
>           any payload type could map to more than one RtpReceiver, map to
>           the RtpReceiver whose m= section appears earliest in the local
>           description.
>
>     I think this is a bad advice. I think it should say that if this is
>     the case, PT should not be used to try to determine which m= section
>     it belongs to. I think wrongly attributing a packet is worse than
>     dropping it in cases where the information is incomplete. This rule
>     can result in that one associate an SSRC with the wrong m= section.
>
>     30. Section 6:
>           If the packet has a MID and that MID is in the table mapping MID
>           to RtpReceiver, update the incoming SSRC mapping table to include
>           an entry that maps the packet's SSRC to the RtpReceiver for that
>           MID.
>
>     This is all fine, but requires an addition. If there was already a
>     MID associated with the SSRC, i.e. a new MID has been assigned to
>     this SSRC, then there is deleting in the old MIDs association table
>     that needs to happen.
>
>     31. Section 6:
>
>     I think this section should be updated to work with the BUNDLE text
>     as that matures. I understand that there are a few additional cases
>     one likes to cover for non WebRTC endpoints, but still, lets not
>     introduce unnecessary failure cases over that.
>
>     32. Section 6:
>
>     The RTCP packet routing. This doesn't match my view of how RTCP
>     processing happens. I think the "deliver a copy of the packet to
>           the RtpReceiver associated with that SSRC."
>
>     Is the part that I have most issues with. What I find more relevant
>     and correct and likely matching more implementations are"
>
>     deliver the information to
>           the RtpReceiver associated with that SSRC.
>
>
>     Cheers
>
>
>     Magnus Westerlund
>
>     ----------------------------------------------------------------------
>     Services, Media and Network features, Ericsson Research EAB/TXM
>     ----------------------------------------------------------------------
>     Ericsson AB                 | Phone  [phone]
>     <tel:%2B46%2010%207148287>
>     Färögatan 6                 | Mobile [phone]
>     <tel:%2B46%2073%200949079>
>     SE-164 80 Stockholm, Sweden | mailto: [e-mail]
>     <mailto:[e-mail]>
>     ----------------------------------------------------------------------
>
>


-- 

Magnus Westerlund

----------------------------------------------------------------------
Services, Media and Network features, Ericsson Research EAB/TXM
----------------------------------------------------------------------
Ericsson AB                 | Phone  [phone]
Färögatan 6                 | Mobile [phone]
SE-164 80 Stockholm, Sweden | mailto: [e-mail]
----------------------------------------------------------------------