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

[Romain Beauxis]

Le mer. 22 janv. 2025 à 11:03, Kieran Kunhya
<kieran618 at googlemail.com> a écrit :
>
> On Thu, Jan 16, 2025 at 8:15 PM 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:
> > * The decoder stores packet using their index.
> > * Packets relevant to the decoding of the current packet are kept,
> >   using the received row and column FEC packets to determine the FEC
> >   matrix within with the current packet belongs to.
> > * Beside that, the decoder keeps a maximum number of packets and FEC
> >   packets.
>
> I don't see any evidence of wraparound handling unless I miss
> something. An FEC matrix can span a wraparound.

Got it in my local branch, working on testing/cleanup.

> Do you plan to support a fixed duration? Buffering N packets has a
> variable duration when there is packet loss.

As far as I can see there is a separation of concern here.

The FEC decoder works on the demuxer/transport layer and is
independent from the content layer.

The FEC decoder parameters can be set by the user according to their
content settings to determine the delay incurred by buffering and
packet loss for a CBR content.

The scope of this patch is limited to the FEC spec which is explicitly
defined for CBR content.

> Do you plan to support calculating the matrix size from the RTP FEC packets?
> Do you support block and non-block aligned permutations?

Those two could be done together and shouldn't be too hard to add to
the current implementation.

I'll look into it and would like to have it in the next version of the patch.

> Also saying you don't support an encoder restart is basically saying
> you want to write something that only deals with steady state
> behaviour.

As I replied before, this is just not something that ffmpeg can
support as a whole at the moment.

Do you have any other concerns?

The other change that I forgot to apply is the copyright headers
cleanup. I'll have it in the next iteration as well.

This process is not clear to me, am I supposed to work with you on
getting this patch accepted?

I do appreciate the feedback when they apply but I'd like to make sure
that I'm working toward a mutually agreed goal here.

Thanks,
-- Romain