Re: [rtcweb] [payload] WGLC on RTP Payload Format for Flexible Forward Error Correction

[Stephen Botzko <[e-mail]>]

I have reviewed it, but I don't think it is ready for publication yet.

My commends follow.

BR
Stephen



>>>

Abstract

>>>

Overall, the purpose is to reconstruct RTP source packets that were
lost in transmission, using the source and repair packets that were
received.  I expected that to be stated somewhere in the abstract and
introduction.



>>>

These parity codes are systematic codes, where a number of repair

   symbols are generated from a set of source symbols.

>>>

This is of course the usual FEC jargon.  But it isn’t clear if the
symbols in this draft are bits, bytes, or the full source packets
payloads.

Overall, the draft generally uses the phrase “source packet” and
“repair packet”.  “repair symbols” shows up in section 4.2, and there
it seems to mean bits or bytes.  The draft needs to clarify this, as
the many implementers won’t have any real knowledge of FEC.



I suggest removing references to “symbols”, as the XOR construction
and repair mechanisms really don’t need to talk about “symbols”.



Also, “FEC packets” appears to be the same thing as “repair packets”.
It would be better to use one term for both.  Repair packets might be
clearer (though they reconstruct lost packets, and don't actually
repair them).


>>>

These repair symbols are sent in a repair flow separate from the source flow

>>>

The word “flow” appears 19 times in the document with no explanation on
what exactly is meant.  I believe it simply means that the source and
repair packets use different SSRCs (or different payload types?), but this
is not as clear as it might be.

FWIW, I realize that “flow” is also used extensively in RFC6363.   But
RFC6363 defines flows in terms of transport identifier (such as
standard 5 tuple), and that definition doesn’t apply here, since
source and repair flows can use the same 5 tuple.

>>>

Moreover, alternate FEC codes may be used with the payload formats presented.

>>>

Does this mean that the FEC codes might not be XOR?  If so, the document
doesn’t say anything about how that is done (and the repair generation and
recovery methods in the draft are specific to XOR).  So if means what is
meant, the sentence should probably be removed.

If it doesn’t mean that, then what does it mean?



Section 1.1

>>>

   Note that the source and repair packets belong to different source

   and repair flows, and the sender must provide a way for the receivers

   to demultiplex them, even in the case they are sent in the same

   5-tuple (i.e., same source/destination address/port with UDP).

>>>

The “different source and repair flows” aspect of the note is repetitive,
since it is also stated in step 3 immediately above this paragraph.  “Even
in the case” is understated, “especially in the case” is closer to the
truth of it.  It might be simpler to just say that source and repair
packets MUST use different SSRCs  (or different payload types?), and delete
this sentence.



>>>

The repair packets may be sent proactively or on-demand.

>>>

How are they sent on demand?  I don’t see any methods for requesting repair
packets.  Either procedures should be defined/referenced or the sentence
deleted.

>>>

In this document, we refer to the time that

   spans a FEC block, which consists of the source packets and the

   corresponding repair packets, as the repair window.  At the receiver

   side, the FEC decoder should wait at least for the duration of the

   repair window after getting the first packet in a FEC block, to allow

   all the repair packets to arrive.

>>>

Not sure if the intent is SHOULD wait or should wait.

More importantly, the FEC patterns in the draft are all defined to have a
fixed number of packets.  The “time that spans an FEC block” is not
constant, especially for variable-rate video.  The receiver has no idea how
long that repair window time might be for a *specific *FEC block.

At some point the receiver decides it’s time to deliver the source packets
– either because

(a)    It is receiving source packets with no break in sequence number, so
there is no need to wait for repair

(b)    It has received enough repair packets to recover the missing source
packets

(c)     It has received all the repair packets, and they aren’t sufficient
to recover the missing source packets

(d)    It is choosing not to wait any longer.



Case (d) might be reached because it is starting to receive packets in the
next FEC block, or it might be reached because of a real-time constraint



The wording in the above paragraph doesn’t cover these cases very well.
And the FEC decoder doesn’t always need to wait for all the repair packets
to be received before it can begin the repair process.

It’s probably more useful to point out that using flexflec does add delay,
and it needs to be clear that the repair window (in time units) is
nominal.  Or maybe specify the repair window in packets?

>>>

Suppose that we have a group of D x L source packets that have

   sequence numbers starting from 1 running to D x L, and a repair

   packet is generated by applying the XOR operation to every L

   consecutive packets as sketched in Figure 3.  This process is

   referred to as 1-D non-interleaved FEC protection

>>>

I think the document would be clearer if this was in new section – preceded
by a short into saying how flexspec provides non-interleaved (row),
interleaved (column), and hybrid row+column (2-D) FEC patterns.  This is a
key concept in the draft, and should be more prominent.

