Nicolas George: > Andreas Rheinhardt (12023-01-31): >> Details please. > > I was not going to spend time explaining unless I had assurance it was > because the issue of requiring changes in the internal implementation > and developers habits just to hide some fields was taken into account. > Coming from somebody else, I would have expected the question to be only > to find the first excuses to shoot the proposal down. > > But you more or less found the same ideas I did anyway. > >> 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. > > Or use a “#include "avfilter_link_internal_fields.h" instead of a macro. > >> 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. > > Once and for all: If the solution requires changing things in the > internal implementation C files and changing the habits of the people > who work on these files (that goes together), then it is inferior to a > solution that does not require that. > > Your solution (a) has this flaw, plus relies on an extension that is > probably not available on all supported platforms (Microsoft I am > looking at you). >
It's called -fms-extensions for a reason. > My favor goes to (d). Probably with a stricter test for the (c) part > because the internal fields are not necessary for the filters and > therefore could be hidden there too. > This can be accomplished with a), too. All one has to do is use multiple levels of extensions. - Andreas PS: Upon rethinking, it is not only b) that contains undefined behaviour; it is b)-d) as well as the current code. The reason is that the type of AVFilterLink as seen in the files with FF_INTERNAL_FIELDS is not compatible with the type of AVFilterLink from the files without FF_INTERNAL_FIELDS. This also makes derived types, like the types of function declarations containing pointers to AVFilterLink incompatible and this is a violation of 6.2.7(2) of C11. I wonder if this will become a real problem with lto someday. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".