Re: [rtcweb] Working Group Last Call for draft-ietf-rtcweb-rtp-usage-13

[Colin Perkins <[e-mail]>]

Hi,

Some comments inline.

On 12 May 2014, at 12:40, Magnus Westerlund <[e-mail]> wrote:
> Thanks for the feedback, I will work with my co-authors to propose text
> changes where so is appropriate and see inline for comments or response.
> 
> On 2014-05-09 16:41, Cullen Jennings wrote:
>> 
>> Overall the document looks in excellent shape. I do have a bunch of
>> comments but they are all pretty easy to deal with.
>> 
>> Page 5 I think that draft-ietf-avtcore-rtp-multi-stream-optimisation
>> should be MAY not a MUST. We don’t loose significant interoperability
>> by not having it a MUST.
> 
> It is not a MUST:
> 
> "Support for the RTCP
>      optimisations for multi-SSRC sessions defined in
>      [I-D.ietf-avtcore-rtp-multi-stream-optimisation] is RECOMMENDED."
> 
> And it is recommended for a good reason. Although this is a new spec it
> provides a significant improvement in RTCP efficiency compared to
> without it.
> 
> Below you find the statistical simulation results for the intervals
> between regular RTCP packets achieved over 10000 RTCP packets in an
> WebRTC like situation with X number of SSRCs per endpoint between two
> endpoints, all SSRC are active senders. The RTCP bandwidths are
> configured as RR: 15000 bps, RS: 10000 bps and T-rr-int = 0
> 
> AVPF is base line AVPF
> AVPF-AGG-ADJ is the scheduling aggregation that is being specified in
> draft-ietf-avtcore-rtp-multi-stream
> AVPF-RG-AGG is with the report group extension and the removal of cross
> reporting specified in draft-ietf-avtcore-rtp-multi-stream-optimisation
> 
> Average transmission interval from using the different solutions:
> 
> SSRCs         1        2        3        4        5
> 
> AVPF          1.043879 1.126795 1.246220 1.410738 1.599270
> AVPF-AGG-ADJ  1.041918 1.110367 1.217367 1.362500 1.549829
> AVPF-RG-AGG   1.051294 1.080191 1.113448 1.150983 1.187954
> 
> Gain from using the different solutions:
> 
> SSRCs         1         2         3         4         5
> 
> AVPF-AGG-ADJ  -0.001879 -0.014579 -0.023152 -0.034193 -0.030915
> AVPF-RG-AGG    0.007103 -0.041360 -0.106540 -0.184127 -0.257190
> 
> As can be seen the significant gain is for the report group extension,
> especially as the SSRCs per end point increases.
> 
>> 
>> Page Sending correct sync information should be MUST not SHOULD. We
>> can’t make lip sync work without this and I think the flows I have
>> seen already send it. Note people don’t have to deal with correctly
>> implementing lip sync on the receiving end, they just have to send
>> enough information to make it possible that receivers that want to
>> can.
> 
> You have misread this:
> 
>   o  Sending correct synchronisation information in the RTCP Sender
>      Reports, to allow receivers to implement lip-synchronisation;
>      support for the rapid RTP synchronisation extensions (see
>      Section 5.2.1) is RECOMMENDED.
> 
> The first part "Sending correct synchronisation information in the RTCP
> Sender Reports, to allow receivers to implement lip-synchronisation;"
> is MUST, the second part is providing a recommendation.
> 
> we had an comment from Olle about this and propose to reformulated this
> to be:
> 
>   o  Sending correct synchronisation information in the RTCP Sender
>      Reports, to allow receivers to implement lip-synchronisation; see
>      Section 5.2.1 regarding support for the rapid RTP synchronisation
>      extensions.
> 
> 
>> 
>> Has the text "support for RTCP timer reconsideration." but I don’t
>> know what this means or how to implement it - suspect we just need a
>> ref or bit more text.
> 
> Okay, we can add a reference to Section 6.3 of RFC 3550 where this is
> discussed and defined.

