Re: [FFmpeg-devel] [PATCH] lavfi: get rid of FF_INTERNAL_FIELDS

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Nicolas George:
> Nicolas George (12023-01-31):
>>> * it prevents filterlink internals from being visible in a
>>>   public header, where they have no business being
>>> * it is a step towards hiding more of lavfi internals from public
>>>   headers
>>> * the same pattern is already and ever more widely used in the other
> 
> Note to the TC who will decide: I do not oppose the efforts mentioned in
> these two points (that are actually the same points twice), I only
> oppose this particular solution because of this drawback:
> 
>> * It requires the developers to remember which field is public and which
>>   field is private, which is not something relevant here (is is relevant
>>   elsewhere).
> 
> Without looking very far, I can think of several different ways of
> hiding the internal fields better without requiring changes to the
> implementation. I would not oppose such a change.
> 

Details please. I can only think of the following:
a) Allow the use of -fms-extensions. This allows structs with tags and
typedefs thereof to be used as unnamed struct members with the members
of the unnamed structure being treated as members of the enclosing
structure. One could then use a pointer to the big internal structure
internally and one does not need to remember whether a field is internal
or not. There is still a problem, though: One needs to cast from
AVFilterContext.(inputs|outputs). This should be done via dedicated
inlined getters, but this is a bit more typing. E.g.
"input_from_ctx(ctx, i)" instead of "ctx->inputs[i]". Of course, it
might also be shorter if someone has a short name.
GCC, Clang, MSVC and the IIRC the intel compilers support this.

b) Add a big #define AVFILTERLINK in avfilter.h that expands to the
public part of AVFilterLink and change the declaration of AVFilterLink
to "struct AVFilterLink { AVFILTERLINK };" and use declare the internal
struct via
"struct FilterLinkInternal {
    AVFILTERLINK
    /* actual internal fields */
};"
This has the downside of actually being an aliasing violation and of
adding considerable ugliness to avfilter.h, in particular during
deprecations (like with FF_API_OLD_CHANNEL_LAYOUT -- you can't check via
#if in the implementation of a macro). I also don't know whether it
plays nicely with tools that deal with source code annotations.
c) Wrap the internal part in an #ifdef HAVE_AV_CONFIG_H, optionally
using #if defined(HAVE_AV_CONFIG_H) && defined(BUILDING_avfilter).
d) Same as c), but strip this stuff from installed headers.

I consider b)-d) as inferior to a), which I consider superior to Anton's
proposal, but the big drawback is its reliance on a compiler extension.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".