Re: [tsvwg] WGLC on FECFRAME RLC FEC

[Vincent Roca <[e-mail]>]

Hello Gorry,

> I have the read draft-ietf-tsvwg-rlc-fec-scheme-02 during WGLC and have the following comments (as an individual).
> These comments are all on the text and the standards requirements being stated, they do not request changes to the method.
> Gorry
> 

Thanks a lot for this very detailed review. It’s sometimes challenging and really helpful.
Here are my answers. Basically I agree with everything and updated the I-D accordingly.

I'll send the updated version of the I-D next week since we have spotted a few more things that need
to be fixed. More to come soon...


Cheers,

  Vincent and Belkacem


> ---
> /  They are meant to
>   protect arbitrary media streams along the lines defined by FECFRAME
>   extended to sliding window FEC codes./
> - I think the word /meant/ reads a little weird. Rather as if they maybe do not do this. I suggest something like /They can provide protection to/ may be better?

VR: Fixed. 

NEW: 
They can protect arbitrary media streams...


> ---
> Just to be sure, I would add /packet/ or something to /erasure recovery capabilities/ to be clear that this is not a bit-level FEC. Perhaps the first use of /FEC/ could also be better as /Application-Lyer FEC/ for the same purpose?

VR: Very good point. We now mention "packet erasure recovery capabilities » in the abstract to avoid mis-understanding.
We also expend FEC acronym in the abstract (it was only the case in the title and Introduction).


> ---
> /in case of reliable file transfers/
> could be clearer as:
> /when used for reliable file transfers/

VR: Done.


> ---
> /Defining this block size requires to find an/
> could be easier as:
> /To define the block size, it is required to find an/

VR: Done.

> ---
> In 1.2 it states:
> /This choice will impact all receivers./
> - Maybe I misunderstand, but does it impact a receiver that sees no loss, because that could (in theory) act on the data as it arrives?

VR: A perfect receiver will receive source data first and should not be impacted in any way. But that’s an exception. All the other receivers,
even those who experience a tiny (but not zero) loss rate, need to wait the same time on average: the time needed to receive repair packets
for that block, which directly depends on the block size chosen for the worst receiver. I’ve clarified the sentence.

OLD:
This choice will impact all receivers.

NEW:
This choice then impacts the
FEC-related latency of all receivers, even those experiencing a good
communication quality, since no FEC encoding can happen untill all
the source data of the block is available at the sender, which
directly depends on the block size.

> ---
> /are extremely efficient/
> - This seems like a value judgment. I'd be content with saying /efficient/, but to say more I feel really needs a strong evidence that has consensus. RFCs must not be used to market techniques.

VR: Sure, you’re right. I propose the following sentence: 

OLD:
	These FEC Schemes are extremely efficient for instance with media that
	feature real-time constraints sent within a multicast/broadcast session.

NEW:
	These FEC Schemes, and more generally sliding window FEC codes, are recommended
	for instance with media that feature real-time constraints sent within a multicast/broadcast
	session [Roca17].


> ----
> /latency close to zero/
> - same comment, but perhaps this could be stated as /of the order of the time to transmit a packet/ our similar?

VR: Similarly I have changed for something more objective:

OLD:
	However the receivers experiencing a good to medium communication quality will observe
	a FEC-related latency close to zero [Roca17] since...

NEW:
	However the receivers experiencing a good to medium communication quality will observe
	a reduced FEC-related latency compared to block codes [Roca17] since...


> ---
> /waiting the end of the block for the first repair packet to arrive./
> - perhaps clearer as:
> /waiting for the first repair packet to arrive at the end of the block./

VR: Done.


> ---
> /sliding window codes achieve more easily a target/
> - perhaps clearer as:
> /sliding window codes can more achieve more efficiently achieve a target/

VR: Done.

NEW: 
	… sliding window codes can more efficiently achieve a target transmission quality...


> ---
> /The Sliding Window RLC FEC Scheme is designed so as to reduce the transmission overhead/
> - perhaps clearer as:
> /The Sliding Window RLC FEC Scheme is designed to reduce transmission overhead/
> -> however I note the next phrases talk about /minimize packet overhead/, which could be seen as header bytes, is the distinction intended, then please explain - if not please use the same term.

VR: Yes, the term « overhead » is misleading here. 

NEW:
	The Sliding Window RLC FEC Scheme is designed to limit the packet header overhead.


> ----
> /receiver to easily reconstruct/
> - remove /easily/, or say they are /sufficient to allow a receiver to reconstruct/ or whatever, but avoid the value statement.

VR: Done.


> ---
> This strikes me as a little odd:
> /In all situations, we MUST have:/
> - If this is an actual RFC2119 requirement, then you need to state it terms of what the message contains. Although here I am unsure this is a protocol requirement, and if that is the case, it could be better to state the /method needs to ensure:/?

