The build config menu contains the following checkboxes: │ │ [x] HI10P Enable HI10P Support (64-bit only) │ │ │ │ [x] HI12P Enable HI12P Support (64-bit only) │ │ This implies additional profiles, but as it stands, only the highest is built (so chosing 12 means 8 and 10 aren't built). Example code to enable multi-profiles is in ....multimedia/x265/work/x265_3.0/build/libux/multilib.sh As per https://x265.readthedocs.io/en/default/api.html#packaging-and-distribution : "We recommend that packagers distribute a single combined shared/static library build which includes all the bit depth libraries linked together. See the multilib scripts in our build/ subdirectories for examples of how to affect these combined library builds. It is the packager’s discretion which bit-depth exports the public C functions and thus becomes the default bit-depth for the combined library." Cheers, Jamie
Or, if that seems like overkill, may I suggest changing the options to an 8/10/12 bit radio-button selection? Cheers
Thanks for pointing this out, Jamie. If you can make a patch for the port, I can transfer maintainership to you as well...
I'm also trying to sort this out, but have no idea how to craft a ports Makefile to do what I think is expected. Is there documentation which would help with understanding its structure? From build/linux/multilib.sh in the x265 source archive it builds individual 8-, 10- and 12-bit binaries when selected via configuration then bundles them together using libtool. Another example is the homebrew build formula: https://github.com/Homebrew/homebrew-core/blob/master/Formula/x265.rb
Ahh.. Yeah, I've been meaning to do this, but real life keeps getting in the way... Give me a few days. Watch this space!
Pending the bigger change (see below), changing the options to an OPTIONS_SINGLE group seems the most desirable short-term change proposal to resolve the issue "as reported", given the current behaviour that only the highest is actually built, which is not expected by user. We can resolve this issue with that change, after creating a new/separate issue to "Add multiple-bit-depth profile linking' mechanism/feature to the port if someone would like to take that on.
I'm a complete novice regarding this, but I tried to manually bundle the 3x binaries using libtool as per the Linux examples, but it appeared to not perform this function as expected. Is there any documentation, or a similar existing example, I could refer to which would help explain how this is expected to be performed for this environment?
I have somewhat of a working Makefile but I'm not sure how to handle install-do section. https://projects.pyret.net/files/public/freebsd/H265-WIP-Makefile.txt Also, this might be a horrible idea so suggestions are welcome. I don't see the harm in providing 8,10,12 support by default as that seems to be what other distributions do. Test needs to be fixed but I'll tackle that later. Suggestions are welcome!
Created attachment 210333 [details] Patch for x265 Fixes following issues: Only require NASM on i386 and amd64 No need to pass the same switches using both CFLAGS and CPPFLAGS Compile 8,10,12-bit support unconditionally on 64-bit platforms Compile and run-tested on aarch64 and amd64 Supersides PR#239510
Created attachment 210334 [details] Poudriere log
(In reply to daniel.engberg.lists from comment #8) Apologies for not having the time to get back to this as I promised, but brilliant job sorting it out! I haven't tried running it yet, but it looks good. As for your original question about whether to install 8, 10, 12 by default. Originally, I was going to make each one optional - installing the 3 basically triples compile time / library disk usage. However, when the coffee finally kicked in, I realised that this will only be installed for encoding x265. Any machine that has problems with a slightly bigger library, and the extended compile time won't be suitable for x265 encoding! Also, it will make support, and the binary package easier to maintain bundling the whole lot together. My only slight concern is about changed behavior. Before your patch, as the highest quality profile selected was the only one installed, it therefore was used as the default profile. With your patch, the default encoding will be the 8bit profile. My personal feeling is that again, for support and maintainance, this is the correct approach to follow. However anyone who previously used a high profile at default - without specifying it specifically in their configuration - will now have their code changed to use the 8 bit profile. They will need to alter their config to explicitly select the 10bit/12bit profiles as appropriate. I'm just wondering whether this (admitedly probably rare) situation should be warned about when a user upgrades - either via a pkg-message or an entry in ports/UPDATING? Cheers again! Jamie P.S. Mikhail offered ownership of this port to me - I think instead you should take it, if that's ok with you!
(In reply to Wade from comment #3) Hey Wade. Apologies for the delay in replying. It's good that you attempted this, so I hope you don't give up in future! There is quite detailed information on building ports - including the Makefile - in the porters handbook : https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/ Also, the mailing lists and the forum https://forums.freebsd.org/ can be used for further help. Cheers, Jamie
Hi, Splitting it up just makes it unnecessarily complicated for no obvious benefit. I just made this patch for a small project and it seems to work fine on my end. I think it's a better idea for you to maintain this port if you're interested as I'm only using x265 a few times a year at best. As far as the changed behaviour goes I don't think it's needed as it's clearly stated what settings are being used during encoding.
(In reply to daniel.engberg.lists from comment #12) Ah ok, I didn't remember that it showed the settings.. I've rarely used x265 encoding myself up to now! Anyway, I've applied your patch, and it installed without issue, and I've tested it in operation, coding at various profiles, and it all works perfectly. Mikhail, I'd recommend Daniels patch as it stands. If you'd also still like to make me the maintainer, feel free to do so, if no-one else wants it. Cheers, Jamie
Created attachment 210557 [details] Patch for x265 Maintainer updated updated to Jamie and minor style fix
Created attachment 210581 [details] Support multiple pixel-widths Gentlemen, it occurred to me, we can fix the original problem without changing the promised behavior. The high-bit options can even be ON by default -- if we don't care for different arches having different functionality -- but it should still be possible to turn them off, I think. Could you review this new patch, please? It adds the build of the higher-resolution modes, while still keeping them optional. It also updates the pkg-plist... I would've committed this already, except, when I compile with -march=sandybridge, the test-executable crashes halfway through the run. Compiling with -march=core2 tests fine -- and I'd like to spend some more time investigating the crash. Another remaining item is to enable to the higher-bit options for other 64-bit systems (besides amd64). How can this be done elegantly -- and who can test the builds?
Hi, this doesn't work as intended and you shouldn't have multiple libraries. x265 [info]: build info [Unk-OS][clang 9.0.1][64 bit] 8bit This is with both 10 and 12-bit selected. Does the binary using the previous patch also crash on your end?
(In reply to daniel.engberg.lists from comment #16) > Hi, this doesn't work as intended and you shouldn't have multiple libraries. Uhm, sorry, what? > x265 [info]: build info [Unk-OS][clang 9.0.1][64 bit] 8bit > This is with both 10 and 12-bit selected. How did you get the above output? > Does the binary using the previous patch also crash on your end? No... I filed the https://bitbucket.org/multicoreware/x265/issues/531/crash-in-x265_pelfilterlumastrong_h_sse4
(In reply to Mikhail Teterin from comment #15) Hi Mikhail! The "promised behavior" was never correct. It implied 8 bit build, along with optional 10bit and 12bit. It never stated what the default bitdepth would be if not specified on the command line. As it stood, only the highest bitdepth selected was actually build, and hence, it became the defacto default. Daniels patch does indeed build all bitdepths into one library. The default bitdepth will now be 8bit, with 10 and 12 bit optional on the x265 command line. I agree with Daniel that this is the best solution - if you want a higher bit depth, it's easily selectable. (I think the bitdepth should be specified regardless anyway..) To emulate the previous behaviour you'd need to have a compile time option to configure it at build time within the ports Makefile. I think this is overkill, and will make support/pkg harder to manage. I did suggest mentioning that this may make an unexpected change for some use cases, but as Daniel pointed out, it's clear at use time what bit depth is being used. I'm not sure where the extra libraries and include files comes from - Daniels patch simply bundles the whole lot into one (ie. the plist doesn't change) As for the output, use "x265 --version". This is what I get with Daniels patch applied: % x265 --version x265 [info]: HEVC encoder version 3.2.1+1-b5c86a64bbbe x265 [info]: build info [Unk-OS][clang 7.0.1][64 bit] 8bit+10bit+12bit x265 [info]: using cpu capabilities: MMX2 SSE2Fast LZCNT SSSE3 SSE4.2 AVX FMA3 BMI2 AVX2 , and a quick test encode of all three profiles worked, producing different binaries, that ffprobe showed had the correct bit depth selected. As for the architecture issues, I must admit I did noting other than see if it compiled on my box, which is Freebsd12 with the following flags: 04:09 (7) "/tmp" jamie@thompson% show-clang-native-features Clang "native" target for this machine: skylake (Triple: x86_64-unknown-freebsd12.0) Features enabled (+) : sse2, cx16, sahf, prfchw, bmi2, fsgsbase, xsavec, popcnt, aes, xsaves, mmx, rdseed, clflushopt, xsave, invpcid, avx, fma, bmi, rdrnd, sse4.1, sse4.2, avx2, sse, lzcnt, pclmul, f16c, ssse3, sgx, cmov, movbe, xsaveopt, adx, sse3 Features disabled (-) : tbm, avx512ifma, sha, gfni, fma4, vpclmulqdq, cldemote, ptwrite, avx512bitalg, movdiri, avx512er, avx512vnni, avx512vpopcntdq, pconfig, clwb, avx512f, clzero, pku, lwp, rdpid, xop, waitpkg, movdir64b, sse4a, avx512bw, avx512vbmi2, avx512vl, avx512cd, vaes, rtm, mwaitx, wbnoinvd, prefetchwt1, shstk, avx512vbmi, avx512dq, avx512pf 04:09 (8) "/tmp" jamie@thompson% cc + cc -Wall -std=gnu99 -O2 -pipe -march=native -mtune=native -ftree-vectorize -fstack-protector-strong Hope this helps! cheers!
Hi, I gave this a go on 11.3 which is the latest and only supported release version of 11.X according to https://www.freebsd.org/security/ (you're listing 11.2 in your bug report to upstream). Using my patch "make test" passes using both O2 and O3 (OPTIMIZED_CLAGS) setting CPUTYPE?=sandybridge in /etc/make.conf which results in -march=sandybridge when building x265. This is on a fresh 11.3 install (AMD64) with a newly pulled ports tree using portsnap. $ clang --version FreeBSD clang version 8.0.0 (tags/RELEASE_800/final 356365) (based on LLVM 8.0.0) Target: x86_64-unknown-freebsd11.3 Thread model: posix InstalledDir: /usr/bin $ nasm --version NASM version 2.14.02 compiled on Jan 18 2020 $ x265 --version x265 [info]: HEVC encoder version 3.2.1+1-b5c86a64bbbe x265 [info]: build info [Unk-OS][clang 8.0.0][64 bit] 8bit+10bit+12bit x265 [info]: using cpu capabilities: MMX2 SSE2Fast SSSE3 SSE4.2 AVX I've tested this on VM with the host running an Ivy bridge CPU as I don't have a Sandybox based machine available. Can you please retest using 11.3 as it would be nice to get this in tree in a fully working state.
Created attachment 210907 [details] Bundle multiple pixel-widths > The "promised behavior" was never correct. It implied 8 bit build, > along with optional 10bit and 12bit. The promise was correct -- we just never delivered on it. The attached version always builds the 8bit version, plus 10- and/or 12-bit versions if requested. The libhdr10plus is also always built now. When debugging is enabled, nasm is invoked with "-g -O0" as well. Things yet to be determined: 1. On my machine the test executable still crashes as discussed earlier. 2. Should the highest bit-width be the default -- to match the current state of affairs? 3. Should it be possible to build ONLY one or both high-bit versions -- without the 8-bit variant?
I think we should drop the option to select bit depths as I misunderstood how the libraries where built (I assumed that they here tacked on) so the options shouldn't have been introduced at all. In addition to that no one else does it looking at a few distros like Arch, Alpine, Debian, Gentoo, OpenBSD if they even bother with multilib at all and I'd imagine that it could potentially break compatibility with sme users depending on libx265. 8-bit should always be the default and that's the bit depth devices for sure are to support if they understand H.265 aka HEVC. Are you still on 11.2 or did you upgrade to 11.3? That may fix the issue looking at the results from my VM. While your Makefile works it's not very clear what it does, multiple variables are being overridden which also makes it hard to follow and I'm not sure why you're building HDR as a separate lib which upstream doesn't seem recommend. I also not sure if kinda(?) shoehorning stuff into pre-build is a good idea. Looking at a few other distros: Alpine: Doesn't build anything other than stock 8-bit https://git.alpinelinux.org/aports/tree/main/x265/APKBUILD Arch: Uses upstreams multilib script more or less (only builds x86_64) https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/x265 Debian: Seems to build multilib on all arches as far as I can tell, doesn't include HDR support at all however. https://salsa.debian.org/multimedia-team/x265/blob/master/debian/rules Gentoo: Provides multiple variants including multilib, no HDR however https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/x265/x265-3.2.1.ebuild OpenBSD: Doesn't build anything other than stock 8-bit http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/multimedia/x265/Makefile?rev=1.45&content-type=text/x-cvsweb-markup NetBSD: Same as OpenBSD http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/multimedia/x265/Makefile?rev=1.28&content-type=text/x-cvsweb-markup Slack: Uses upstreams multilib script more or less (only builds x86_64) no HDR support however https://slackbuilds.org/slackbuilds/14.2/multimedia/x265/x265.SlackBuild
(In reply to Mikhail Teterin from comment #20) As Daniel says, I think 8bit should be the default, AND should always be included (it seems to be what everyone else expects, and it's the base bit rate guaranteed to be supported.) If you deem the potential change in default to be disruptive, a warning could be provided, rather than continuing to support an uncommon default which I doubt anyone has come to rely on anyway. The possible options would therefore be: 8 8,10 8,10,12 8,12 No-one seems to have just "8 and 10".. "8 and 12" seems pointless, as as far as I can tell, if anyone wants to use extended bit width they'd be currently targetting 10. So, I think if you have options at all, the options would be just "8 bit" or "all 3" (multilib) However, wouldn't just using multilib make things much easier moving forward? I can't see any reason for the options. The only downsize to multilib would be increase disk space use and compile time, but remember, this is a CPU intensive codec which will never be used on small or embedded systems. The differences to me therefore seem negligible. As for HDR, I have no knowledge there. I remember HDR wasn't initially enabled by default a few year ago because it was a bit problematic, but I thought that was to do with HDR processing in x265 in general and not related specifically to exposing the interface to 3rd party apps.
8 bit is absolutely required because it's the standard bit depth for any common video of decent quality for some time. 10 bit might be optional for most x265 users but is required by some. That divide between optional and mandatory for x265 is shifting towards mandatory. 8 and 10 bit decoders are both very commonly found in mid-level and up devices including mobile. 12 bit is very rarely used IME and that is unlikely to change anytime in the foreseeable near-term, but none of us can see all ends. Perhaps that too will change. Dolby Vision and other contenders may be able to make it useful. HDR/HDR10 is similar to 10 bit in that it is optional for some users as it requires full chain(encoder, player and display) support, but mandatory for others if possible. I have a whole stack of 4k movies waiting to be encoded into my VOD collection and when I do so, the encoder **will** have HDR* support. If not the FreeBSD port, then something else will have to do. All that being said, I think all 3 profiles should be installed by default. Most people encoding videos have multiple needs and the needs grow, not shrink. I'm guessing the extra compile time and disk space is going to be quite negligible to anyone taking on HEVC encoding. So it seems to me to be only a matter of port complexity and the implementation.
(In reply to daniel.engberg.lists from comment #21) > 8-bit should always be the default and that's the bit depth devices > for sure are to support if they understand H.265 aka HEVC. This seems to be the common sentiment, and I'd agree except for the legacy of situation -- our _package_ has always been 8bit-only, but the _port_ would use the different depth if enabled -- this is, what Jamie was pointing out in comment #10. One could say, that people building from source are smart enough to change their settings, except I for one am not such a person myself... I'd fire off "portupgrade" and expect things to "just work" -- and would be upset, if suddenly my program starts behaving differently: if I had, say, 10bit-width enabled, I'd expect it to continue to remain the default... Maybe, that's unavoidable -- because the package has always defaulted to 8bit (only) and should remain 8bit by default even if other widths will be enabled too from now on. > While your Makefile works it's not very clear what it does My method allows the main part to be built as a "vanilla" cmake-using port -- without overwriting the do-build and the do-install targets. I think, that's a win. I also avoid the cd-ing and mv-ing things around, but that's less important. > why you're building HDR as a separate lib which upstream doesn't > seem recommend Because _your own_ x265-v1.patch did :-) (ENABLE_HDR10_PLUS=true). You just weren't installing the result, because you had your own do-install... Now, maybe, that can be a separate option -- but given the general spirit of this discussion, I figured I'll just include it unconditionally. > Are you still on 11.2 or did you upgrade to 11.3? I am on 11.3, actually -- I made a mistake filing that bug-report. A mistake I cannot (easily) correct, because the report was filed anonymously. I doubt, upstream developers would use a FreeBSD-machine, though. But they may be able to find the same processor and clang/nasm versions... > I think we should drop the option to select bit depths Considering the effort already spent on developing support for the options, I'd like to keep it. Whoever was happy building the port with just one bit-depth, should be able to continue to have that, even if the default package will include all three. (In reply to Jamie Landeg-Jones from comment #22) > 8bit should be the default, AND should always be included > (it seems to be what everyone else expects Everyone _except_ FreeBSD-users -- who, until the port's inception -- have used exactly one bit-depth... I'm leaning towards preserving that ability -- while adding the ability to include multiple depths. (In reply to amvandemore from comment #23) > 8 bit is absolutely required because it's the standard bit depth > for any common video of decent quality for some time That's an argument for including 8 bit by default (building a package). But someone configuring the port's options should, in my opinion, be able to turn the 8bit-width off, if they have no use for it for whatever reason... Thank you very much, gentlemen. I really do appreciate both the effort you put into this ticket and your opinions...
Fair points. One thing which may address your concerns - it's possible to set the default to whichever you want. If you wanted to, you could bundle all bit widths, and use 8 bit as default (preserving how pkg has behaved) but use OPTIONS to change the default - Remember, those using 10 bit will have already set that in the OPTIONS - so if in the Makefile, instead of changing the OPTIONS field, you reuse the same field (i.e. the internal variable name) , but instead now use it for determining the default, then it should preserve previous behavior for all FreeBSD users of ports and pkg, unless I'm missing something! As an aside, I don't think I should maintain the port. It's obvious that everyone in this thread has more of an interest in it that I do. I'll still take it on if you really want to, but it deserves to be in much more capable hands. Cheers!
(In reply to Mikhail Teterin from comment #24) Yeah, I realized it right after posting... (regarding the extra library) ;-) If you insist on keeping the option(s) do so but enable all by default on 64-bit platforms. Also, mat@ suggested that $b should be ${b}. Other than that I noticed that the latest patch adds 10-bit in front of 12-bit, that may be a potential issue?
Created attachment 211039 [details] Bit-widths as multiple choice How about this version, gentlemen? > mat@ suggested that $b should be ${b}. Eew -- make each variable-use twice wider? To what end? There is nothing in style.Makefile(5) about it, so I'll stick to my own preference.
A commit references this bug: Author: mi Date: Wed Feb 5 04:40:24 UTC 2020 New revision: 525259 URL: https://svnweb.freebsd.org/changeset/ports/525259 Log: Reintroduce the recent upgrade after fixing the generation of the shared libraries. Actually list the PR properly. PR: 238773 Not reported by: antoine Changes: head/multimedia/x265/Makefile head/multimedia/x265/pkg-plist
(In reply to commit-hook from comment #28) Build fine on 12.1, but fails on 11.3: [90/98] : && /usr/bin/c++ -O2 -pipe -march=core2 -DNDEBUG -O3 -DLINKED_10BIT -DLINKED_12BIT -fstack-protector-strong -fno-strict-aliasing -O2 -pipe -march=core2 -DNDEBUG -O3 -DLINKED_10BIT -DLINKED_12BIT -fstack-protector-strong -fno-strict-aliasing -fstack-protector-strong /opt/obj/usr/ports/multimedia/x265/work/x265_3.2.1/10bit/libx265.a /opt/obj/usr/ports/multimedia/x265/work/x265_3.2.1/12bit/libx265.a -Wl,-Bsymbolic,-znoexecstack CMakeFiles/cli.dir/input/input.cpp.o CMakeFiles/cli.dir/input/y4m.cpp.o CMakeFiles/cli.dir/input/yuv.cpp.o CMakeFiles/cli.dir/output/output.cpp.o CMakeFiles/cli.dir/output/raw.cpp.o CMakeFiles/cli.dir/output/reconplay.cpp.o CMakeFiles/cli.dir/output/y4m.cpp.o CMakeFiles/cli.dir/output/yuv.cpp.o CMakeFiles/cli.dir/x265.cpp.o -o x265 -Wl,-rpath,/opt/obj/usr/ports/multimedia/x265/work/.build: libx265.so.179 -lpthread -lrt -ldl && : FAILED: x265 : && /usr/bin/c++ -O2 -pipe -march=core2 -DNDEBUG -O3 -DLINKED_10BIT -DLINKED_12BIT -fstack-protector-strong -fno-strict-aliasing -O2 -pipe -march=core2 -DNDEBUG -O3 -DLINKED_10BIT -DLINKED_12BIT -fstack-protector-strong -fno-strict-aliasing -fstack-protector-strong /opt/obj/usr/ports/multimedia/x265/work/x265_3.2.1/10bit/libx265.a /opt/obj/usr/ports/multimedia/x265/work/x265_3.2.1/12bit/libx265.a -Wl,-Bsymbolic,-znoexecstack CMakeFiles/cli.dir/input/input.cpp.o CMakeFiles/cli.dir/input/y4m.cpp.o CMakeFiles/cli.dir/input/yuv.cpp.o CMakeFiles/cli.dir/output/output.cpp.o CMakeFiles/cli.dir/output/raw.cpp.o CMakeFiles/cli.dir/output/reconplay.cpp.o CMakeFiles/cli.dir/output/y4m.cpp.o CMakeFiles/cli.dir/output/yuv.cpp.o CMakeFiles/cli.dir/x265.cpp.o -o x265 -Wl,-rpath,/opt/obj/usr/ports/multimedia/x265/work/.build: libx265.so.179 -lpthread -lrt -ldl && : libx265.so.179: undefined reference to `x265_10bit::x265_api_get_179(int)' libx265.so.179: undefined reference to `x265_10bit::x265_api_query(int, int, int*)' libx265.so.179: undefined reference to `x265_12bit::x265_api_query(int, int, int*)' libx265.so.179: undefined reference to `x265_12bit::x265_api_get_179(int)' c++: error: linker command failed with exit code 1 (use -v to see invocation) ninja: build stopped: subcommand failed. *** Error code 1
A commit references this bug: Author: mi Date: Thu Feb 6 04:57:37 UTC 2020 New revision: 525362 URL: https://svnweb.freebsd.org/changeset/ports/525362 Log: My usage of -fuse-ld=lld has hidden a linking problem reported by several people. Rework for the default linker to work as well. Also, fix the build on i386, where use of assembler code needs to be disabled for pixel-widths other than 8. PR: 238773 Reported by: VVD, Alan Valentine, kib@, jbeich@ Changes: head/multimedia/x265/Makefile
(In reply to commit-hook from comment #30) Build fine on 11.3 amd64 now for me. Thanks.