Re: [payload] AD Evaluation of draft-ietf-payload-flexible-fec-scheme-13

[Giridhar Mandyam <[e-mail]>]

Thank you for the detailed response.

>> FLEX FEC certainly was inspired by FEC FRAME, but the document can stand on its own.    Since several definitions are leveraged from RFC 6363, I believe a normative reference is still appropriate.  My suggestion is to add one sentence to the last paragraph of Section 1.
>> Proposed change:  Add "This document also leverages several definitions along with the basic source/repair header description from RFC 6363 in their application to the parity codes defined here.”

> That helps. I might suggest something along the lines of “While this document fully defines the use of FEC to protect RTP streams, it also leverages...”

Modified proposed change:  “While this document fully defines the use of FEC to protect RTP streams, it also leverages several definitions along with the basic source/repair header description from RFC 6363 in their application to the parity codes defined here.”

>>>4.2.1: - There’s a number of 2119/8174 keywords in this section that seem more natural consequences of using RTP than new normative requirements. If that is the case, they shouldn’t use normative keywords unless in the form of direct quotes.
>> Apologies - I had trouble identifying such instances.  All instances of such keywords are related to requirements with respect to RFC 3550.  Do you have specific examples?

>- definition of sequence number. Doesn’t RTP already require the sequence number to increment by one, and recommend the use of a random starting value?

Proposed change:  “Sequence Number (SN): The sequence number follows the standard definition provided in [RFC3550]. Therefore it must be one higher than the sequence number in the previously transmitted repair packet, and the initial value of the sequence number should be random (i.e. unpredictable).”

>- SSRC: 3550 already says SSRCs SHOULD be assigned randomly, so the requirement is a statement of fact as far as _this_ spec is concerned. (The text even mentions “as suggested by RFC 3350", although 3550 does more than “suggest” it.)

Section 8 of RFC 3550 does not use normative language such as MUST or SHALL, but the existing text can stand to be clarified.

Proposed change:  “Synchronization Source (SSRC): The SSRC value for each repair stream SHALL be randomly assigned as per the guidelines of Section 8 of [RFC3550]. …”

>- "the collisions MUST be resolved as described in [RFC3550]” : again, that follows naturally from using RTP. It does not define a new requirement.

Proposed change:  “… the collisions must be resolved as described in …”

>>>- 10th paragraph: "The repair streams’ SSRC’s CNAME SHOULD be identical to the CNAME of the source RTP stream(s) that this repair stream protects.” Why is the SHOULD not a MUST? What happens if it is violated?

>>Proposed change:  Agreed that the requirement makes more sense as a MUST.  Will change accordingly.

>I concur with the change, but it is a material change. Please point it out to the WG in case anyone objects. (It may not be safe to assume people are reading this note in that level of detail, especially over the holidays :-)) To be clear, I don’t want to block progress of the draft over it; if anyone objects they can do so as part of the IETF last call.

I will point this out in a follow-on email to the WG list after I submit the next revision.

>>>§4.2.2: Please add a short explanation of why we need two different FEC packet formats. (Not counting the retransmission packet; the reasoning there is pretty obvious.)

>>Proposed change:  Add additional sentence to first paragraph of Sec. 4.2.2:  "Two of these variants are meant to describe different methods for deriving the source data from a source packet for a repair packet.  The third variant is for a straight retransmission of the source packet.”

>That’s useful, but it doesn’t explain _why_ we need two different methods.

Modified proposed change:  Add additional sentences to first paragraph of Sec. 4.2.2:  "Two of these variants are meant to describe different methods for deriving the source data from a source packet for a repair packet.  This allows for customizing the FEC method to allow for robustness against different levels of burst errors and random packet losses.  The third variant is for a straight retransmission of the source packet.”

>>>§5.1.1 (and subsequent media type definitions) - Please consider using the IESG ([e-mail]<mailto:[e-mail]>) as the contact for further information. Working groups come and go, and authors change jobs. But someone on the IESG should (hopefully) always be able to find the right person,

>>TENTATIVE proposed change:  I am happy to swap out Varun's email for IESG, but this does not seem to be a widespread practice even for recently-published RFC's.  Can you confirm that this is now becoming a best practice for contact info regarding registries?

>It’s been common of late. The reasoning is that the IESG email address is the least likely to change of any contact we might give.

Proposed change: Swap out personal email contact(s) for IESG address.

>>>- Change control:  In 5.1.1, shouldn’t that be the PAYLOAD working group (or it’s successor as delegated by the IESG...)

>>Proposed change:  Change to "IETF Audio/Video Payloads Transport Payloads Working Group”

