Re: [payload] Benjamin Kaduk's Discuss on draft-ietf-payload-flexible-fec-scheme-17: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Tue, Mar 05, 2019 at 05:08:43PM +0000, Giridhar Mandyam wrote:
> Looks like my response got cut off.  Let me try again.
> 
> Thanks Benjamin.  Apologies for the late reply, but we (the editors) were confirming the best response to your follow-up questions.

And my apologies as well for the late reply here.

The published -18 addresses my DISCUSS-level concerns, so I've changed my
ballot to No Objection.

I just have a couple more thoughts, inline.

> > Maybe my question was not phrased well.  IIUC, the repair packets can only cover real media source streams that are part of the same RTP session.
> So in order for both audio and video to be available in the same RTP session as potential input to asingle repair packet stream, the audio and video would need to be together in the same RTP session; presumably that would involve getting demux'd based on payload type or SSRC, which is what I'm reading 3550 as saying to not do.  Am I just misunderstanding that the repair inputs must be part of the same RTP session?
> 
> In my interpretation, the issue revolves around what "demultiplexing" means in this context.  I do not believe it was meant to contemplate the recovery of packets from a repair stream, which involves more than just "demultiplexing" of audio and video payloads.  I don't believe Sec. 5.2 of RFC 3550 really applies.

Magnus also followed up here.  I wasn't worried about the direct behavior
specified in this document, but rather that a scenario we were describing
as an example usage was describing something that RFC 3550 says to not do.
Fortunately, that guidance from RFC 3550 has been rescinded, so my initial
guess of "3550 was overly cautious and we don't worry about that anymore"
was correct.

> > >       Payload Type: The (dynamic) payload type for the FEC repair packets is determined through out-of-band means.  [...]
> > >      Is "(e.g., SDP)" applicable here?
> 
> Sorry I missed this in my original response.  Yes, it does make sense and I have added it.
> 
> >>> Any reason not to include "retransmit" and "fixed block" mnemonics for the 'R' and 'F' bits? 
> >> Can you explain further?  The R/F definitions already include mention of retransmission and fixed FEC L/D.
> > I was thinking that the first sentence could say '(R and F bits, for "retransmit" and "fixed block")' to (in effect) "name" the bits right away.
> 
> Proposed text:
> 
> 4.2.2.  FEC Header of FEC Repair Packets
> 
>    The format of the FEC header has 3 variants, depending on the values
>    in the first 2 bits (R and F bits) as shown in Figure 10.  Note that
>    R and F stand for "retransmit" and "fixed block", respectively.
> 
> > >> Please include a note here that several fields (e.g., P, PT, etc.)  
> > >> in the FEC header are not meant to be interpreted directly but are 
> > >> instead actual FEC parity data akin to the following "payload". 
> > >> (Absent such an indication, the reader could see that these fields 
> > >> are "used to determine" values when they appear to contain values 
> > >> directly, and get confused.)
> > > >I would suggest adding a forward-reference to Section 6 since that describes how the Repair Payload is calculated.
> >> I am sorry.  I do not understand what is meant by "used to determine" values.  Can you explain?
> >I was trying to consolidate remarks about 4.2.2, 4.2.2.1 and 4.2.2.2 into a single comment, but it came across badly; sorry for the confusion.  The last bullet point of Section 4.2.2 notes that "The P, X, CC, M and PT recovery fields are used to determine the corresponding fields of the recovered packets". Similarly, Figures 12 and 13 both have fields named for "PT recovery", "length recovery", etc., with corresponding descriptions in the main text like "the length recovery (16 bits) field is used to determine the length of the recovered packets" -- these are the "used to determine" text to which I refer.  So my concrete suggestion is to add a reference to Section 6.3.2 at the end of that final bullet in Section 4.2.2.  I'd consider adding it to the field descriptions in 4.2.2.1 as well, but that might be overkill, so use your judgement.
> 
> Forward reference is added.  Thanks.
> 
> > >> Should implementations set bounds on L and D that are smaller than the maximum encodable value (255)?
> > > Yes.  This is assumed.
> >Perhaps it's best to state it explicitly rather than leaving an implicit assumption.
> 
> > >> If L=0, D=0, use the optional payload format parameters for L and D.  What is the behavior when those payload format parameters were not provided?
> > >Optional payload format parameters may be provided in SDP (see Sec. 5.1).
> >Right.  But suppose they weren't, and yet the L=0,D=0 packet shows up on the wire.  This is clearly an error condition; what's the error handling?
> 
> Proposed text immediately following Fig. 14:
> 
>    Given the 8-bit limit on L and D (as depicted in Figure 13), the
>    maximum value of either parameter is 255.  The "optional payload
>    format parameters" (Figure 14) to be used when L=0 and D=0 can be
>    determined through out-of-band signaling (see Section 5).  If L=0 and
>    D=0 and cannot be determined through out-of-band signaling, then the

nit: maybe "If L=0 and D=0 in a packet and the intended values cannot be
determined [...]"

