[FFmpeg-devel] [PATCH v0] Implement promeg decoder.

[Kieran Kunhya]

On Fri, Jan 17, 2025 at 2:24 PM Kieran Kunhya <kieran618 at googlemail.com> wrote:
>
> On Fri, Jan 17, 2025 at 2:00 PM Romain Beauxis <romain.beauxis at gmail.com> wrote:
> >
> > Hi,
> >
> > Le jeu. 16 janv. 2025 à 16:40, Kieran Kunhya
> > <kieran618 at googlemail.com> a écrit :
> > >
> > > On Thu, Jan 16, 2025 at 9:35 PM Romain Beauxis <romain.beauxis at gmail.com> wrote:
> > > >
> > > > Le jeu. 16 janv. 2025 à 14:48, Kieran Kunhya
> > > > <kieran618 at googlemail.com> a écrit :
> > > > >
> > > > > On Thu, 16 Jan 2025, 20:15 Romain Beauxis, <romain.beauxis at gmail.com> wrote:
> > > > >>
> > > > >> This patch implements the decoding logic for the FEC error-
> > > > >> correction method described in the Pro-MPEG CoP #3-R2 FEC[1]
> > > > >>
> > > > >> We are still in the process of testing this patch with our encoders but
> > > > >> I wanted to send it now in case it could use some early review.
> > > > >>
> > > > >> The code seems clean enough and straight forward:
> > > > >
> > > > >
> > > > > As someone who wrote an FEC receiver in Upipe, I'm surprised to see the word "straight forward" and FEC used in the same sentence. The Upipe implementation (which was never merged) is extremely messy and complex and commercial implementations are even worse as I understand. There are tons of edge cases.
> > > >
> > > > Yes, you're right! What I meant by straight forward is that, in my
> > > > experience, in the absence of a specific heuristics, the most simple
> > > > and elegant solution is usually the best, at least to make sure the
> > > > code is easy to read and to improve on later.
> > >
> > > This is not academia. FEC is about maximising error recovery and there
> > > are tons of edge cases to create something that works 24/7.
> > >
> > > > I should have also said that I'm hoping that this implementation is an
> > > > initial one that provides the functionality for the most "simple"
> > > > situations and that could be improved later.
> > > >
> > > > > The biggest question I have, is how do you decide to schedule packets? In the FEC spec, the incoming streams are CBR so the matrix duration is fixed. So if there is packet loss you know when to schedule the output packets because the latency is fixed. But in FFmpeg you don't know the incoming streams are CBR, nor do you have a concept of time. Min and max packets is complete guesswork in a VBR stream and the code takes a bursty stream and makes it even more bursty. And you of course destroy the CBR nature of a valid FEC stream.
> > > > >
> > > > > The other thing that's difficult is handling a change in stream without causing an infinite loop and keeping FEC behaviour Heuristics are needed for this.
> > > > >
> > > > > It's not clear from your code how you handle the matrix recursion. i.e the fact you recovered one packet allows you to rerun the recovery and potentially recover more from a different column/row.
> > > >
> > > > The way the algorithm works is:
> > > > * Start by buffering an initial amount of packets
> > >
> > > But you have packet loss so this number represents a variable amount of time.
> > >
> > > > * Read all available FEC packets on each RTP packet read, keeping a
> > > > max number on each axis
> > > > * On first missing packet, keep reading additional RTP packets and:
> > > >   - Try to reconstruct using the packet row if available
> > > >   - Or else, try to reconstruct using the packet column if available
> > > >   - Or else, try to reconstruct using the whole FEC matrix if both are available
> > > >
> > > > In this matrix reconstruction phase, we keep looping on the rows and
> > > > columns of the matrix while at least one packet is reconstructed. We
> > > > also return immediately if the missing packet has been reconstructed.
> > > >
> > > > If the packet cannot be reconstructed, we keep reading packets and
> > > > trying this algorithm. When the max number of packets is reached, we
> > > > consider this packet unrecoverable.
> > >
> > > What about out of order packets?
> > >
> > > > If you adjust the max number of packets well, this makes sure that we
> > > > (optimistically) keep one FEC matrix forward from the packet.
> > > >
> > > > When a packet is returned, we look at the next packet's corresponding
> > > > FEC row and column packets. If we have both, we know exactly the index
> > > > of the first packet in the current packet's FEC matrix and we drop all
> > > > ones before that (RTP and FEC). Otherwise, we keep a whole column x
> > > > row matrix before the packet.
> > >
> > > So this basically bursts packets because you don't account for packet
> > > loss in your buffered packet sizes, nor CBR vs VBR.
> > > As I said, if you stop and start the encoder this implementation will
> > > almost certainly infinite loop and/or crash.
> > >
> > > > > I also question the need for FEC in 2025. I am happy to see this awful protocol gone and am not sure maintaining something like this is possible in FFmpeg apart from use cases where latency doesn't matter (so why use FEC?).
> > > >
> > > > The Radio France people have a real problem with packet loss in their
> > > > RTP streams.
> > > >
> > > > The rate of packet loss seems to be low enough so that this
> > > > implementation would work just fine.
> > >
> > > Yes but FFmpeg code needs to work in the real world, not just for your
> > > one customer.
> > >
> > > > Their hardware encoders have support for FEC encoding so this seems
> > > > like a real use and benefit right there.
> > >
> > > There are so much more modern technologies that are vastly easier to
> > > implement. We have learnt in the last twenty years.
> > >
> > > Have you tested this implementation with iptables or tc? You'll see
> > > it's very difficult to create a stable implementation that doesn't
> > > infinite loop as well as survives an encoder restart.
> > > I would suggest putting this on a real domestic internet connection
> > > where it's very hard to create a stable implementation.
> >
> > Hi,
> >
> > While I appreciate your feedback, I wonder about the need to be
> > combative like I perceive here.
>
> Because in the end writing something for your simple case and
> disappearing, FFmpeg will get the blame for things not being good.
> Same with FFmpeg supporting complex protocols WebRTC, SDR etc etc.
>
> >
> > At the end of the day, we are looking at a probabilistic problem
> > (connectivity issues).
> >
> > Just like the problem, the solution is probabilistic by nature. In
> > some cases, it will be able to recover some packets but. for some, it
> > will not.
> >
> > Even if it does not cover all corner cases, this implementation does help.
> >
> > I have tested it with stimulated network issues and found it to be
> > able to restore most packets event at 10 or 20% packet drops.
> >
> > I have also tested it with a bad local broadband.
> >
> > We plan on testing it more with a concrete problem and hardware encoders.
> >
> > In my experience, experienced software engineer thrive to propose dead
> > simple code that is good enough to solve the problems that it
> > addresses in most cases.
>
> Your code is honestly way too simplistic for an FEC implementation.
> There is a reason other implementations are much more complex.
> You have variable latency (user chooses the buffer size by hand,
> seriously...), create burstiness, likely infinite loop with an encoder
> restart etc.
>
> Writing a network protocol is about making it as tolerant to the real
> world as possible where all sorts of things happen.
>
> > Also, interfacing with legacy systems is at the core of what we do
> > because it does solve real world problems. This project has plenty of
> > support for legacy systems.
> >
> > I will address the encoder restart, thank you for pointing this out. I
> > imagine that the SSRC can be used to detect a new stream and
> > bump/reset packets indexes.
>
> SSRC is mandated to be 0 so it can't be used.
>
> Regards,
> Kieran Kunhya

I would seriously look at a sophisticated FEC implementation that
doesn't require the user to input the matrix size, has a low and
constant latency, handles reordered packets and is tolerant to encoder
restarts:
https://github.com/Upipe/upipe/blob/master/lib/upipe-ts/upipe_rtp_fec.c

And even then there are more complex approaches that do multiple
matrix recursions.

For an ancient protocol like FEC, nobody is going to work on it and so
FFmpeg's implementation will remain basic (that happens to work for
you).

Regards,
Kieran Kunhya