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

[Nicolas George]

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.

> That might not prevent tools from wrongly detecting ABI breakage from
> headers.

Can you explain?

>	   The best way to have private fields is to make them private for
> real, so I would still prefer an opaque pointer, "leaking" zero
> information.

And making the corresponding code unreadable, as I just explained.
Rejected.

> 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.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161114/0ded479f/attachment.sig>