>    repair packet MUST be ignored by the receiver.  In addition when L=1
>    and D=0, the repair packet becomes a retransmission of a
>    corresponding source packet.
> 
> > >> The L=1 case seems to imply that some full packet retransmission will be used; is it worth calling that out as a consequence of such a parameter choice?
> >> I am not sure that I understand this statement.  L=1 does not imply anything about the value of D.
> >I agree with your statement.  But, we seem to allow for L to be 1, and in particular for L=1,D=0 (also L=1,D=1).  For those combinations of L and D, the "Row FEC" case will in fact be packet retransmission, as a degenerate case.  I'm proposing that either this oddity is mentioned explicitly (e.g., "note that when L=1, the Row FEC packets will just be retransmission"), or disallowed by the numerical constraints.  Though now that I know better how the multi-SSRC case is handled (per Magnus's comments and my reply thereto), I guess you could use L=1 as part of a multi-SSRC repair and it would not be equivalent to retransmission.  So the situation is more complicated than I first thought.
> >Regardless, since the conditions here do allow L=1 as a possibility, I suggest some additional discussion in the text about how it's a bit strange and probably not expected to be used.
> 
> As you mentioned above for bundled protection, individual L/D combinations can be declared for each SSRC (such as L=1,D=0) but the overall repair packet may in fact not be a retransmission of any individual packet.  An example could be an audio/video session where very few audio packets (as few as one) are bundled with several video packets in the generation of the repair packet.  In such a case, it might be perfectly valid to set L=1, D=0 for the audio packet.   So I am not sure about what is best approach to the additional discussion.  Did you have a concrete suggestion?

I think that there are too many possibilities for how source packets can be
combined into the recovery packet for it to make sense to add any text
along these lines -- the case where the repair packet is effectively
retransmission is just a small part of the set of possibilities, and not
one that is especially important or common, so it does not need to get
mentioned specially.  Leaving the text as-is should be fine.

> >> > What is the interaction between rate, repair-window, and the L and D values?  That is, if we set L and D to be large, and rate to be small, can we end up claiming a repair window that is too small to accumulate the necessary L*D source packets and compute recovery packets?
> >> Yes, it is possible.  We expect that specific uses of FLEX FEC will bound the appropriate values for repair window, L and D.  It is difficult to establish these bounds in this specification, however, since the applications that are currently making use of FLEX FEC are varied (e.g. WebRTC, 3GPP MTSI).  We expect these specification to provide concrete guidance on the expected repair windows, L and D, based on the target application.
> >Please do not rely on unstated expectations for the behavior of consumers of this mechanism!  I strongly suggest to provide some indication, even a vague and incomplete one, that there are to be interrelations and implementation restrictions on these parameters.
> 
> Proposed additional section:
> 
> 
> 1.1.7.  Repair Window Considerations
> 
>    The value for the repair window duration depends on the L and D
>    values and cannot be chosen arbitrarily.  More specifically, L and D
>    values determine the lower limit for the repair window duration.  The
>    upper limit of the repair window duration does not depend on the L
>    and D values.
> 
> Note that a discussion of rate is difficult (in my opinion) without making assumptions about the source SSRC's.  For the above section, however, I can add a sentence such as "The rate of the source streams should also be considered, as the repair window duration should ideally span several packetization intervals in order to leverage the error correction capabilities of the parity code."  

I think that would be helpful to add.

> >> > Section 5.2.1
> >>>    o  The value for the repair-window parameter depends on the L and D values and cannot be chosen arbitrarily.  More specifically, L and D values determine the lower limit for the repair-window size. The upper limit of the repair-window size does not depend on the L and D values.
> >>>Per my above remark, this consideration seems generally applicable and not limited to SDP Offer/Answer.
> >> This is also covered in Sec. 1.1, which provides the general guidance.
> >An inline note that the repair window "is defined as the time that spans a FEC block", I see now.  But this is still in fairly abstract terms; I would have expected to see the note I quoted in my ballot to appear in a top-level Section 5 entry, and not in the subsection specific to SDP O/A.
> 
> >>>    o  Any unknown option in the offer MUST be ignored and deleted from the answer.  If FEC is not desired by the receiver, it can be deleted from the answer.
> >>> This sounds like it is restating an existing normative requirement  (in which case a reference and descriptive, non-normative, text seems appropriate).
> >> This requirement is specific to SDP O/A. Can you explain further as to why you think there is a different normative requirement?
> >Ali got what I intended, though I was hoping for a reference to the relevant SDP RFC as well as the s/MUST/must/.
> 
> Proposed modifications:
> 
>    o  The value for the repair-window parameter depends on the L and D
>       values.  Based on the values of L and D, a lower bound on the
>       repair-window can be inferred (see Section 1.1.7).
> 
>    o  Although combinations with the same L and D values but with
>       different repair-window sizes produce the same FEC data, such
>       combinations are still considered different offers.  The size of
>       the repair-window is related to the maximum delay between the
>       transmission of a source packet and the associated repair packet.
>       This directly impacts the buffering requirement on the receiver
>       side and the receiver must consider this when choosing an offer.
> 
>    o  Any unknown option in the offer must be ignored and deleted from
>       the answer (see Section 6 of [RFC3264]).  If FEC is not desired by
>       the receiver, it can be deleted from the answer.
> 
> Sec. 6.2:
> > To be clear, this is an editorial matter and you are free to ignore me, but my suggested wording is "The first 16 bits of the RTP header (16 bits), though the first two (version) bits will be ignored by the recovery procedure".
> 
> Proposed text is added.
> 
> > s/compute the FEC bit string from/extract the FEC bit string as/
> 
> Proposed text is added.
> 
> >>> I don't see how this matches up with the bit string construction in  Section 6.2.
> >> As per Sec. 6.2,  "The rest of the FEC bit string, which contains everything after ...
> >In particular I'm confused about the order between length recovery and TS recovery.  In the FEC header (for non-retransmissions), length recovery appears before TS recovery.  In Section 6.2, as we extract things from the FEC bit string, we write out the length recovery value into the FEC header, and then (the next bits from the FEC bit string are) the TS recovery value.
> >But here we take the TS field and then the length field from the recovery bit string, which is in the other order.  Am I missing some step that causes the order that these fields appear in the bit stream to change?
> 
> Agreed.  The proposed change that seems to fix this  in Sec. 6.3.2 is:
> 
>    11.  Set the SN field in the new packet to SEQNUM.
> 
>    12.  Take the next 16 bits of the recovered bit string and set the
>         new variable Y to whatever unsigned integer this represents
>         (assuming network order).  Convert Y to host order.  Y
>         represents the length of the new packet in bytes minus 12 (for
>         the fixed RTP header), i.e., the sum of the lengths of all the
>         following if present: the CSRC list, header extension, RTP
>         payload and RTP padding.
> 
>    13.  Set the TS field in the new packet to the next 32 bits in the
>         recovered bit string.
> 
>    14.  Set the SSRC of the new packet to the SSRC of the missing source
>         RTP stream.

