[FFmpeg-devel] [PATCH 2/2] lavfi: make filter_frame non-recursive.

[Clément Bœsch]

On Mon, Nov 14, 2016 at 04:52:01PM +0100, Nicolas George wrote:
> Le quartidi 24 brumaire, an CCXXV, Clement Boesch a écrit :
> > I don't see how. You add a pointer at the beginning of your function
> > pointing on that internal structure and use that.
> 
> That makes extra code. Remember we are talking about lavfi: we
> frequently have inlink1, inlink2, outlink. If we must have inlink1priv,
> inlink2priv, outlinkpriv on top of that, that is starting to make a lot
> of namespace pollution.
> 
> Also, good luck keeping the variables names in sync. Someone will rename
> link to inlink someday, but not the corresponding internal variable, and
> the code will become an unreadable mess because variable that mean the
> same thing do not have a similar name.
> 
> Plus, even with a single link, whether we access it using the
> indirection explicitly "link->internal->x" or with a dedicated variable
> "linkpriv->x", we still have to remember, for every damn bloody field,
> whether it is public or private.
> 
> For codec/filter private context, there is a rather simple rule: if it
> is specific to the filter, it is private, if it is common to all filters
> it is in the common context. But it is still pretty annoying to have two
> variables and to must decide every time which one to use. And I waste a
> non-negligible amount of time getting it wrong somehow (for example
> after a copy-paste) and fixing it.
> 
> For private/public fields, the rule is much more ambiguous, a lot of
> fields are visible but could be internal, or are in the public structure
> below the "/*private*/" line. If we were to move a field from visible to
> internal, or the other way around, we would have to fix all the code
> that uses it.
> 
> Sorry, this is bad design, I refuse to have that in my code. I want to
> write link->fifo the same way I write link->src or link->time_base. You
> can reject my proposals, sure, but I can reject any proposal that do not
> verify this property.
> 

You could also make a local copy of the fields you need; it is -in my
opinion- not that much noise to add one or a few more lines of
declaration.

> > That might not prevent tools from wrongly detecting ABI breakage from
> > headers.
> 
> Can you explain?
> 

Some tool parsing the public header for analyzing structure layout changes
in public structure to detect ABI and API breakage would not be able to
make a difference in which fields are public or private.

BTW, it also means you're showing all the internals and their doc to the
users, which is a lot of noise for them.

> > It's not an emotional argument, it's a political one. "look at the shit
> > FFmpeg developers still do" does not help the project from getting
> > contributors/contributions.
> 
> Or maybe it help the project not getting bad contributors.
> 

Then I guess I'm a bad contributor.

Your private ifdefery suggestion is probably the least worst, but I still
prefer the common paradigm for previously said reasons that IMO still
stand.

Anyway, I have your points and you have mine, and my care security fuse
has blown, so I'm out of the debate.

Regards,

-- 
Clément B.