VR: Since RFC2119 recommends to use these terms sparingly, I’ve changed for:

OLD:
	In all situations, we MUST have:
	         ew_size <= ew_max_size

NEW:
	At session start, the encoding window will probably be small and then progressively increase until it reaches its maximum value. 
	At any time: 
		ew_size <= ew_max_size


> ---
> This seems an off phrase in a specification:
> /     In practice they
>      will usually be fixed, especially with multicast/broadcast
>      transmissions. /
> - could it instead be true to say something like /An implementation at a sender and receiver can fix these values/... or something like.

VR: I have clarified this part.

NEW:
      An implementation
      at a sender can fix the E parameter and communicate it as part of
      the FEC Scheme-Specific Information (Section 4.1.1.2).  There is
      no need to communicate the cr parameter per see (it's not required
      to process a repair packet at a receiver).  This cr parameter can
      be fixed, however, in specific use-cases and in particular with
      unicast transmissions in presence of a feedback mechanism that
      estimates the communication quality (out-of-scope of FECFRAME),
      the code rate may be adjusted dynamically.


> ---
> /Let us assume that the/ and /Let us also assume/
> - seems rather like a paper, than a specification. Could this be reworded as /In the case when the code rate is .../etc.?

VR: Reworded.


> ---
> /It means that the source flow bitrate needs to be/
> May be easier as:
> /This requires the source flow bitrate to be/

VR: Reworded.


> ---
> Similarly:
> /It means that the transmission bitrate/ as /This requires the transmission bitrate/
> -> but I have a question. of whether the /flow bitrate/ or /the transmission bitrate/ are actually different, and if not=, then why use two terms? Do we need either term as a /rate/ since this appears to be about volume of data and not rate?

VR: We are dealing here with bitrates in bits/s, not volume. We use source (resp. repair) flow bitrates and transmission bitrate to denote
the bitrate at the FECFRAME sender output.
There was a additional term, « input bitrate », that I have replaced with source flow bitrate.


> ---
> /Finding the optimal value/
> - I am unsure how you would define /optimal/ here, is it possible to say suitable/acceptable tradeoff or something like that without implying a need for optimal?
> /and should be determined after simulations or field trials.  This is of course out of scope of this document.
> /
> - and finished like it is a report, rather than a specification. Is it therefore possible to simply say something like /An appropriate tradeoff needs to be determined depending on the use case…./

VR: Removed « optimal » as suggested, and tried to have it less like a report.


> ---
> In section 3.2
> /An ADU, coming from the application/
> - is this at the sender? - just to be sure?

VR: Yes, added « At a sender ».


> ---
> / Indeed, a lost ADU recovered at a receiver must
>   contain enough information to be assigned to the right application
>   flow (UDP port numbers and IP addresses cannot be used to that
>   purpose as they are not protected by FEC encoding).
> /
> - I could not parse this. Are the UDP port numbers and IP addresses not protected by the link integrity check and IP/transport checksum?
> - If they were corrupted they would cause the packet to be delivered to another endpoint??? what is this about?

VR: I admit this is a complex paragraph (the technic is from FECFRAME, it’s not something added here for the pleasure).
It’s not a matter of corruption. I’ve tried to improve the explanation. Here it is:

NEW:
   At a sender, an ADU coming from the application cannot directly be
   mapped to source symbols.  When multiple source flows (e.g., media
   streams) are mapped onto the same FECFRAME instance, each flow is
   assigned its own Flow ID value (see below).  At a sender, this
   identifier is prepended to each ADU before FEC encoding.  This way,
   FEC decoding at a receiver also recovers this Flow ID and a recovered
   ADU can be assigned to the right source flow (note that UDP port
   numbers and IP addresses cannot be used to that purpose as they are
   not recovered during FEC decoding).


> ---
> There seems to be no requirements language at all in section 3.1. Is this intended?

VR: I guess you mean 3.2. No, it’s not intentional. I’ve added a MUST: 

NEW:
	For each incoming ADU, an ADUI MUST created as follows.


> ---
> Section 3.2 speaks of /UDP/ datagrams. I am not sure this is correct, surely this applied to any type of datagram - DCCP, UDP, or anything else that could in the future carry FECFRAME.

VR: Right! Fixed.


> ---
> / Yet, any other
>   implementation of the PRNG algorithm that matches the above
>   validation criteria, like the ones detailed in [PM88], is
>   appropriate.
> /
> - is that actually saying an implementation MAY use any PRNG algorithm that matches …/

VR: Any implementation of the multiplicative congruential algorithm with the constants defined, that
additionally matches the validation criteria could be used.
Look at http://www.firstpr.com.au/dsp/rand31/ <http://www.firstpr.com.au/dsp/rand31/> to get an idea of how prolific the topic is. It’s impressive.

