[rtcweb] Review of draft-ietf-rtcweb-fec

["Mo Zanaty (mzanaty)" <[e-mail]>]

Here is my review of draft-ietf-rtcweb-fec-04.

4.1. Recommended Mechanism (for audio FEC), first paragraph says:

OLD:
"When using the Opus codec, use of the built-in Opus FEC mechanism is
RECOMMENDED. This provides reasonable protection of the audio stream
against typical losses, with modest overhead. Note that as indicated
above the built-in Opus FEC only provides single-frame redundancy; if
multi-packet protection is needed, the built-in FEC should be
combined with [RFC2198] redundancy to protect the N-2th, N-3rd, etc.
packets."

The last sentence about multi-packet protection should not be recommended due to overhead. Packing multiple full-size Opus frames, each with their own built-in prior-frame FEC, into RED packets is akin to packing multiple PCMU frames into RED packets. The latter is discouraged 3 paragraphs later:

"When using constant-bitrate codecs, e.g. PCMU, use of [RFC2198]
redundant encoding MAY be used, but note that this will result in a
potentially significant bitrate increase, and that suddenly
increasing bitrate to deal with losses from congestion may actually
make things worse."

These same warnings would apply to multi-packet Opus RED. Therefore I suggest rewording the first paragraph as follows.

NEW:
"When using the Opus codec, use of the built-in Opus FEC mechanism is
RECOMMENDED. This provides reasonable protection of the audio stream
against typical losses, with modest overhead. Note that as indicated
above the built-in Opus FEC only provides single-frame redundancy; if
multi-packet protection is needed, the built-in FEC MAY be
combined with [RFC2198] redundancy to protect prior packet pairs,
but note this will result in significant bitrate increase which may
aggravate congestion losses."

4.2. Negotiating Support, first sentence says:

OLD:
"Support for redundant encoding MUST be indicated by offering "red"..."

This may be misinterpreted as mandating that WebRTC endpoints MUST offer "red", rather than merely indicating that if they choose to support redundant encoding (which is only RECOMMENDED for VBR codecs without internal FEC in the prior section), then it MUST be indicated by offering "red". I suggest rewording this as:

NEW:
"If redundant encoding is supported, it MUST be indicated by offering "red"..."

Same comment for Opus in the following paragraph:

OLD:
"For Opus, a receiver MUST indicate that it is prepared to use
incoming FEC data with the "useinbandfec=1" parameter..."

NEW:
"For Opus, a receiver that it is prepared to use incoming FEC data
MUST include the "useinbandfec=1" parameter..."

5.1. Recommended Mechanism (for video FEC) says:

OLD:
"For video content, use of a separate FEC stream with the RTP payload
format described in [I-D.ietf-payload-flexible-fec-scheme] is
RECOMMENDED. The receiver can demultiplex the incoming FEC stream by
SSRC and correlate it with the primary stream via the SSRC field
present in the FEC header."

Flex FEC moved the SSRC binding from the FEC header to the CSRC list. Only the retransmission format still has the SSRC field in the FEC header. Reword as:

NEW:
"For video content, use of a separate FEC stream with the "flexfec" RTP payload
format described in [I-D.ietf-payload-flexible-fec-scheme] is
RECOMMENDED. The receiver can demultiplex the incoming FEC stream by
SSRC and correlate it with the primary stream(s) via the CSRC(s)
in the RTP header of the FEC repair packet, or via the SSRC field
in the FEC header for retransmissions."

The next paragraph suggests multiple source streams is a problem.

OLD:
"Support for protecting multiple primary streams with a single FEC
stream is complicated by WebRTC's 1-m-line-per-stream policy, which
does not allow for a m-line dedicated specifically to FEC."

But Flex FEC already supports this with SSRC(s) of primary stream(s) as CSRC(s) of the FEC stream, so strike the above OLD paragraph.

8. Adaptive Use of FEC, first paragraph says:

OLD:
"...methods like RTX [RFC4588], which only transmits redundant data when..."

Flex FEC also supports retransmissions, so reword as:

NEW:
"...methods like RTX [RFC4588] or the "flexfec" retransmission format,
which only transmits redundant data when..."

Same comment in the next paragraph.

OLD:
"Given this, WebRTC implementations SHOULD consider using RTX instead..."

NEW:
"Given this, WebRTC implementations SHOULD consider using RTX or
the "flexfec" retransmission format instead..."

9. Security Considerations

Add a final paragraph on the order of FEC and SRTP operations.

NEW:
"SRTP [RFC3711] defines the default order of FEC and SRTP as FEC followed by SRTP at the sender, and SRTP followed by FEC at the receiver. DTLS-SRTP [RFC5764] uses this same default order for all SRTP Protection Profiles."

Editorial:

Abstract and Introduction should use WebRTC "endpoint" as defined in -overview.
Abstract: "... FEC ... used by WebRTC applications" -> WebRTC endpoints
Introduction: "... FEC ... for WebRTC client implementations" -> WebRTC endpoints
Or you could be very generic and just say WebRTC implementations everywhere.

Mo