[FFmpeg-devel] [PATCH v15 00/16] *** SUBJECT HERE ***

[Hendrik Leppkes]

On Thu, Nov 25, 2021 at 1:12 PM Soft Works <softworkz at hotmail.com> wrote:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of James Almer
> > Sent: Thursday, November 25, 2021 11:52 AM
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v15 00/16] *** SUBJECT HERE ***
> >
> > Have all the concerns people had been addressed? I'm also certain the
> > design itself wasn't well received.
> > This is big and needs more than 24 hours or just one person LGTMing it, so
> > please wait.
>
> James, just a little note: After 16 version and many weeks, the window for
> architecture discussions is closed for me.
> I've been asking for comments often enough - and way more often than others
> usually do.
>
> Let alone small changes, but at this point it's a take-it-or-leave it decision:
>
>         Nice new features vs. nothing for another 10 years.
>
> PS: It's of course totally fine when somebody would speak up asking for some
> more time, but otherwise, there has been more than enough time by now.
>
>
> > Also, patch 15/16 breaks FATE. Even if 16/16 fixes the tests, it will make
> > bisecting annoying, so it needs to be green.
> >
>
> Yes, one commit is from fftools, the other one from avcodec. Squashing them would
> solve the problem - Shall I do this
>

I would be more curious about why does it break?
If we view fftools as a user of our libraries, shouldn't all the
library commits be neutral to fftools, and not introduce changes,
unless there is an API break in there, which is something we wanted to
avoid?

I have not checked what exactly breaks there, but its always a warning
sign when this happens.

Otherwise, instead of merging complex changes into one mega commit,
fate tests should probably be updated instead, even if its going to be
updated again in the next commit.

Speaking of mega commits, I think the first commit should be broken up
more. There is no reason that all this has to happen in one commit,
eg. introducing AVFrame structure changes, new frame helper functions,
decode support, could and (imho) should all be separate changes.
I'm pretty sure I have commented on this already a long time ago, too.

- Hendrik