>I suggest adding “... or it’s successor as delegated by the IESG.

Modified proposed change:  Change to “IETF Audio/Video Payloads Transport Payloads Working Group or its successor as designated by the IESG”

>>>- Are these registrations really intended to be provisional?

>>At this point, no.
>>Proposed change:  Eliminate mention of provisional registration for all media types.

>I agree. However this is another material change that should be pointed out to the WG.

I will point this out in a follow-on email to the WG list after I submit the next revision.

>>>- “In a network-friendly implementation”: Do you expect to see network-unfriendly implementations? Is that okay?

>>I don't view "network-friendly" as being a static condition.  There are scenarios where an FEC transmitter/receiver pair have negotiated a connection which is network-friendly for one application, but may not be for a different application.  There was a discussion about when FEC is appropriate and when it is not on the WebRTC mailing lists - see https://lists.w3.org/Archives/Public/public-webrtc/2018Nov/0156.html.   "Network-unfriendly" implementations could conceivably occur when an FEC session has been negotiated between endpoints, but neither endpoint has assessed transmission conditions.

>Is that a reasonably implementation? It sounds like a bug to me. (I probably could have phrased my question better as “ are network-unfriendly implementations ever acceptable”.)

>>In short, I think the text is fine as is.

>I disagree. “Network-friendly” is a condition that triggers a SHOULD. I think there’s a high chance you will see different implementors interpret that condition differently. If the intent is to give normative guidance, there needs to be enough information to avoid that. OTOH, if the intent is to point out things an implementer needs to think about, then the normative SHOULD may not be appropriate.

Yes, I agree with your interpretation.

Proposed change:  “In a network-friendly implementation, an application should avoid sending/receiving FEC repair streams if it knows that sending/receiving those FEC repair streams would not help at all in recovering the missing packets.”

Ensuing language on RECOMMENDED practices can remain unchanged.


>>>§1.1.1: - It would be helpful to describe what L and D represent here, or at least give a forward reference to the definition. (I note that a lot of text goes by before you get to the  definitions or the 2119 boilerplate.)

>>Current text states:  " Consider a group of D x L source packets that have sequence numbers starting from 1 running to D x L, and a repair packet is generated by applying the XOR operation to every L consecutive packets as sketched in Figure 3."
>>I don't think that there is a further definition required.  However, an addition sentence could follow the above description to clarify.
>>Proposed text:  "In general, D and L represent values that describe how packets are grouped together for parity code generation when interleaving all D x L source packets.”

>That’s kind of circular explanation. I was just looking for something along the lines of ‘ “D” and “L” represent the depth and length, respectively”.

Proposed change:  Add following sentence – “In general D and L represent values that describe how packets are grouped together from a depth and length perspective (respectively) when interleaving all D x L source packets.”

>>>§4.2.2, paragraph after Figure 10: The text seems to be suggest there is one source packet protected by a given repair packet. IIUC, that’s not the case; a given FEC packet protects several source packets, doesn’t it?

>>The text reads in part "... includes repair of everything following the fixed 12-byte RTP header of the source packet ...".
>>"... everything following ..." implies more than just a single source RTP packet.

>Let’s say for argument that the repair packet protects 2 source packets. Is the 12-byte RTP header of the 2nd packet protected?
>(Even if the answer is yes, it would be helpful to say " ... and subsequent packets...")

As per the last paragraph of Sec. 4.1:  "The source packets are full RTP packets with optional CSRC list, RTP header extension, and padding. If any of these optional elements are present in the source RTP packet, and that source packet is lost, they are recovered by the FEC repair operation, which recovers the full source RTP packet including these optional elements.”

Proposed change:  Change “... includes repair of everything following the fixed 12-byte RTP header of the source packet ..." to “... includes repair of everything following the fixed 12-byte RTP header of the source packet along with subsequent source packets ..."

>>>§9, last paragraph: I don’t understand the intent of the last sentence.

>>Originally suggested in https://mailarchive.ietf.org/arch/msg/payload/AIHw5DSABEJLa6lSOLryLdI0eBk.  The application source data may not be perfectly matched with FLEX FEC source partitioning.  If this is the case, there is a possibility for unprotected source data if, for instance, the FLEX FEC implementation discards data that does not fit perfectly into its source processing requirements.

>That explanation is more clear than the text in the draft. I suggest including it.

Proposed change:  Replace “In addition, the interaction between a FLEX FEC implementation and higher-layer applications may be affected by non-uniform processing requirements of the FEC scheme” with “Moreover the application source data may not be perfectly matched with FLEX FEC source partitioning.  If this is the case, there is a possibility for unprotected source data if, for instance, the FLEX FEC implementation discards data that does not fit perfectly into its source processing requirements.”

