Bug 266575
| Summary: | audio/flac: Update to 1.4.1 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Daniel Engberg <diizzy> | ||||||||
| Component: | Individual Port(s) | Assignee: | Christian Weisgerber <naddy> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Only Me | CC: | diizzy, pkubaj | ||||||||
| Priority: | --- | Flags: | bugzilla:
maintainer-feedback?
(naddy) |
||||||||
| Version: | Latest | ||||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| URL: | https://github.com/xiph/flac/releases/tag/1.4.1 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Daniel Engberg
2022-09-24 06:39:56 UTC
pkubaj, can you give this a spin on ppc? Best regards, Daniel I haven't even tried it yet, but: you should disable LTO on riscv64 (doesn't build) and powerpc64 (builds fine, but all the LTO-ized binaries just crash). Created attachment 236785 [details]
Patch for flac v2
Only enable LTO on "safe" archs for now based on information provided by pkubaj@
Hm, that only allows LTO on aarch64, amd64 and i386 by default, but other architectures could also it. Why not simply enable everywhere but set OPTIONS_EXCLUDE_powerpc64 and OPTIONS_EXCLUDE_risc64? And also, since it just sets -flto, why add this option at all? A user could set the same in make.conf. Given the information you provided it might be broken on other platforms despite compiling which I'm unable to test so this seems like a better tradeoff. I'm not sure what you mean about make.conf, setting -flto globally will break things and how would you know what ports works and doesn't without a toggle? LTO definitely works on powerpc and powerpc64le.
AFAIK it also works on armv7.
Not sure about armv6 or powerpcspe.
Regarding make.conf, I meant something like:
.if ${.CURDIR:M*/audio/flac}
CFLAGS+= -flto
.endif
I know that you can use /etc/make.conf but that doesn't help the end user and there's no way to tell whether it works or not just by looking at a port. We should in that regard give top priority to provide working binaries which is why I'd like to limit it on platforms given the earlier discussion unless we can verify that they're working as intended. Well, I verified that it works on powerpc and powerpc64le. Created attachment 236813 [details]
Patch for flac v3
Enable LTO by default for powerpc and powerpc64le (tested by pkubaj)
(In reply to Piotr Kubaj from comment #8) Added, thanks! :-) A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=3c2fd0550b4992ff6a4de626f2154de3fb696937 commit 3c2fd0550b4992ff6a4de626f2154de3fb696937 Author: Christian Weisgerber <naddy@FreeBSD.org> AuthorDate: 2022-09-25 19:10:52 +0000 Commit: Christian Weisgerber <naddy@FreeBSD.org> CommitDate: 2022-09-25 20:12:21 +0000 audio/flac: update to 1.4.1 Many small changes: Changelog: https://www.xiph.org/flac/changelog.html PR: 266575 audio/flac/Makefile | 11 +- audio/flac/distinfo | 6 +- audio/flac/files/patch-configure (new) | 11 ++ audio/flac/files/patch-man_flac.1 (gone) | 11 -- audio/flac/pkg-plist | 270 ++++++++++++++++++++++++++++--- 5 files changed, 267 insertions(+), 42 deletions(-) (In reply to Daniel Engberg from comment #0) I disagree with all of your suggestions: - There is no reason for a GitHub master site. - I prefer autotools over cmake. - There is no consensus to remove static libraries. - LTO should be applied at the system level, or possibly used for selected ports. There is no reason to consider flac such a special port. Adding LTO to all ports individually does not scale. - SSE2 is detected at runtime on i386 and optimized SSE2 code is then used. The very misleading --enable-sse option just adds -msse2 to all compiled code. Again, this is something best done at the system level by those who want it. That said, I have updated the port to 1.4.1. - If we can avoid having a single point of failure (unfetchable port) it's for the better. downloads.xiph.org redirects to ftp.osuosl.org so it's just one host in the end.
- Projects are migrating away from GNU Autotools especially multimedia related ones so it makes sense trying to cater those as much as we can, it also in this case eliminates external patch files (please upstream if you intend to stick with GNU Autotools) and CMake builds much faster. About twice on my Intel 11th gen and aarch64 boxes tested with only shared libraries enabled (tested setup). Having that in mind I would like to think that it's better for everyone if we could avoid external patching and keep build times as short as possible overall.
amd64
14.07s real 22.92s user 5.81s sys
7.98s real 12.36s user 2.72s sys
aarch64
1m4.04s real 1m35.02s user 26.65s sys
36.99s real 1m33.22s user 12.28s sys
- Static libraries can be kept if needed (by ports) however we are also removing static libraries if they're not used in the tree so it's optional. pkg is also going to gain the functionality of letting users decide whether to install both variants (if available) at some point according to people hacking on it.
- I got about a 5+% increase in performance utilizing LTO which for a relatively demanding compute intensive library is a good "free" speed increase and we do utilize LTO per port bases in the tree because some ports breaks, doesn't improve performance or final binary size which may not be worth it compared to time it takes to compile. We also don't have time to evaluate each and every port.
- I admit that I misunderstand comments in the code about SSE2 as it seemed to imply that it enforced SSE2 instructions on i386.
Can you look into fixing make test so it also runs (completes) as root as it's ongoing work treewide?
Best regards,
Daniel
(In reply to Daniel Engberg from comment #13) Yes, I'll look into fixing the regression tests for root. I'll also pick up the eventual upstream fix so we don't lose x86 intrinsics optimizations with clang 14+. https://lists.freebsd.org/archives/freebsd-current/2022-September/002632.html A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=b6d3556bbb4b062ea3f922f13c04e93cce2a5f0a commit b6d3556bbb4b062ea3f922f13c04e93cce2a5f0a Author: Christian Weisgerber <naddy@FreeBSD.org> AuthorDate: 2022-10-10 13:33:21 +0000 Commit: Christian Weisgerber <naddy@FreeBSD.org> CommitDate: 2022-10-10 13:44:43 +0000 audio/flac: skip tests when building as root A few tests in the included test suite can intentionally not be run as root, e.g. test_libFLAC: "iterator claims file is writable when tester thinks it should not be" Skip the tests when building as root and provide a brief message. PR: 266575 Reported by: diizzy audio/flac/Makefile | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) Thanks! |