Summary: Needs 4 more YES or NO OBJECTION positions to pass.
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.
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...
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.```