I think there should also be four use-case sections.  Separate sections for
row and column, and the retransmission mode (“R” bit) should probably have
a short section too.

>>>

*1.1.3
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-1.1.3>**.
Overhead Computation*

>>>


Overhead isn’t used much in the document, and I am not sure this particular
definition adds much value.  It might be better phrased as “FEC Overhead
Considerations”, since the overhead fractions really don’t have much
practical use, especially when the source packets are of unequal size.


>>>  3.1
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-3.1>.
Definitions

>>>

FEC Block, and repair window should be defined here (neither are defined in
RFC6363).  Parity Codes are not defined in RFC6363 either, and probably
should be defined here.


1-D non-interleaved, 1-D interleaved, 2-D protection perhaps should be
defined.


It might be cleaner to define source block here, since the RFC6363
definition doesn’t precisely apply (because its definition of ADU flow
depends on 5-tuple).


In general there are very few definitions in RFC6363 that are used in
flexfec, so if it were my draft I’d just redefine them explicitly here.

>>>
3.2
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-3.2>.
Notations

>>>

I’m not sure that bitmask needs to be here, though the text is ok.




4.1
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-4.1>.
Source Packets

>>>

   The source packets MUST contain the information that identifies the

   source block and the position within the source block occupied by the

   packet.

>>>

This is a meaningless MUST, since the source RTP packet is simply sent
without modification.  The sequence number and SSRC are used to map the
packets onto the repair packets.

Overall, paragraph 4.1 is confusing, because it gives the impression that
something else needs to be done, and then says that there isn’t.  Plus, it
is supposed “define the format of the source packet” -  which it doesn’t
do.


I think it’s better just to say earlier in the document that the source
packets can be any RTP packets, and leave it at that.  Perhaps tie that
statement to the use of systematic codes.  Then focus on defining the
repair packet format, which is the heart of the draft.


The advantages of not modifying the source packets is better placed
somewhere else in the document – perhaps in section 1.1, when the FEC
encoder and Decoders are introduced.


4.2
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-4.2>.
Repair Packets



>>>

   o  Marker (M) Bit: This bit is not used for this payload type, and

      SHALL be set to 0

>>>

And ignored by receivers?

>>>