Thanks,

-Giri Mandyam


From: Ben Campbell <[e-mail]>
Sent: Saturday, December 22, 2018 9:15 PM
To: Giridhar Mandyam <[e-mail]>
Cc: [e-mail]; [e-mail]
Subject: Re: AD Evaluation of draft-ietf-payload-flexible-fec-scheme-13

Hi, thanks for the quick response! Comments inline. I removed sections that seem resolved.

Ben.


On Dec 22, 2018, at 7:38 PM, Giridhar Mandyam <[e-mail]<mailto:[e-mail]>> wrote:

Thank you for the careful review.  Before submitting rev 14, I would like to get agreement on the following proposed changes.
-Giri


General: Do I read correctly that this document can completely stand alone from RFC 6363? That is, the calculation of the protection stream and reconstruction of lost source packets is completely specified in this document? I don’t see that as a problem, but if it’s correct some introduction text to that effect would be helpful. Is a normative reference to 6363 even needed?

FLEX FEC certainly was inspired by FEC FRAME, but the document can stand on its own.    Since several definitions are leveraged from RFC 6363, I believe a normative reference is still appropriate.  My suggestion is to add one sentence to the last paragraph of Section 1.

Proposed change:  Add "This document also leverages several definitions along with the basic source/repair header description from RFC 6363 in their application to the parity codes defined here.”

That helps. I might suggest something along the lines of “While this document fully defines the use of FEC to protect RTP streams, it also leverages...”

[...]


§4.2.1:
- There’s a number of 2119/8174 keywords in this section that seem more natural consequences of using RTP than new normative requirements. If that is the case, they shouldn’t use normative keywords unless in the form of direct quotes.

Apologies - I had trouble identifying such instances.  All instances of such keywords are related to requirements with respect to RFC 3550.  Do you have specific examples?

- definition of sequence number. Doesn’t RTP already require the sequence number to increment by one, and recommend the use of a random starting value?

- definition of time stamp: I initially thought that the SHALL reflected a requirement already in 3550, but now I realize that transmission time is not the same as sampling time.  So this one is probably okay.

- SSRC: 3550 already says SSRCs SHOULD be assigned randomly, so the requirement is a statement of fact as far as _this_ spec is concerned. (The text even mentions “as suggested by RFC 3350", although 3550 does more than “suggest” it.)

- "the collisions MUST be resolved as described in [RFC3550]” : again, that follows naturally from using RTP. It does not define a new requirement.



- 10th paragraph: "The repair streams’ SSRC’s CNAME SHOULD be identical to the CNAME of the source RTP stream(s) that this repair stream protects.” Why is the SHOULD not a MUST? What happens if it is violated?

Proposed change:  Agreed that the requirement makes more sense as a MUST.  Will change accordingly.

I concur with the change, but it is a material change. Please point it out to the WG in case anyone objects. (It may not be safe to assume people are reading this note in that level of detail, especially over the holidays :-)) To be clear, I don’t want to block progress of the draft over it; if anyone objects they can do so as part of the IETF last call.

[...]




§4.2.2: Please add a short explanation of why we need two different FEC packet formats. (Not counting the retransmission packet; the reasoning there is pretty obvious.)

Proposed change:  Add additional sentence to first paragraph of Sec. 4.2.2:  "Two of these variants are meant to describe different methods for deriving the source data from a source packet for a repair packet.  The third variant is for a straight retransmission of the source packet.”

That’s useful, but it doesn’t explain _why_ we need two different methods.




§5.1.1 (and subsequent media type definitions) - Please consider using the IESG ([e-mail]<mailto:[e-mail]>) as the contact for further information. Working groups come and go, and authors change jobs. But someone on the IESG should (hopefully) always be able to find the right person,

TENTATIVE proposed change:  I am happy to swap out Varun's email for IESG, but this does not seem to be a widespread practice even for recently-published RFC's.  Can you confirm that this is now becoming a best practice for contact info regarding registries?

It’s been common of late. The reasoning is that the IESG email address is the least likely to change of any contact we might give.




- Change control:  In 5.1.1, shouldn’t that be the PAYLOAD working group (or it’s successor as delegated by the IESG...)

Proposed change:  Change to "IETF Audio/Video Payloads Transport Payloads Working Group”

I suggest adding “... or it’s successor as delegated by the IESG.





- Are these registrations really intended to be provisional?

At this point, no.

Proposed change:  Eliminate mention of provisional registration for all media types.