Added.

>> I’d refer to see "reduced minimum RTCP reporting interval" be a MUST
>> and I could live with MAY but SHOULD is just lame for this.
> 
> You will not get a protest from me to make this a MUST. Does anyone else
> protests?

Added, along with some clarifications on how this affects RTCP source timeout intervals. 

>> Page 8
>> 
>> It has the text
>> 
>> An end-point that has signalled support for multiple RTP payload
>> formats SHOULD be able to accept data in any of those payload formats
>> at any time, unless it has previously signalled limitations on its
>> decoding capability.
>> 
>> I think that has to be a MUST not a SHOULD or else this will not
>> work.
> 
> Yes, you are correct this really needs to be a MUST to get the desired
> effect. Otherwise you will have media failures when the sender does this
> and the receiver isn’t capable.

Fixed.

>> Page 18
>> 
>> This has RTX (RFC4588) and MUST. RTX has turned out to be close to
>> useless in interactive communications systems because the RTT is just
>> too high to make this viable. You need to set your jitter buffer to
>> be larger than the RTT for this work and I am aware of any
>> significant internet collaborations system that sets the jitter
>> buffer size that high. I think this should be MAY not MUST but that
>> said, I can easily live with MUST, it just seems useless at a time
>> where we are trying to reduce the complexity of the system.
> 
> I want to really protest about this Retransmission being useless. The
> experiences we have had when deploying RTP retransmission in a video
> conference system has been good. The only case where we had real issues
> have been on intercontinental distances, for example between Stockholm
> and San Jose where the RTT is large enough to actually cause issues that
> the user notices. But on short distances like Stockholm to Germany or
> Finland it works in excellent. This is a system I use almost daily and
> have been in internal usage for several years.

Agree. The RTTs I see for calls around the UK are often in the range where retransmission is entirely feasible. 

> But, yes there are implementation choices regarding the buffering
> strategies here that do matters. One actually don't have to let the
> jitter buffer always have one or more RTT extra depth, the alternative
> is to delay the playout on the occurrences when retransmission is required.
> 
> As noted by Colin, the MUST is more subtle than requiring RTP
> retransmission. I think what is in the draft represents the conclusions
> of the discussion in Stockholm interim as well as on the mailing list
> after.
> 
>> 
>> 
>> Page 19
>> 
>> It says FEC is not required. I’m OK with this but my notes have it as
>> the WG previously agreed on this being required. It might be worth
>> putting this up on a slide in the May meeting and checking the WG is
>> good with no FEC. Perhaps my notes are all wrong - I did not try and
>> go back throughout minutes. It’s possible the WG decided different
>> things and different times.
> 
> My recollection has also been that the WG has been positive towards FEC,
> but none has made a suggestion for a specific FEC scheme and RTP
> encapsulation of it to include at any implementation level. Thus, the
> formulation that is present in the draft.
> 
>> 
>> 
>> Page 20
>> 
>> Has the text
>> 
>> signalled bandwidth limits SHOULD be used to limit traffic
>> 
>> I might be reading this the wrong way but given the weak congestion
>> control in general, I feel pretty strongly that the browser can’t
>> ignore signaled bandwidth limits and this needs to say something more
>> like the "MUST not exceed signaled bandwidth"
> 
> Lets quote the sentence before the one you partially references:
> 
>   In addition, the signalling
>   channel can establish maximum media bit-rate boundaries using the SDP
>   "b=AS:" or "b=CT:" lines, and the RTP/AVPF Temporary Maximum Media
>   Stream Bit Rate (TMMBR) Requests (see Section 5.1.6 of this memo).
> 
> Looking at the full sentence you refer to:
> 
>   The combination of media codec choice and signalled bandwidth limits
>   SHOULD be used to limit traffic based on known bandwidth limitations,
>   for example the capacity of the edge links, to the extent possible.
> 
> My personal interpretation of this text has been that:
> 
> 1. You follow the limitations the peer signals
> 2. You SHOULD set these values your self based on known limitations and
> code configurations.
> 
> But, looking at the text, we probably needs to make these two
> distinctions clearer. Adding a MUST statement for the first thing I
> support.
> 
> 
>> Page 20-22,
>> 
>> Section 7.2 - I felt like this section was supposed to convince me it
>> was all OK but the more I read it the more confused I was. What does
>> one do if sending video on says a 768 kbps link? Send every 5 frames?
>> 
>> 
>> I think part of the confusion stemmed from the implication that to do
>> good congestion control, you needed a report once ever say RTT but in
>> RTP we could not do that so we would fall back to a far less frequent
>> reporting of once ever frame. But ever frame is less than the RTT in
>> most internet case so I was just confused by the whole section.
>> 
>> I don’t think this section is going to help implementors much as it
>> is - perhaps it could just be refactored a bit to help implementers
>> understand how often to send RTCP and why.
> 
> I will have to come back regarding this, I believe I understand the
> issues you see, but I need to consider further what to do about them.

