Bug 274526 - multimedia/x265: Update to 3.5
Summary: multimedia/x265: Update to 3.5
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Mikhail T.
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-10-17 05:45 UTC by takefu
Modified: 2024-01-15 05:51 UTC (History)
6 users (show)

See Also:
bugzilla: maintainer-feedback? (freebsd-2024)


Attachments
x265-3.5.patch (10.61 KB, patch)
2023-10-17 05:45 UTC, takefu
no flags Details | Diff
Use the release-branch (9.80 KB, patch)
2023-10-23 02:31 UTC, Mikhail T.
no flags Details | Diff
Replace the bundled MD5-implementation with -lmd (1.87 KB, patch)
2023-10-23 02:37 UTC, Mikhail T.
no flags Details | Diff
Fix some compiler warnings (set, but unused variables) (5.95 KB, patch)
2023-10-23 02:38 UTC, Mikhail T.
no flags Details | Diff
proposed patch to multimedia/x265 for FreeBSD 12.x (543 bytes, patch)
2023-12-16 04:01 UTC, Tatsuki Makino
no flags Details | Diff
[patch] md5.h requires sys/types.h (1.01 KB, patch)
2023-12-18 12:08 UTC, John Hein
jcfyecrayz: maintainer-approval? (mi)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 takefu 2023-10-17 05:55:04 UTC
PORTCLIPPY(1) Compliant.
Comment 2 Mikhail T. 2023-10-23 02:17:40 UTC
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?
Comment 3 Mikhail T. 2023-10-23 02:31:22 UTC
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!
Comment 4 Mikhail T. 2023-10-23 02:37:06 UTC
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.
Comment 5 Mikhail T. 2023-10-23 02:38:01 UTC
Created attachment 245812 [details]
Fix some compiler warnings (set, but unused variables)

"git diff" didn't include this newly-added file either.
Comment 6 Fernando Apesteguía freebsd_committer freebsd_triage 2023-10-23 08:42:12 UTC
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?
Comment 7 Mikhail T. 2023-10-23 17:30:48 UTC
> 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.
Comment 8 Fernando Apesteguía freebsd_committer freebsd_triage 2023-10-23 20:45:35 UTC
(In reply to Mikhail T. from comment #7)
Terrific, thanks!
Comment 9 Daniel Engberg freebsd_committer freebsd_triage 2023-10-23 21:15:21 UTC
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.
Comment 10 Mikhail T. 2023-10-23 22:50:31 UTC
> 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...
Comment 11 Daniel Engberg freebsd_committer freebsd_triage 2023-10-24 19:16:55 UTC
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.
Comment 12 takefu 2023-10-25 22:31:57 UTC
(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?
Comment 13 Mikhail T. 2023-10-25 23:37:33 UTC
(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...
Comment 14 commit-hook freebsd_committer freebsd_triage 2023-12-15 21:15:11 UTC
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(-)
Comment 15 Tatsuki Makino 2023-12-15 23:36:41 UTC
(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 :)
Comment 16 Mikhail T. 2023-12-15 23:38:52 UTC
> Perhaps broken mark for 12 is needed.

Huh? Could you send me the failure log? Or attach it here?
Comment 17 Tatsuki Makino 2023-12-16 02:52:30 UTC
(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.
Comment 18 Tatsuki Makino 2023-12-16 03:36:26 UTC
(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.
Comment 19 Mikhail T. 2023-12-16 03:38:01 UTC
(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?
Comment 20 Tatsuki Makino 2023-12-16 04:01:40 UTC
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.
Comment 21 John Hein 2023-12-18 12:08:53 UTC
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.
Comment 22 John Hein 2023-12-18 12:20:40 UTC
(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.
Comment 23 commit-hook freebsd_committer freebsd_triage 2023-12-18 16:32:11 UTC
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(-)
Comment 24 Mikhail T. 2023-12-18 16:33:22 UTC
(In reply to John Hein from comment #22)
Thanks, John, I updated the patch-md5. takefu, does it build for you now?
Comment 25 Tatsuki Makino 2023-12-18 21:33:35 UTC
(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.
Comment 26 takefu 2023-12-18 22:10:41 UTC
(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.
Comment 27 commit-hook freebsd_committer freebsd_triage 2023-12-22 20:06:03 UTC
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(-)
Comment 28 Mikhail T. 2023-12-23 17:40:41 UTC
(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...