Re: [MMUSIC] WGLC of draft-ietf-mmusic-media-loopback-18

[Hadriel Kaplan <[e-mail]>]

Hey Magnus,
inline...

On Apr 13, 2012, at 8:22 AM, Magnus Westerlund wrote:

> Hi,
> 
> I have reviewed the version in WGLC. I have some few comments:
> 
> 1. Section 5.5:
> "Note that for any form of NAT traversal to function, symmetric
>    RTP/RTCP MUST be used."
> 
> Any reason to no reference RFC 4961 here?

Will do.

> 
> 2. Section 7.1.1 to 7.1.3 and 7.2.1 to 7.2.3 all are missing a space
> between the section number and the section title.

Ya the formatting got screwed up by the generic text printing of the MS Word doc.

> 
> 3. Section 6:
> A loopback mirror that is compliant to this specification and
>    accepts media with rtp-pkt-loopback loopback-type MUST loopback the
>    incoming RTP packets using either the encapsulated RTP payload
>    format or the direct loopback RTP payload format as defined in
>    section 7 of this specification.
> 
> I don't think this has addressed my previous WG last call comment about
> that the MUST needs an escape clause or at least a pointer to the issue
> of congestion control. Because if you have no escape clause then the
> loopback source MUST perform congestion control not only on the path
> from the source to the mirror, but also on the mirror to the source.

OK.

> 
> 4. Section 7.1.2:
>    The outer RTP header of the encapsulating packet MUST be followed
>    by the payload header defined in this section.  If the received RTP
>    packet has to be looped back in multiple encapsulating packets due
>    to fragmentation, the encapsulating RTP header in each packet MUST
>    be followed by the payload header defined in this section.
> 
> I think this is are unfortunate formulations as it actually defines that
> RTP header extensions and CSRC lists are not allowed. Especially header
> extensions should not be disallowed. I would suggest a formulation saying:
> 
> The RTP payload of the encapsulating RTP packet starts with the payload
> header defined in this section.
> 
> And go on and define the payload structure in relation to the start of
> the payload.

Ah sorry I thought I cleaned up that wording to make it clear.  I'll fix that.


> 
> 5. Section 7.1.2:
>    The
>    Receive timestamp MUST be based on the same clock used by the
>    loopback-source.
> 
> I still think this formulation is extremely wrong. One alternative is
> the same clocks source, like if they where using the same NTP server. Or
> is it actually "clock rate" that is intended here?
> 
> Se my previous comment 28 for more input about this:
> http://www.ietf.org/mail-archive/web/mmusic/current/msg08971.html

Sorry I meant clock rate - will change.


> 
> 
> 6. I can't locate any answer to my issue number 30:
> 
> 30. Section 7.2.1:
> Sequence Number: The RTP sequence number SHOULD be generated by the
>    loopback-mirror in the usual manner with a constant random offset.
> 
> What is the acceptable cases for when to break it? I assume the
> intention with not saying MUST is that one can copy it from the
> incomming packet. Thus when source -> mirror packet losses occur causing
> wholes in the return path which isn't real loss on the mirror->source
> path. It also causes third party monitor to report losses that aren't
> real on this stream.
> 
> Can you please summarize any discussion that has happened?

Actually there has been no discussion on this topic as far as I know - it was not a SHOULD to allow copying, it's a SHOULD because it's a SHOULD in RFC 3550.
If you'd prefer, I can just say MUST follow RFC 3550.

> 
> 7. I also don't see any changes related to my comment 34:
> 
> "34. Section 9.
> This section is thoroughly inadequate. The reason is that you have a
> forward path that is coupled to the reverse path. If the loopback is to
> work correctly the mirror needs to be able to send back what the source
> sent to it. Thus it is the loopback source that needs to adapt the media
> bit-rate so that both forward and reverse path can handle the flows. If
> doesn't the mirror will need to reduce the bit-rate it transmits and
> that can only happen in two ways, dropping packets or transcoding them
> to a lower rate fulfilling the congestion control requirement. Neither
> will fulfill the purpose of the loopback as I see it. This issue needs
> discussion, including some considerations in how to actually accomplish
> it with the tools we have available."
> 
> This is coupled to issue 3 above. If that above MUST stands you
> absolutely have to have more detailed discussion about dealing with the
> whole loop, rather than a single leg that is the norm.

OK we should start a thread on this.


> 
> 8. Section 11.
> 
> The amplification or reflection this allows has at least been
> sufficiently discussed. I am far from certain that the mitigation is
> clear enough.
> 
> I do support Flemming's comment about the SRTP implications of using
> loopback are.

Yup.

> 
> 9. Another not addressed issue:
> 
> 41. The media type registrations "rate" parameter I think needs to be
> redefined. It needs to reference the rate of the payload types it
> intends to mirror back.

OK.