The text in the draft is perhaps better suited to giving guidelines for designers of congestion control algorithms than for browser implementers.

>> Page 24
>> 
>> Has
>> 
>> All WebRTC implementations MUST be prepared to receive RTP XR report
>> packets, whether or not they were signaled.
>> 
>> I disagree with this - we are in a non multicast environment with RTP
>> inside DTLS/SRTP. One should not be getting reports for extensions
>> that were not negotiated.
> 
> I think it is important that RTP stack implementations can ignore
> unknown or none supported features. At the same time I agree with you
> that you should normally not get sent features that you don't support
> and can not use. That is just wasting RTCP bandwidth and processing.
> 
> I can see that detailed control of this may be limited in certain
> multi-party cases or gateways with legacy where controlling the legacy
> might be an issue.
> 
>> From my point of view this should be:
> 
> 1. MUST NOT send unless agreed on.
> 2. MUST be able to receive and discard options not asked for.
> 
> Are you fine with such a clarification?
> 
>> 
>> Similarly on page 25 the text has
>> 
>> The RTP extensions to be used SHOULD be agreed upon,
>> 
>> I think this needs to be a MUST.

For both this, and the previous, I think SHOULD is correct, along with some recommendations about being robust when receiving unexpected packets. We do not want WebRTC implementations to fall over when sent something novel that hasn’t been signalled. 

>> Page 27
>> 
>> I think it would be best to just remove the line
>> 
>> This document [I-D.ietf-mmusic-msid] also defines, in section 4.1,
>> how to map unknown source packet stream SSRCs to MediaStreamTracks
>> and MediaStreams.
>> 
>> as that is not key to this spec.
> 
> The reason that is included is that it can be an important aspect of
> handling legacy interop where you don't require the media plane
> information to be included in the signalling message the signalling
> gateway creates.
> 
>> Page 36 - 37
>> 
>> Section 12.2.2.
>> 
>> So if I read this right, it says you MUST resolve SSRC collisions but
>> RFC 5576 has no way to do that. What are implementors supposed to do
>> ?
> 
> Sorry, what are you referring to? Section 5 of RFC 5576 is very clear on
> what actions needs to happen after an SSRC collision has occurred.

I think this was a typo in our draft. I changed “The Source-Specific SDP Attributes [RFC5576] contains no mechanism to resolve SSRC collisions” to “contains a mechanism to resolve”, since as you point out, it does.

> Basically, choose new SSRC, signal it and include previous-SSRC
> attribute to inform about your previous SSRC that you where forced to
> abandon due to collision.
> 
>> 
>> Even ignoring 5576 - it’s just not clear to me how one detects and
>> resolves SSRC collisions.
> 
> Please see RFC 3550 Section 8.2

I added a reference to this.

-- 
Colin Perkins
http://csperkins.org/