On 1/17/2025 11:24 AM, Kieran Kunhya via ffmpeg-devel 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 packetsBut 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.
Don't assume he will not extend the implementation. Ask him instead what he plans to do in the long term. And this could also be marked as experimental, in which case if abandoned or proven unstable for several real world cases, it can be removed or disabled.
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ 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".
