On Fri, 17 Jan 2025, 21:32 Romain Beauxis, <romain.beau...@gmail.com> wrote:
> Le ven. 17 janv. 2025 à 08:32, Kieran Kunhya > <kieran...@googlemail.com> a écrit : > > > > On Fri, Jan 17, 2025 at 2:24 PM Kieran Kunhya <kieran...@googlemail.com> > wrote: > > > > > > On Fri, Jan 17, 2025 at 2:00 PM Romain Beauxis < > romain.beau...@gmail.com> wrote: > > > > > > > > Hi, > > > > > > > > Le jeu. 16 janv. 2025 à 16:40, Kieran Kunhya > > > > <kieran...@googlemail.com> a écrit : > > > > > > > > > > On Thu, Jan 16, 2025 at 9:35 PM Romain Beauxis < > romain.beau...@gmail.com> wrote: > > > > > > > > > > > > Le jeu. 16 janv. 2025 à 14:48, Kieran Kunhya > > > > > > <kieran...@googlemail.com> a écrit : > > > > > > > > > > > > > > On Thu, 16 Jan 2025, 20:15 Romain Beauxis, < > romain.beau...@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 > > I'm just getting to look at that. > > Is the ffmpeg RTP input currently supposed to recover from an encoder > restart? > > I did: > * Start a RTP output: > > ffmpeg -i ... rtp://... > > * Start a RTP input reading from the input: > > ffmpeg -i rtp://... ... > > * Stop and start the RTP input process > > After that, the running ffmpeg instance keeps dropping packets: > > [rtp @ 0x14a804200] RTP: dropping old packet received too late0.215x > Last message repeated 7 times > > If ffmpeg RTP input is not able to recover from an encoder restart, I > don't think adding support for that in the FEC decoder will help. > > -- Romain > There you go, the only way to recover from real world issues is to use heuristics... Kieran > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
