[FFmpeg-devel] [PATCH v2 1/2] swscale: Replace illegal vector keyword usage in altivec code

[Daniel Kolesa]

On Sun, Aug 11, 2019, at 21:37, Reimar Döffinger wrote:
> On 07.08.2019, at 19:39, Daniel Kolesa <daniel at octaforge.org> wrote:
> 
> > While this technically compiles in current ffmpeg, this is only
> > because ffmpeg is compiled in strict ISO C mode, which disables
> > the builtin 'vector' keyword for AltiVec/VSX. Instead this gets
> > replaced with a macro inside altivec.h, which defines vector to
> > be actually __vector, which accepts random types.
> > 
> > Normally, the vector keyword should be used only with plain
> > scalar non-typedef types, such as unsigned int. But we have the
> > vec_(s|u)(8|16|32) macros, which can be used in a portable manner,
> > in util_altivec.h in libavutil.
> > 
> > This is also consistent with other AltiVec/VSX code elsewhere in
> > the tree.
> 
> I see the consistency argument, but otherwise it really doesn't sound 
> convincing.
> I don't think the intend to support compiling with -std=gnu11 either 
> way, so not sure we should care about that.
> And not allowing typedef'd type seems utterly silly and against any 
> expected C behaviour that for simple maintenance reasons I don't think 
> we want this long-term.
> As far as I remember switching to __vector everywhere is not really an 
> option either though because some compilers do not accept that?
> I don't strictly object to the change, it just doesn't really feel like 
> a truly good path forward long-term.

The main reason really is the consistency, since these two files are the only altivec/vsx files in the tree using directly, every other file uses the utility macros. The utility macros are always defined in a legal manner, regardless of the compiler. Therefore, I don't see any reason not to use them here either, they're both shorter and more compatible.

Switching to __vector everywhere is not an option since it's a GNU extension. The vector-keyword-as-a-builtin is the original altivec extension, and that does not allow the typedef'd types, so right now the code should be considered incorrect, IMO.

Compiling with gnu11 works otherwise, mplayer does this for its vendored ffmpeg tree.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".