Note that this document

      registers new payload formats for the repair packets (Refer to

      Section 5
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-5>
for details).  According to [RFC3550
<https://tools.ietf.org/html/rfc3550>], an RTP receiver

      that cannot recognize a payload type must discard it.  This

      provides backward compatibility.  If a non-FEC-capable receiver

      receives a repair packet, it will not recognize the payload type,

      and hence, will discard the repair packet.

>>>

This probably should be said somewhere, but I think it’s better moved
somewhere other than the repair packet format description.



I believe this also requires that the CSRCs in the repair RTP header be set
to the SSRCs of the source packets.  That isn’t stated, but is implied by
the structure in table 10.


>>>

The format of the FEC header is shown in Figure 10.

The FEC header consists of the following fields:

>>>

No.  This is the first of four? possible formats, depending on R,F,M, and
N, shown in figures 10-15.  M is ambiguous - appears as M and M (columns).
Some fields are really repair symbols (P,X,C,M, PT Recovery, Length
Recovery, recovery), and are meaningless on their own.  This rather
important detail doesn’t show up until you get to 6.2.


Figure 15 also affects figure 9, since the unspecified “Repair symbols” in
figure 9 are replaced by the “retransmission payload” In figure 15.  The
contents of that “retransmission payload” are not specified anywhere in the
draft.


This whole section is a confusing mess, and needs to be restructured.  I
guess one approach is to change 6.2 to “Repair symbol construction”, and
explicitly refer to it for all fields in the FEC header that are XORed.
One way or another, you need to separate out the fields that specify the
FEC parameters from the embedded repair symbols mixed into the structure.



>>>

   o  The CSRC_i (32 bits) field describes the SSRC of the packets

      protected by this particular FEC packet.  If a FEC packet contains

      protects multiple SSRCs (indicated by the CSRC Count > 1), there

      will be multiple blocks of data containing the SN base and Mask

      fields.

>>>

Does this mean that a repair flow can protect multiple RTP sources?  If so,
this should be clearly stated earlier.  There might be other implications
of supporting this (do all the sources need to have the same CNAME???)


>>>

If M>0, N=0,  is Row FEC, and no column FEC will follow

               Hence, FEC = SN, SN+1, SN+2, ... , SN+(M-1), SN+M.



   If M>0, N=1,  is Row FEC, and column FEC will follow.

                 Hence, FEC = SN, SN+1, SN+2, ... , SN+(M-1), SN+M.

            and more to come



   If M>0, N>1,  indicates column FEC of every M packet

                    in a group of N packets starting at SN base.

                 Hence, FEC = SN+(Mx0), SN+(Mx1), ... , SN+(MxN).





             Figure 14: Interpreting the M and N field values

>>>

First, this is not a figure. Second, the names are not consistent with the
1-d non-interleaved, 1-d interleaved, and 2-d described above.  The names
need to be consistent.  FWIW, these names are a bit more intuitive to me.



>>>

Note that the parsing of this packet is different.

>>>

“this” is indefinite.  In general you should name the four(?) header types,
I think that will make things simpler.



>>>

Figure 15: Protocol format for Retransmission

>>>

This is mis-titled.  It also isn’t clear.  The figure shows the alternative
FEC header format, but I believe the intent is that the retransmission
payload replaces the “repair symbols” shown in figure 9.  That isn’t
obvious from the organization.


>>>

   It should be noted that a mask-based approach (similar to the ones

   specified in [RFC2733 <https://tools.ietf.org/html/rfc2733>] and [RFC5109
<https://tools.ietf.org/html/rfc5109>]) may not be very efficient to

   indicate which source packets in the current source block are

   associated with a given repair packet.  In particular, for the

   applications that would like to use large source block sizes, the

   size of the mask that is required to describe the source-repair

   packet associations may be prohibitively large.  The 8-bit fields

   proposed in [SMPTE2022-1
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#ref-SMPTE2022-1>]
indicate a systematized approach.  Instead

   the approach in this document uses the 8-bit fields to indicate

   packet offsets protected by the FEC packet.  The approach in

   [SMPTE2022-1
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#ref-SMPTE2022-1>]
is inherently more efficient for regular patterns, it

   does not provide flexibility to represent other protection patterns

   (e.g., staircase).

>>>

I have no idea why this note is here.  I’d delete it.  There's no point in
talking about all the roads not taken.



>>>
5
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-5>.
Payload Format Parameters

>>>

If no specific FEC code is specified

   in the subtype, then the FEC code defaults to the parity code defined

   in this specification.

>>>

Since there is no syntax to specify the FEC code, a receiver has no
way to know for certain that the parity code is being used.  I *guess*
that the statement in 5.2.1 *might* cover this



>>>

“There are no optional format parameters defined for this payload.

      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.

>>>

For SDP at least, any FEC code in the offer would be deleted by an
existing receiver, so if parity is mandatory-to-implement you’d get
interoperability at least.  But why not just specify the FEC code
parameter (with Parity as a choice) now?



>>>


6.2
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-6.2>.
Repair Packet Construction



   The RTP header of a repair packet is formed based on the guidelines

   given in Section 4.2
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-4.2>.

>>>

Since 4.2 is a mess, it’s not surprising that 6.2 is also.

The FEC header length isn’t limited to 28 octets in the retransmission case
(which is conveniently skipped in 6.2)

It would be good to specify what’s in the skipped bits in the header,
rather than making people figure it out. (“V-2” and Sequence Number)

Also phrases like “The next 4 bits of the FEC bit string are written
into the CC recovery field in the FEC header” should be written as
like “The next 4 bits of the FEC bit string are XORed into the CC
recovery field in the FEC header”



BTW, I don’t think the “FEC bit string” nomenclature is all that useful,
given that you end up specifying the XOR field by field anyway.



>>>

   The repair packet payload consists of the bits that are generated by

   applying the XOR operation on the payloads of the source RTP packets.

   If the payload lengths of the source packets are not equal, each

   shorter packet MUST be padded to the length of the longest packet by

   adding octet 0's at the end.

>>>

This presumably defines what is placed into the “repair symbols” back in
figure 9.  But it doesn’t say that.

I believe this is incomplete.  It doesn’t say how to handle extension
headers.  It isn’t clear whether padding octets are XORed (RFC 3550 says
the padding octets “are not part of the payload”). SRTP MKI and
authentication tag would also be recovered (which is not possible in the
current draft).



>>>
6.3.2
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-6.3.2>.
Recovering the RTP Header

This procedure recovers the header of an RTP packet up to (and

   including) the SSRC field.



>>>

But it doesn’t recover the full header, and that should be more clearly
stated,  FWIW, “if the repair symbols” were defined as running from the
first byte after the SSRC to the end of the source RTP packet, then the
full RTP packet would be recovered – including SRTP MKI and authentication
tag, which cannot be recovered now.



>>>
6.3.3
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-6.3.3>.
Recovering the RTP Payload



   2.  For each of the source packets that are successfully received in

       T, compute the bit string from the Y octets of data starting with

       the 13th octet of the packet.  If any of the bit strings

       generated from the source packets has a length shorter than Y,

       pad them to that length.  The padding of octet 0 MUST be added at

       the end of the bit string.  *Note that the information of the*

*       first 8 octets are protected by the FEC header*

>>>

As is the SSRC, though in a different way.

>>>

   2.  For each of the source packets that are successfully received in

       T, compute the bit string from the Y octets of data starting with

       the 13th octet of the packet.  If any of the bit strings

       generated from the source packets has a length shorter than Y,

       pad them to that length.  The padding of octet 0 MUST be added at

       the end of the bit string.  Note that the information of the

       first 8 octets are protected by the FEC header.



   3.  For the repair packet in T, compute the FEC bit string from the

       repair packet payload, i.e., the Y octets of data following the

       FEC header.  Note that the FEC header may be 12, 16, 32 octets

       depending on the length of the bitmask.





   4.  Calculate the recovered bit string as the XOR of the bit strings

       generated from all source packets in T and the FEC bit string

       generated from the repair packet in T.



   5.  Append the recovered bit string (Y octets) to the new packet

       generated in Section 6.3.2
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-6.3.2>
.

>>>

This is close to the algorithm I suggested above (starting the “Repair
symbols” in figure 9 from the first octet after the SSRC).

That’s good, but it isn’t consistent with the text in 6.2 (“The repair
packet payload consists of the bits that are generated by applying the
XOR operation on the *payloads of the source RTP packets)*.

So the text as written fails to recover (or fails to generate the
repair packet correctly, depending on how you look at it).

Also, any RTP padding, SRTP MKI + authentication tag in the repair
packets themselves shouldn’t be included in the XOR.

RTP Padding, RTP MKI and the authentication tag lengths that are part
of the source packets do need to be taken into account when computing
Y.  I'm not sure the current FEC header handles that well.


This section uses "FEC Bit String" to mean something different than it
meant earlier in the document.


Note the repair algorithms could include authentication of input
packets and recovered source packets.


>>>

8
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-8>.
Congestion Control Considerations



>>>

I’d remove the reference to[I-D.singh-rmcat-adaptive-fec
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#ref-I-D.singh-rmcat-adaptive-fec>].



>>>

However, it MAY still continue to use FEC if considered for bandwidth

   estimation instead of speculatively probe for additional capacity

   [Holmer13 <https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#ref-Holmer13>][Nagy14].

>>>

What does this sentence mean?  The draft doesn’t talk about using FEC for
either bandwidth estimation or capacity probing.  I suggest deleting it.



>>>
9
<https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#section-9>.
Security Considerations

>>>


Since the sender of the repair packets might not be the sender of the
source packets, there is an obvious threat of injection of bogus repair
packets.  Authentication can be helpful in mitigating that threat.

Such authentication (if possible) should be done an any packets that are
used in the recovery process, and also on the reconstructed output packets.


One could strengthen this into "MUST not" normative language if one wished.




NITS

Overall, the document frequently uses first person plural (Suppose we have,
If we apply, We generate,  We refer, We assume, …).  I think the authors
should consider rephrasing those sentences.





Section 1

>>>

Situations where adaptivitiy

   of FEC parameters is desired, the endpoint can use the in-band

   mechanism, whereas when the FEC parameters are fixed, the endpoint

   may prefer to negotiate them out-of-band.

>>>

The sentence uses incorrect grammar.  I suggest

The in-band mechanism is advantageous when the endpoint is adapting the FEC
parameters; the out-of-band mechanism may be preferable when the FEC
parameters are fixed.



Section 1.1

>>>

In a nutshell,

>>>

Informal English – I suggest rephrasing.



>>>

Section 8

However, it MAY still continue to use FEC if considered for bandwidth

   estimation instead of speculatively probe for additional capacity

   [Holmer13 <https://tools.ietf.org/html/draft-ietf-payload-flexible-fec-scheme-05#ref-Holmer13>][Nagy14].

>>>

Probing?









On Sun, Nov 19, 2017 at 4:07 AM, Ali C. Begen <[e-mail]>
wrote:

> Obviously the deadline for the WGLC is Dec. 7th.
>
> -acbegen
>
> > On Nov 17, 2017, at 5:35 AM, Roni Even <[e-mail]> wrote:
> >
> > Hi,
> > I would like to start a three week WGLC on RTP Payload Format for
> Flexible Forward Error Correction in draft-ietf-payload-flexible-
> fec-scheme-05.
> > The WGLC will end on November 7th
> >
> > Mo has posted some clarifications to the payload WG mailing list
> payload mailing list archives
> >
> > Please send comments to the payload mailing list.
> > THe double posting is to  notify RTCweb WG that the WGLC has started
> since this document is needed for RTCweb
> >
> >
> > Roni Even
> > Payload WG co-chair
> >
> >
> > _______________________________________________
> > payload mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/payload
>
> _______________________________________________
> payload mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/payload
>