[FFmpeg-devel] [PATCH] avformat: Add parityfec and ulpfec protocol

[Martin Storsjö]

Hi,

On Fri, 9 Apr 2021, Camille Gonnet wrote:

> Parityfec (RFC 2733) and ulpfec (RFC 5109) generic FEC encoding for RTP streams.
>
> Signed-off-by: Camille Gonnet <camille at sound4.biz>
> ---
> Changelog                 |   1 +
> doc/general_contents.texi |   1 +
> doc/protocols.texi        | 106 +++++
> libavformat/Makefile      |   1 +
> libavformat/fecrtp.c      | 884 ++++++++++++++++++++++++++++++++++++++
> libavformat/protocols.c   |   1 +
> libavformat/rtpproto.c    |   2 +-
> 7 files changed, 995 insertions(+), 1 deletion(-)
> create mode 100644 libavformat/fecrtp.c

I had a brief look at this... I'm not familiar with how fec protocols are 
hooked up and all that, but it does seem to follow the existing practice 
for the prompg protocol.

Stylistically, the code could adhere a bit more to the ffmpeg style (it 
misses space around operators in many places, random example:

+                mask_clear=1<<(s->span-1);  // This one is not in the xor
+                idx=s->last_idx+1;  // circular -> go first

The bitrev16/32 functions have incorrect indentation and odd spacing 
("return((x >> 8 ..."). I wonder if we have any existing function that do 
the same - as av_bswap* only swap bytes - maybe not...

The code uses a number of commented out av_log calls, which isn't 
stylistically great... As they already are added with a low enough debug 
level (AV_LOG_DEBUG) couldn't they be left in the code (so that they end 
up compiled and syntax checked)?

I didn't try following the whole algorithm, but I had a peek at some 
comments which I didn't quite understand, e.g. this one:

+    int size;           // size of the bitstring, so the bigger input packet size

Is that a typo in the comment, or just something I'd understand if I'd 
read the code more thoroughly?

// Martin