multimedia/x264: Add optimization and some minor clean-ups

[diizzy]

Summary

Enable Link-Time Optimization and OPTIMIZED_CFLAGS on aarch64, armv7 and amd64
Remove unneeded CONFIGURE_ARGS, it already picks up CFLAGS, CPPFLAGS and LDFLAGS
Strip -O2 when enabling OPTIMIZED_CFLAGS to avoid setting multiple -O FLAGS

Test Plan

Compile and Run-time tested on:
aarch64, Orange Pi PC2, 13.0-CURRENT
armv7, Orange Pi PC, 13.0-CURRENT
amd64, Dell T20 (Intel i7-3770), 13.0-CURRENT

[koobs]

Looks OK. Questions:

1. Does libx264 still pass QA (poudriere: build, link, stage, package) with the --extra_*flags removals?
2. What are the optimized cflags? I see no build changes (other than -02 removal) when the option is enabled?

[diizzy]

Hi,

Unfortunately I don't have a pouderie setup ready mainly because of lack of proper (fast) hardware but I'll give it a try although it might take a while (probably one or two weeks).
Just to make sure, you're referring to this? https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/testing-poudriere.html

I used OPTIMIZED_CLAGS as a toggle as (lib)x264 sets -O3 by default but is overridden by toolchain CFLAGS and from what I can tell it seems like a good choice of name looking at other ports but perhaps there's a better one?

Disabled

cc -Wshadow -O3 -ffast-math -m64 -O2 -pipe  -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing  -Wall -I. -I. -std=gnu99 -D_GNU_SOURCE -mstack-alignment=64 -fPIC -fomit-frame-pointer -flto -fno-tree-vectorize -c common/osdep.c -o common/osdep.o

Enabled

cc -Wshadow -O3 -ffast-math -m64 -pipe  -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing  -Wall -I. -I. -std=gnu99 -D_GNU_SOURCE -mstack-alignment=64 -fPIC -fomit-frame-pointer -flto -fno-tree-vectorize -c common/osdep.c -o common/osdep.o

[diizzy]

No poudriere build yet but make stage && make check-plist && make stage-qa && make package completes without any errors

[diizzy]

libx264-0.157.2980.log40 KBDownload

Poudriere run

[diizzy]

@koobs
Is there something else you want me to do or is this something we can commit?

[koobs]

multimedia/x264/Makefile

57–59

Can you provide a comment/explain why only these archs get these flags by default, or rather, why other archs dont

I'd like to provide that detail in the commit log message

67

If some decent number of ports use LTO as an option name, we could/should probably factor it out into a shared description in bsd.options.desc.mk

110

I'm pretty sure CONFIGURE_ARGS already contains this value by default from the framework. Can you check, and if so, remove this

133–135

OPTIMIZED_CFLAGS should probably 'append' to CFLAGS (such that it honours and extends user-CFLAGS), rather than replacing/removing system flags, or flags provided from somewhere else.

OPTIMIZED_CFLAGS_CFLAGS=-O3 or similar

[diizzy]

multimedia/x264/Makefile

57–59

Those are the platforms I could run-time test on (using real hardware), I'm not sure if it works as intended on platforms such as i386, MIPS, PPC etc.

67

I posted on the mailing list about it for input and to verify if it's even doable.
https://lists.freebsd.org/pipermail/freebsd-ports/2019-October/117081.html

110

You're correct, I'll remove it

133–135

I agree it's not ideal but I wanted to avoid potential confusing compiler output as libx264 sets -O3 by default. Another solution would be to strip default -O3 argument from sources, not sure which way to go.

Untouched
cc -Wshadow -O3 -ffast-math -m64 -O2 -pipe...

With CFLAGS appended
cc -Wshadow -O3 -ffast-math -m64 -O2 -pipe -O3...

[diizzy]

O3 makes very little difference new and the new versions of libx264 enables LTO by default (x264 build does not however).