Add x265 4.1#2513
Conversation
amyspark
force-pushed
the
add-x265-4.1
branch
7 times, most recently
from
cca229f to
26bf622
Compare
nirbheek
left a comment
nirbheek
left a comment
There was a problem hiding this comment.
LGTM, more complete than most other wraps we have 🙂
| ], | ||
| ) | ||
|
|
||
| if get_option('optimization') != '3' |
There was a problem hiding this comment.
Why not also put this in default_options?
There was a problem hiding this comment.
It is a forbidden option:
Line 698 in eabe86c
(However, it's compulsory in order to run tests, so I instead made sure to disable them if the right optimization level isn't set.)
There was a problem hiding this comment.
it's not clear that wraps should have an opinion about [optimization defaults]
@bgilbert this is not true for many projects, and x265 is one of them. Upstream specifically wants you to only build with -O3, and will probably ignore bug reports for any other optimization level. I think we should allow wraps to override this sanity check.
| include_directories: include_dirs, | ||
| override_options: aarch64_override_opts, | ||
| build_by_default: false, | ||
| # gnu_symbol_visibility: 'inlineshidden', |
There was a problem hiding this comment.
Why is this commented out and at the wrong indentation, and repeated everywhere?
There was a problem hiding this comment.
x265, as well as other CMake based dependencies of GStreamer's, leaks all STL-generated inline C++ symbols. This causes duplicate symbol warnings when linking against the resulting static library (and, in the case of GStreamer's, when putting together the static framework for iOS deployment, causing GitLab CI to run out of job log space).
The fix, like I did for librsvg, is to:
- mark them as
inlineshidden(the line above), - and then export the right symbols with prelinking.
Meson supports this already, but when I tried this on macOS, the library breaks down because of Meson implicitly assuming ELF/GNU symbol semantics and ignoring the target architecture: mesonbuild/meson#15234
As for the formatting itself, I do not know; earlier CI runs errored out with "missing formatting", and running tools/format.py adjusted lines like this.
amyspark
force-pushed
the
add-x265-4.1
branch
from
26bf622 to
226b710
Compare
2 participants
Hi all,
Coming here from https://gitlab.freedesktop.org/gstreamer/cerbero/-/merge_requests/1962 to upstream the wrap I made for the x265 codec library.
The main objective of this port is to support SDR, HDR10(+) and HDR12 in a single build, instead of doing three separate builds + a libtool/ar merge as is done by us and e.g. Homebrew (https://github.com/Homebrew/homebrew-core/blob/de72d493fe5311696d04cc079405ca478e65848d/Formula/x/x265.rb#L37).
All feedback is appreciated!
cc @nirbheek