Thank you for checking this.  I was really worried that I was just missing
a step in the procedure, so it's good to hear that I was correct to be
confused.

> >> "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.  An attacker could leverage unused source buffers to as a means of occupying memory in a FLEX FEC endpoint.  ***In addition, an attack against the FEC parameters themselves (e.g. repair window, D or L values) can result in a receiver having to allocate source buffer space that may also lead to excessive consumption of resources.***  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. " 
> 
> >That's an improvement, thanks.  I don't think it covers my penultimate paragraph about a network attacker being able to force bit flips in the recovered packet length field, though, and I do think it's worth documenting that as a risk (when integrity protection is not used).
> 
>   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.  An attacker could leverage
>    unused source buffers to as a means of occupying memory in a FLEX FEC
>    endpoint.  In addition, an attack against the FEC parameters
>    themselves (e.g. repair window, D or L values) can result in a
>    receiver having to allocate source buffer space that may also lead to
>    excessive consumption of resources.  ***Similarly, a network attacker
>    could modify the recovery fields corresponding to packet lengths
>    (assuming there are no message integrity mechanisms) which in turn
>    could force unnecessarily large memory allocations at the receiver.***
>    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, that's exactly what I was looking for.

> >I would strongly suggest adding some text at the end of or after the first pargaraph of section 4.2.2.2, to the effect of:
> 
> >  The values of L and D for a given block of recovery data will 
> > correspond to
>   the type of recovery in use for that block of data.  In particular, for 2-D
>   repair, the (L,D) values will not be constant across all packets for a
>   given SSRC being repaired.  Similarly, the L and D values can differ across
>   different blocks of repair data (repairing different SSRCs) in a single
>   packet.
> 
> Proposed text has been added.  Thanks.
> 
> >Regardless, I am still not 100% clear on the layout of the repair payload for the multi-SSRC case.  E.g., in Figure 13, we see that there are multiple SN base/L/D entries before the payload, which is a single consolidated payload that (presumably) covers all the recovery SSRCs.
> Your description above makes it sound like each SN base/L/D indicates a set of packets, and the payload is the XOR of all of those sets together (padded if necessary).  That is, one could perhaps imagine conceptually doing this in two steps: (1) XOR together the packets indicated by the respective SN base/L/D for each SSRC, and then (2) XOR those results together to combine the N SSRC recovery blocks into a single repair payload.  Did I get it right this time?
> 
> You can do it as you have described.  It is not strictly required to do it that way.  The source packets could be arranged in such a way that the XOR operation can be performed in one step, rather than the two you describe.  Any method is fine as long as the desired parity is created - row-wise, column-wise, or flex mask.

Understood.  I was breaking the procedure into two steps in my head so that
it was easier to understand (or describe), but of course they do not need
to be separate steps in an actual implementation.

Thank you again for the discussion and updates!

-Benjamin

> Note that we have also modified sec. 4.2.1 as follows to better describe the repair RTP header:
> 
> CSRC Count (CC) 4 bits, and CSRC List (CSRC_i) 32 bits each:
> Source packets can have an optional CSRC list and count, which can
> be recovered. FEC repair packets MUST use the CSRC list and count
> to specify the SSRC(s) of the source RTP stream(s) protected by
> this FEC repair packet.
> 
>