Page MenuHomeFreeBSD

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

Authored by daniel.engberg.lists_pyret.net on Jul 22 2019, 8:03 AM.

Details

Reviewers
koobs
jbeich
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

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

koobs added a comment.Jul 23 2019, 4:50 AM

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?

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

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

Poudriere run

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

koobs added inline comments.Oct 6 2019, 11:45 PM
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

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

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