That being said, I’ve removed this sentence to avoid confusion since I already explain that the PM88
algorithm MUST be used. No need to say more.


> ---
> /  The FEC Framework Configuration Information (or FFCI) includes
>   information that MUST be communicated between the sender and
>   receiver(s)./
> - This confuses me about what are the specific requirements, can you please ensure that all the fields in this subsection carry an appropriate RFC2119 keyword (in upper case), to say which are OPTIONAL, REQUIRED, etc and what they MUST/MAY etc specify.

VR: You’re right, the current sentence can be confusing.
We follow here FECFRAME requirements (RFC 6363 section 5.6) that explains what MUST be
detailed in a FEC Scheme specification in terms of signalling (which we actually provide).
I’ve changed text to avoid this confusion (no MUST any more).

NEW:

4.1.1.  FEC Framework Configuration Information

   Following the guidelines of [RFC6363], section 5.6, this section
   provides the FEC Framework Configuration Information (or FFCI).  This
   FCCI needs to be shared (e.g., using SDP) between the FECFRAME sender
   and receiver instances in order to synchronize them.  It includes a
   FEC Encoding ID, mandatory for any FEC Scheme specification, plus
   scheme-specific elements.

4.1.1.1.  FEC Encoding ID
[…]

And the same applies to section 5.1.1. (but without any explanatory text).


> ---
> /A FEC Repair Packet can contain one or more repair symbols./
> - is that a statement, or a permission? e.g.,
> /A FEC Repair Packet MAY contain one or more repair symbols./

VR: Good catch. That’s a technical possibility. Changed for MAY as suggested.


> ---
> /integer carried the Density Threshold/
> - is this /carries/

VR: Fixed.

> ---
> Is this discussion OK, or do you think we need to define the protocol actions using RFC2119 keywords:
> / The only constraint is to increment by
>   1 the repair key for each of them, keeping the same ew_size source
>   symbols, since only the first repair key will be carried in the
>   Repair FEC Payload ID.  The FEC Repair Packet can then be sent./

VR: You’re right.

NEW:
   In that case the repair key for each of
   them MUST be incremented by 1, keeping…


> - /sent/ probably should be /passed to the transport layer for transmission./

VR: Yes, as in the FECFRAME I-D. Done. There’s also another instance where « sent » has been replaced.


> ---
> / containing an ADU, can be passed to the application either
>   immediately or after some time to guaranty an ordered delivery to the
>   application. /
> - This reads like it could be /MAY be passed/

VR: Done. I’ll try to be more careful with RFC2119 keywords in the future.
 

> ---
> /  received after this delay should not be passed to
>   the application./
> - I am unsure whether this needs to be read as /ought/ or /SHOULD/ could you use one of these instead of /should/?

VR: it’s a matter of deciding what component is responsible of detecting late ADUs. 
All things considered, I’d prefer to relax the requirement to do that within FECFRAME. It’s an implementation decision,
we just explain (section 3.1.  Parameters Derivation) how to reduce the risk to lead to late decodings.

NEW:
   With real-time flows, a lost ADU that is decoded after the maximum
   latency or an ADU received after this delay has no value to the
   application.  This raises the question of deciding whether or not an
   ADU is late.  This decision MAY be taken within the FECFRAME receiver
   (e.g., using the decoding window, see Section 3.1) or within the
   application (e.g., using RTP timestamps within the ADU).  Deciding
   which option to follow and whether or not to pass all ADUs, including
   those assumed late, to the application are operational decisions that
   depend on the application and are therefore out of scope of this
   document.

> ---
> In addition some standard transport comments that ought to be fine tuned:
> / Repair packets (symbols) are generated and sent on-the-fly/
> - seems to perhaps be possibly to read as avoids CC, which it should not. Please be careful to indicate that packets are /passed to the transport layer for transmission/, if you think that these may be sent with higher priority than other messages, then please explicitly say this.

VR: Changed.

NEW:
   Repair packets (symbols) are generated on-the-fly, computing a random linear
   combination of the source symbols present in the current encoding window, and
   passed to the transport layer.


> ---
> /where packets are sent with a fixed period/
> - I think a word of caution here is needed with respect to CC. So this probably needs to be carefully worded, or at least say /are passed to the transport layer each fixed period/.

VR: the CBR assumption was needed to derive parameters, but I agree it gives the
impression CC is bypassed (in fact in our 3GPP use-case there’s no CC).
At the light of our discussions (and also with Belkacem) I’m not satisfied with 
	section 3.1.  Parameters Derivation
and will come back with a brand new version.


> ---
> /The FEC Repair Packet can then be sent./
> .... maybe:
> /passed to the transport layer for transmission/

VR: Done.