WebRTC Forward Error Correction Requirements
draft-ietf-rtcweb-fec-08

Summary: Has enough positions to pass.

(Ben Campbell) Yes

Comment (2019-02-20)
Thanks for this effort. I am balloting "yes", but have some minor comments:

§8:
- "Given this, WebRTC implementations SHOULD consider using RTX or
flexfec retransmissions instead of FEC when RTT is low,"

"consider" seems vague for a normative requirement. Can you describe concrete requirements? Otherwise I suggest descriptive language.

Can you give guidance on what RTT would be reasonable to consider as "low"?

- "Note that when probing bandwidth, i.e., speculatively sending extra
data to determine if additional link capacity exists, FEC SHOULD be
used in all cases."

I assume the point of this is the redundant FEC data should _be_ that extra data. But one could read this to mean that, if you are already sending extra data, you should also use FEC.

§9, 2nd paragraph:

I'm by no means an expert in this, and leave it to the crypto experts to know if this matters, but does the additional redundancy of FEC have any impact on SRTP encryption?

Adam Roach Yes

Ignas Bagdonas No Objection

Deborah Brungard No Objection

Alissa Cooper No Objection

(Spencer Dawkins) No Objection

Benjamin Kaduk No Objection

Comment (2019-02-17)
Section 3

nit: The subsequent discussion seems to indicate that at least some of
these mechanisms are already specified and not new in this document; (if
so) it would be nice to have the exposition reflect that.

Section 3.3

   For Opus, packets deemed as important are re-encoded at a lower
   bitrate and added to the subsequent packet, allowing partial recovery

Is "added" supposed to be something other than "appended" (which strongly
resembles the "redundant encoding" of the previous section)?

Section 4.1

Does it make sense to give subsection backreferences when talking about
(e.g.) redundant encoding?

Section 5.2

   Support for a SSRC-multiplexed flexfec stream to protect a given RTP
   stream SHOULD be indicated by including one of the formats described
   in [I-D.ietf-payload-flexible-fec-scheme], Section 5.1, as an

nit: Since this Section 5 is solely for video, is it more appropriate to
refer to Section 5.1.2 ("Registration of video/flexfec") of
draft-ietf-payload-flexible-fec-scheme?

Section 7

   To support the functionality recommended above, implementations MUST
   be able to receive and make use of the relevant FEC formats for their
   supported audio codecs, and MUST indicate this support, as described
   in Section 4.  Use of these formats when sending, as mentioned above,
   is RECOMMENDED.

Just to double-check: this is explicitly only mandating FEC for audio and
ignoring video entirely?

Section 8

   Because use of FEC always causes redundant data to be transmitted,
   and the total amount of data must remain within any bandwidth limits
   indicated by congestion control and the receiver, this will lead to
   less bandwidth available for the primary encoding, even when the
   redundant data is not being used.  This is in contrast to methods
   like RTX [RFC4588] or flexfec [I-D.ietf-payload-flexible-fec-scheme]
   retransmissions, which only transmit redundant data when necessary,
   at the cost of an extra roundtrip.

This seems to imply that "FEC" is a specific usage and that flexfec is not
a subset of generic FEC.  If so, this could probably be reworded to be
less confusing to the reader (though my suspicion is that the "always
causes redundant data to be transmitted" is only intended to apply to
specific mechanisms from Section 3).

Section 9

This document seems to be agnostic on the question of RTP vs. SRTP; I would
consider referencing their respective security considerations as well as
what is already covered here.

   As described in [RFC3711], Section 10, the default processing when
   using FEC with SRTP is to perform FEC followed by SRTP at the sender,
   and SRTP followed by FEC at the receiver.  This ordering is used for
   all the SRTP Protection Profiles used in DTLS-SRTP [RFC5763], as
   described in [RFC5764], Section 4.1.2.

I was going to comment about the lack of clarity here, but I see that Ekr
has already gotten the core points, and that the secdir review has already
resulted in some chanegs in the editor's copy.  It would be nice to have
the result of the (merged) edits available to look at, though.

Suresh Krishnan No Objection

Warren Kumari No Objection

Comment (2019-02-20)
Please see the OpsDir review at: https://datatracker.ietf.org/doc/review-ietf-rtcweb-fec-08-opsdir-telechat-schoenwaelder-2019-02-12/ -- it contains some useful comments.

Also, a small nit: 
Abstract:
"This document defines a new concep of enabling" - typo for concept.

Mirja Kühlewind No Objection

Comment (2019-02-19)
I'm not fully sure about the intended status of this document. The shepherd write-up says "the document has normative requirements for conforming WebRTC implementations", however, for me it seems this document makes "only" recommendations and has actually no normative requirements. Therefore informational status might be more appropriate, however, I will not block publication as PS.

One mostly minor editorial note:

Sec 3.3: "experiments performed indicate that when Opus FEC is used, the overhead imposed
   is about 20-30%, depending on the amount of protection needed."
Would it be possible to provide a reference for this number?

Also this section says: "See [RFC6716], Section 2.1.7 for complete details."
However, section 2.1.7 in RFC6716 is very short and actually does not provide any details...

Alexey Melnikov No Objection

(Eric Rescorla) No Objection

Comment (2019-02-17)
Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D3278



COMMENTS
S 5.2.
>      as described in [RFC5956], Section 4.1 is not currently defined for
>      WebRTC, and SHOULD NOT be offered.
>   
>      Answerers SHOULD reject any FEC-only m-lines, unless they
>      specifically know how to handle such a thing in a WebRTC context
>      (perhaps defined by a future version of the WebRTC specifications).

It seems like the above three paragraphs are generic to this document,
and just grouped with video because the audio codecs tend to have
internal FEC? If so, maybe put them elasewhere?


S 9.
>   
>      As described in [RFC3711], Section 10, the default processing when
>      using FEC with SRTP is to perform FEC followed by SRTP at the sender,
>      and SRTP followed by FEC at the receiver.  This ordering is used for
>      all the SRTP Protection Profiles used in DTLS-SRTP [RFC5763], as
>      described in [RFC5764], Section 4.1.2.

I of course agree with this text, but I wonder if it's maximally
clear. Perhaps rewrite the
first sentence as:

```The FEC schemes described in this document use other packets to
recover when a packet is lost or damaged but do not allow for recovery
of a damaged packet on its own. This is consistent with the default
processing for using FEC with SRTP described in RFC 3711, which is to
perform FEC followed by SRTP at the sender, and SRTP followed by FEC
at the receiver, which implies that damaged packets will be rejected
by the SRTP integrity check and discarded.```

Alvaro Retana No Objection

Martin Vigoureux No Objection

Roman Danyliw No Record

Barry Leiba No Record

Éric Vyncke No Record

Magnus Westerlund No Record