I agree. However this is another material change that should be pointed out to the WG.




§5.2.1, last bullet: "Any unknown option in the offer MUST be ignored and deleted from the answer.”: Is that a a new requirement specific to flex-fec, or just a normal offer/answer requirement?

RFC 3264 Section 6 states
'For each "m=" line in the offer, there MUST be a corresponding "m=" line in the answer.  The answer MUST contain exactly the same number of "m=" lines as the offer.  This allows for streams to be matched up based on their order.  This implies that if the offer contained zero "m=" lines, the answer MUST contain zero "m=" lines.'

I don't believe this is normal offer/answer.  It is specific to FLEX FEC.

Okay.

[...]



- “In a network-friendly implementation”: Do you expect to see network-unfriendly implementations? Is that okay?

I don't view "network-friendly" as being a static condition.  There are scenarios where an FEC transmitter/receiver pair have negotiated a connection which is network-friendly for one application, but may not be for a different application.  There was a discussion about when FEC is appropriate and when it is not on the WebRTC mailing lists - see https://lists.w3.org/Archives/Public/public-webrtc/2018Nov/0156.html.   "Network-unfriendly" implementations could conceivably occur when an FEC session has been negotiated between endpoints, but neither endpoint has assessed transmission conditions.

Is that a reasonably implementation? It sounds like a bug to me. (I probably could have phrased my question better as “ are network-unfriendly implementations ever acceptable”.)


In short, I think the text is fine as is.

I disagree. “Network-friendly” is a condition that triggers a SHOULD. I think there’s a high chance you will see different implementors interpret that condition differently. If the intent is to give normative guidance, there needs to be enough information to avoid that. OTOH, if the intent is to point out things an implementer needs to think about, then the normative SHOULD may not be appropriate.




§9, last paragraph: What are the security implications of unused source-buffers? Is this a potential DoS vector?

It could be.  Unused source buffers combined with potentially large packet size (e.g. for bundled protection) could eventually result in a lack of resources (e.g. memory) in an endpoint.

Proposed additional clarifying text: Following  "Given that FLEX FEC enables the protection of multiple source streams, there exists the possibility that multiple source buffers may be created that may not be used" add  "An attacker could leverage unused source buffers to as a means of occupying memory in a FLEX FEC endpoint.”

Works for me.

[...]




- 2nd paragraph: s/Redunadncy/Redundancy”

Proposed change:   "Redunadncy" changed to "Redundancy".  Thank you for catching this.


§1.1.1: - It would be helpful to describe what L and D represent here, or at least give a forward reference to the definition. (I note that a lot of text goes by before you get to the  definitions or the 2119 boilerplate.)

Current text states:  " Consider a group of D x L source packets that have sequence numbers starting from 1 running to D x L, and a repair packet is generated by applying the XOR operation to every L consecutive packets as sketched in Figure 3."

I don't think that there is a further definition required.  However, an addition sentence could follow the above description to clarify.

Proposed text:  "In general, D and L represent values that describe how packets are grouped together for parity code generation when interleaving all D x L source packets.”

That’s kind of circular explanation. I was just looking for something along the lines of ‘ “D” and “L” represent the depth and length, respectively”.

[...]


§4.2.2, paragraph after Figure 10: The text seems to be suggest there is one source packet protected by a given repair packet. IIUC, that’s not the case; a given FEC packet protects several source packets, doesn’t it?

The text reads in part "... includes repair of everything following the fixed 12-byte RTP header of the source packet ...".

"... everything following ..." implies more than just a single source RTP packet.

Let’s say for argument that the repair packet protects 2 source packets. Is the 12-byte RTP header of the 2nd packet protected?

(Even if the answer is yes, it would be helpful to say " ... and subsequent packets...")

[...]


§5.2.2, first paragraph: Is there a reason to cite the obsolete RFC 2326?

This also came up as part of the I.-D. nits.  I decided to leave the 2326 reference in because I did not want to imply that FLEX FEC cannot be used for RTSP 1.0, and RTSP 2.0 is not backwards-compatible.

I suggest adding a few words to explain that.

[...]



§9, last paragraph: I don’t understand the intent of the last sentence.

Originally suggested in https://mailarchive.ietf.org/arch/msg/payload/AIHw5DSABEJLa6lSOLryLdI0eBk.  The application source data may not be perfectly matched with FLEX FEC source partitioning.  If this is the case, there is a possibility for unprotected source data if, for instance, the FLEX FEC implementation discards data that does not fit perfectly into its source processing requirements.

That explanation is more clear than the text in the draft. I suggest including it.

[...]