Summary: | multimedia/x265: Update to 3.5 | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | Ports & Packages | Reporter: | takefu | ||||||||||||||
Component: | Individual Port(s) | Assignee: | Mikhail T. <freebsd-2024> | ||||||||||||||
Status: | Closed FIXED | ||||||||||||||||
Severity: | Affects Only Me | CC: | diizzy, fernape, freebsd-2024, jcfyecrayz, mi, tatsuki_makino | ||||||||||||||
Priority: | --- | Flags: | bugzilla:
maintainer-feedback?
(freebsd-2024) |
||||||||||||||
Version: | Latest | ||||||||||||||||
Hardware: | Any | ||||||||||||||||
OS: | Any | ||||||||||||||||
Attachments: |
|
Description
takefu
2023-10-17 05:45:27 UTC
PORTCLIPPY(1) Compliant. Thank you very much for the patch. Looking at the upstream sources, I see they added explicit branch for the 3.5 release. Thus, shouldn't we be using something like BB_TAG=Release_${DISTVERSION} instead of chasing commit-hashes? Created attachment 245810 [details]
Use the release-branch
This version of the upgrade has the following changes:
1. Restores the order of variable-settings in the Makefile -- to reduce the size of the diff compared to the current version.
2. Use the upstream's release-branch instead of a hash.
3. Adds fixes for some compiler-warnings.
4. Replaces the bundled (warnings-prone) MD5-implementation with our -lmd
It passes the "make test check-plist" here -- would you care to test too? Thank you!
Created attachment 245811 [details]
Replace the bundled MD5-implementation with -lmd
"git diff" didn't include this newly-added file... It is supposed to be dropped into files/ verbatim.
Created attachment 245812 [details]
Fix some compiler warnings (set, but unused variables)
"git diff" didn't include this newly-added file either.
Hi Takefu, Mikhail Thanks for the work! There are three patches in the PR but none of them has the maintainer-approval set. Is any of them ready to be committed? > Is any of them ready to be committed?
Thanks for the help, Fernando. I'll do the committing myself, once we figure things out with the reporter.
(In reply to Mikhail T. from comment #7) Terrific, thanks! The layout of the Makefile can be polished but we can do that once everything else is in place. I would however be very cautious about grabbing patch files from Bitbucket due to unknown stability of patch file layout, preferably we should place it in files directory. > The layout of the Makefile can be polished Is there anything wrong with it as-is? Portlint does not seem to care -- is there some other tool, that's pickier? > Bitbucket due to unknown stability of patch file layout Well, the checksum of the patch is captured, so if the layout changes, we'll know right away. Any reasons to suspect Bitbucket in particular? I modeled this part on the libva-intel-media-driver, which fetches a dozen of patches from Github... portclippy in https://www.freshports.org/ports-mgmt/portfmt/ While I don't agree with fully, I'd say it gets ~97% right You're correct that we'll noticable breakage due to checksum mismatch my point is that we want to avoid that if possible. GitHub is relatively stable in that regard but I don't think anyone have enough data to say the same about Bitbucket. Gitlab for example is known to break patch files. (In reply to Daniel Engberg from comment #11) I'm trying to get the release version of the distfile and get the release version of each patch, but I wonder if I can solve the patch checksum problem by getting the stable version that I sentpr first? (In reply to takefu from comment #12) > I wonder if I can solve the patch checksum problem by getting the stable version that I sentpr first? It was my impression, that the 8f18e3a didn't have some other improvements, that the upstream committed and merged into their Release_3.5. If I'm wrong, we can certainly go back to the 8f18e3a -- or even a later one... A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=645419c8e46533e7e9953a432fa02dc7c140b63c commit 645419c8e46533e7e9953a432fa02dc7c140b63c Author: Mikhail Teterin <mi@FreeBSD.org> AuthorDate: 2023-12-15 21:12:58 +0000 Commit: Mikhail Teterin <mi@FreeBSD.org> CommitDate: 2023-12-15 21:12:58 +0000 multimedia/x265: upgrade to 3.5. Finally... PR: 274526 Submitted by: takefu Release Notes: https://x265.readthedocs.io/en/master/releasenotes.html#version-3-5 multimedia/x265/Makefile | 16 +- multimedia/x265/distinfo | 8 +- multimedia/x265/files/patch-md5 (new) | 51 ++++++ multimedia/x265/files/patch-source_CMakeLists.txt | 12 +- .../files/patch-source_cmake_Findsvthevc.cmake | 8 +- .../x265/files/patch-source_common_version.cpp | 11 +- multimedia/x265/files/patch-source_encoder_api.cpp | 19 +- multimedia/x265/files/patch-source_x265.h | 8 +- multimedia/x265/files/patch-warnings (new) | 201 +++++++++++++++++++++ multimedia/x265/pkg-plist | 2 +- 10 files changed, 296 insertions(+), 40 deletions(-) (In reply to commit-hook from comment #14) Perhaps broken mark for 12 is needed. There are only 15 days left, though :) It is an error to such an extent that it is tedious to write about :) > Perhaps broken mark for 12 is needed.
Huh? Could you send me the failure log? Or attach it here?
(In reply to Mikhail T. from comment #16) It seems to have failed from the very first move :) building for: FreeBSD src-default-job-01 12.4-STABLE FreeBSD 12.4-STABLE 1204500 amd64 FAILED: encoder/CMakeFiles/encoder.dir/frameencoder.cpp.o /usr/bin/c++ -DEXPORT_C_API=0 -DHAVE_INT_TYPES_H=1 -DHIGH_BIT_DEPTH=1 -DX265_ARCH_X86=1 -DX265_DEPTH=10 -DX265_NS=x265_10bit -DX86_64=1 -D__STDC_LIMIT_MACROS=1 -I/wrkdirs/usr/ports/multimedia/x265/work/source/. -I/wrkdirs/usr/ports/multimedia/x265/work/source/common -I/wrkdirs/usr/ports/multimedia/x265/work/source/encoder -I/wrkdirs/usr/ports/multimedia/x265/work/10bit -O2 -pipe -O3 -fstack-protector-strong -fno-strict-aliasing -O2 -pipe -O3 -fstack-protector-strong -fno-strict-aliasing -DNDEBUG -Wall -Wextra -Wshadow -std=gnu++98 -fPIC -Wno-array-bounds -ffast-math -mstackrealign -fno-exceptions -Wno-uninitialized -MD -MT encoder/CMakeFiles/encoder.dir/frameencoder.cpp.o -MF encoder/CMakeFiles/encoder.dir/frameencoder.cpp.o.d -o encoder/CMakeFiles/encoder.dir/frameencoder.cpp.o -c /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:27: In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/common/frame.h:29: In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/common/lowres.h:29: In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/common/picyuv.h:28: In file included from /usr/include/md5.h:46: /usr/include/sys/md5.h:39:3: error: unknown type name 'u_int32_t' u_int32_t state[4]; /* state (ABCD) */ ^ /usr/include/sys/md5.h:40:3: error: unknown type name 'u_int32_t' u_int32_t count[2]; /* number of bits, modulo 2^64 (lsb first) */ ^ In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:32: In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/encoder.h:30: /wrkdirs/usr/ports/multimedia/x265/work/source/common/scalinglist.h:36:12: warning: declaration shadows a variable in namespace 'x265_10bit' [-Wshadow] enum { NUM_SIZES = 4 }; // 4x4, 8x8, 16x16, 32x32 ^ /wrkdirs/usr/ports/multimedia/x265/work/source/common/cudata.h:51:5: note: previous declaration is here NUM_SIZES ^ In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:33: In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.h:39: In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/ratecontrol.h:30: /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/sei.h:173:9: error: cannot initialize object parameter of type 'x265_10bit::SyntaxElementWriter' with an expression of type 'x265_10bit::SEIDecodedPictureHash' WRITE_CODE(m_method, 8, "hash_type"); ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/sei.h:179:21: error: cannot initialize object parameter of type 'x265_10bit::SyntaxElementWriter' with an expression of type 'x265_10bit::SEIDecodedPictureHash' WRITE_CODE(m_digest[yuvIdx][i], 8, "picture_md5"); ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/sei.h:184:17: error: cannot initialize object parameter of type 'x265_10bit::SyntaxElementWriter' with an expression of type 'x265_10bit::SEIDecodedPictureHash' WRITE_CODE(val, 16, "picture_crc"); ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/sei.h:189:17: error: cannot initialize object parameter of type 'x265_10bit::SyntaxElementWriter' with an expression of type 'x265_10bit::SEIDecodedPictureHash' WRITE_CODE(val, 32, "picture_checksum"); ^~~~~~~~~~ In file included from /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:33: /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.h:242:50: error: cannot initialize object parameter of type 'x265_10bit::WaveFront' with an expression of type 'x265_10bit::FrameEncoder' void enqueueRowEncoder(int row) { WaveFront::enqueueRow(row * 2 + 0); } ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.h:243:50: error: cannot initialize object parameter of type 'x265_10bit::WaveFront' with an expression of type 'x265_10bit::FrameEncoder' void enqueueRowFilter(int row) { WaveFront::enqueueRow(row * 2 + 1); } ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.h:244:50: error: cannot initialize object parameter of type 'x265_10bit::WaveFront' with an expression of type 'x265_10bit::FrameEncoder' void enableRowEncoder(int row) { WaveFront::enableRow(row * 2 + 0); } ^~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.h:245:50: error: cannot initialize object parameter of type 'x265_10bit::WaveFront' with an expression of type 'x265_10bit::FrameEncoder' void enableRowFilter(int row) { WaveFront::enableRow(row * 2 + 1); } ^~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:167:21: error: cannot initialize object parameter of type 'x265_10bit::WaveFront' with an expression of type 'x265_10bit::FrameEncoder' if (!WaveFront::init(m_numRows * 2)) ^~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:270:42: error: incompatible pointer types assigning to 'x265_10bit::JobProvider *' from 'x265_10bit::FrameEncoder *' curFrame->m_encData->m_jobProvider = this; ^~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:533:30: error: no matching member function for call to 'tryBondPeers' if (m_pool && wa.tryBondPeers(*this, 1)) ~~~^~~~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/common/threadpool.h:140:9: note: candidate function not viable: no known conversion from 'x265_10bit::FrameEncoder' to 'x265_10bit::JobProvider &' for 1st argument int tryBondPeers(JobProvider& jp, int maxPeers) ^ /wrkdirs/usr/ports/multimedia/x265/work/source/common/threadpool.h:150:9: note: candidate function not viable: no known conversion from 'x265_10bit::FrameEncoder' to 'x265_10bit::ThreadPool &' for 1st argument int tryBondPeers(ThreadPool& pool, int maxPeers) ^ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:632:16: error: cannot initialize object parameter of type 'x265_10bit::WaveFront' with an expression of type 'x265_10bit::FrameEncoder' WaveFront::clearEnabledRowMask(); ^~~~~~~~~~~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:876:17: error: cannot initialize object parameter of type 'x265_10bit::JobProvider' with an expression of type 'x265_10bit::FrameEncoder' tryWakeOne(); ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:881:9: error: cannot initialize object parameter of type 'x265_10bit::JobProvider' with an expression of type 'x265_10bit::FrameEncoder' tryWakeOne(); /* ensure one thread is active or help-wanted flag is set prior to blocking */ ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:884:13: error: cannot initialize object parameter of type 'x265_10bit::JobProvider' with an expression of type 'x265_10bit::FrameEncoder' tryWakeOne(); ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:1725:37: error: cannot initialize object parameter of type 'x265_10bit::WaveFront' with an expression of type 'x265_10bit::FrameEncoder' if (dequeueRow(m_row_to_idx[r] * 2)) ^~~~~~~~~~ /wrkdirs/usr/ports/multimedia/x265/work/source/encoder/frameencoder.cpp:1779:17: error: cannot initialize object parameter of type 'x265_10bit::JobProvider' with an expression of type 'x265_10bit::FrameEncoder' tryWakeOne(); /* wake up a sleeping thread or set the help wanted flag */ ^~~~~~~~~~ fatal error: too many errors emitted, stopping now [-ferror-limit=] 1 warning and 20 errors generated. (In reply to Tatsuki Makino from comment #17) If patches for 12.x will be provided, <sys/types.h> must be included for <sched.h>. Probably before <cstdlib>. It seems that this file will compile. for now, just this first. (In reply to Tatsuki Makino from comment #18) > <sys/types.h> must be included for <sched.h>. Probably before <cstdlib>. Could you upload an actual patch, please? Created attachment 247074 [details] proposed patch to multimedia/x265 for FreeBSD 12.x (In reply to Mikhail T. from comment #19) For now, this is all that is expected to compile successfully. Perhaps this patch is enough. Created attachment 247127 [details] [patch] md5.h requires sys/types.h (In reply to Tatsuki Makino from comment #20) /usr/include/md5.h (not sched.h) needs sys/types.h (see files/patch-md5 for the change that now includes the system md5.h header). And it is not just for FreeBSD 12. To illustrate that, try compiling a single line C file that includes md5.h on FreeBSD 13: ================================= % cat > /tmp/incmd5.c << eof #include <md5.h> eof % cc -c -o /dev/null -Wall tmp/incmd5.c In file included from tmp/incmd5.c:1: In file included from /usr/include/md5.h:46: /usr/include/sys/md5.h:39:3: error: unknown type name 'u_int32_t'; did you mean '__int128_t'? u_int32_t state[4]; /* state (ABCD) */ ^ note: '__int128_t' declared here /usr/include/sys/md5.h:40:3: error: unknown type name 'u_int32_t'; did you mean '__int128_t'? u_int32_t count[2]; /* number of bits, modulo 2^64 (lsb first) */ ^ note: '__int128_t' declared here /usr/include/sys/md5.h:53:39: error: redefinition of parameter 'off_t' char * MD5FdChunk(int, char *, off_t, off_t); ^ /usr/include/sys/md5.h:53:32: note: previous declaration is here char * MD5FdChunk(int, char *, off_t, off_t); ^ /usr/include/sys/md5.h:55:50: error: redefinition of parameter 'off_t' char * MD5FileChunk(const char *, char *, off_t, off_t); ^ /usr/include/sys/md5.h:55:43: note: previous declaration is here char * MD5FileChunk(const char *, char *, off_t, off_t); ^ 4 errors generated. ================================= It is just luck (or namespace pollution if you prefer to call it) that on FreeBSD 13 sys/types.h gets included as before hitting the new '#include <md5.h>'. And the md5(3) man page points out that sys/types.h is required. So it should not be a surprise. Attached is my suggested patch. No need for conditional compilation based on FreeBSD version. It is just what should have been in the original commit of files/patch-md5. Also adding sys/types.h to common.h (as suggested in the patch in comment 20) is a much wider scope that is "less correct" and is not necessary. (In reply to John Hein from comment #21) p.s. No need to bump PORTREVISION as this does not change the package at all. And this was tested on stable/12/amd64 & stable/13/amd64, with and without poudriere. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=4a9eb2b55c70f3057a82966c77542484ab434373 commit 4a9eb2b55c70f3057a82966c77542484ab434373 Author: Mikhail Teterin <mi@FreeBSD.org> AuthorDate: 2023-12-18 16:28:44 +0000 Commit: Mikhail Teterin <mi@FreeBSD.org> CommitDate: 2023-12-18 16:31:48 +0000 multimedia/x265: include <sys/types.h> before <md5.h> Though it worked on some releases, where another header quietly drags-in the types.h, it was broken on FreeBSD-12.x. Suggested by: John Hein PR: 274526 multimedia/x265/files/patch-md5 | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (In reply to John Hein from comment #22) Thanks, John, I updated the patch-md5. takefu, does it build for you now? (In reply to John Hein from comment #21) > /usr/include/md5.h needs sys/types.h > (not sched.h) So in 14.0 and later, there was no problem because <sched.h> (included by <cstdlib>) includes <sys/types.h>. And actually, <md5.h> needed the <sys/types.h>. (In reply to commit-hook from comment #23) (In reply to Mikhail T. from comment #24) The build was successful on my 12.4-STABLE amd64. Thank you very much. (In reply to Tatsuki Makino from comment #25) The md5 patch for 12.x-RELEASE seems to be working fine. However, work has stopped due to other reasons. Build with VMAF option is no longer possible after updating from 2.3.1 to 3.0.0.r1. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=10be66c9daed3b0f1d964469eb84caeaf38676ae commit 10be66c9daed3b0f1d964469eb84caeaf38676ae Author: Jan Beich <jbeich@FreeBSD.org> AuthorDate: 2023-12-17 11:50:06 +0000 Commit: Jan Beich <jbeich@FreeBSD.org> CommitDate: 2023-12-22 20:00:57 +0000 multimedia/x265: force rebuild consumers after 645419c8e465 $ ffmpeg ld-elf.so.1: Shared object "libx265.so.192" not found, required by "libavcodec.so.60" PR: 274526 Reported by: vvd (on dev-commits-ports-main@ list) Reported by: Kevin Oberman (on multimedia@ list) cad/opencascade/Makefile | 2 +- emulators/fceux/Makefile | 1 + graphics/digikam/Makefile | 1 + graphics/libbpg/Makefile | 2 +- graphics/libheif/Makefile | 1 + multimedia/avidemux-plugins/Makefile | 2 +- multimedia/emby-server/Makefile | 2 +- multimedia/ffmpeg/Makefile | 2 +- multimedia/ffmpeg4/Makefile | 2 +- multimedia/gstreamer1-plugins-x265/Makefile | 2 +- multimedia/vlc/Makefile | 2 +- 11 files changed, 11 insertions(+), 8 deletions(-) (In reply to takefu from comment #26) > Build with VMAF option is no longer possible after updating from 2.3.1 to 3.0.0.r1. I suppose, I should remove/disable the VMAF-option for now, but I don't want to do it for those, who still have the older VMAF installed... |