Skip to content

x265: 3.4 → 3.5#154347

Merged
c0bw3b merged 1 commit intoNixOS:stagingfrom
c0bw3b:pkg/x265
Jan 21, 2022
Merged

x265: 3.4 → 3.5#154347
c0bw3b merged 1 commit intoNixOS:stagingfrom
c0bw3b:pkg/x265

Conversation

@c0bw3b
Copy link
Contributor

@c0bw3b c0bw3b commented Jan 10, 2022

Motivation for this change
  • Lib update

  • Refactor to eliminate code duplication and crust

  • Don't use mkDerivation to build 10bits and 12bits static libs as we don't want
    to install them in the nix store forever (only needed during the build)

  • Enable check phase when possible

  • Split headers into a "dev" output

  • Use SPDX 3.0 license identifier

Close #149817

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@c0bw3b
Copy link
Contributor Author

c0bw3b commented Jan 10, 2022

Outputs sizes comparison

# V3.5 shared lib (8+10+12 bits) and CLI and shared HDR10+ lib
$ nix path-info -sSh /nix/store/xvcw5hw5h77r7ycjwmy7nivk484s2f5z-x265-3.5
/nix/store/xvcw5hw5h77r7ycjwmy7nivk484s2f5z-x265-3.5      20.7M   58.3M
# V3.5 headers and pkgconfig
$ nix path-info -sSh /nix/store/l989a0vjwwywgs6ryxxmbw7h7rxaya8p-x265-3.5-dev
/nix/store/l989a0vjwwywgs6ryxxmbw7h7rxaya8p-x265-3.5-dev         109.4K   58.4M
# V3.4 shared lib and CLI without HDR10+ lib
$ nix path-info -sSh /nix/store/54klpxlyhq41d64ylfpmywhxrc13z6k7-x265-3.4
/nix/store/54klpxlyhq41d64ylfpmywhxrc13z6k7-x265-3.4      20.6M   58.2M
# plus static 10bits lib and static HDR10 and duplicated headers
$ nix path-info -sSh /nix/store/y7vipkdwgmj0c3r2fgw87kvf3118p9df-libx265-10-3.4/
/nix/store/y7vipkdwgmj0c3r2fgw87kvf3118p9df-libx265-10-3.4         9.1M    9.1M
# plus static 12bits lib and duplicated headers
$ nix path-info -sSh /nix/store/7nc5jhi6jp33bxg52v48hz31z6dx7d2c-libx265-12-3.4/
/nix/store/7nc5jhi6jp33bxg52v48hz31z6dx7d2c-libx265-12-3.4         9.0M    9.0M

@ofborg ofborg bot requested a review from codyopel January 10, 2022 23:50
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Jan 10, 2022
Copy link
Contributor Author

@c0bw3b c0bw3b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully rebuilt most packages directly depending on x265 with

$ nixpkgs-review pr 154347 -p x265 -p ffmpeg -p libheif -p avidemux -p handbrake -p digikam


Result of nixpkgs-review pr 154347 run on x86_64-linux 1

6 packages built:
  • avidemux
  • digikam
  • ffmpeg
  • handbrake
  • libheif
  • x265

avidemux runs fine

@c0bw3b
Copy link
Contributor Author

c0bw3b commented Jan 16, 2022

@GrahamcOfBorg test handbrake

@c0bw3b
Copy link
Contributor Author

c0bw3b commented Jan 17, 2022

Rechecking Darwin platforms
@ofborg build x265

@SuperSandro2000
Copy link
Member

Rechecking Darwin platforms @ofborg build x265

darwin is currently very broken on staging.

@ofborg ofborg bot added 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. and removed 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Jan 17, 2022
* Lib update

* Refactor to eliminate code duplication and crust

* Don't use mkDerivation to build 10bits and 12bits static libs as we don't want
  to install them in the nix store forever (only needed during the build)

* Enable check phase when possible

* Split headers into a "dev" output

* Use SPDX 3.0 license identifier
@c0bw3b c0bw3b linked an issue Jan 20, 2022 that may be closed by this pull request
2 tasks
@c0bw3b c0bw3b merged commit 3f471af into NixOS:staging Jan 21, 2022
@c0bw3b c0bw3b deleted the pkg/x265 branch January 21, 2022 09:34
@vs49688
Copy link
Contributor

vs49688 commented Feb 7, 2022

This broke aarch64-darwin:

[100%] Linking CXX executable x265
[100%] Built target cli
running tests
/nix/store/0qlq4s175mzlx9g0aqcdk95j92g1z7mf-stdenv-darwin/setup: line 1363: ./test/TestBench: No such file or directory
builder for '/nix/store/sgf9mmgk6c8b063182b6833k6g30is7x-x265-3.5.drv' failed with exit code 127

Changing unittestsSupport fixes it, but I'm not sure if that's the right solution:

diff --git a/pkgs/development/libraries/x265/default.nix b/pkgs/development/libraries/x265/default.nix
index 8ddcedf198b..409783b48c2 100644
--- a/pkgs/development/libraries/x265/default.nix
+++ b/pkgs/development/libraries/x265/default.nix
@@ -17,7 +17,7 @@
 , custatsSupport ? false # Internal profiling of encoder work
 , debugSupport ? false # Run-time sanity checks (debugging)
 , ppaSupport ? false # PPA profiling instrumentation
-, unittestsSupport ? (!stdenv.is32bit) # Unit tests - only testing x64 assembly
+, unittestsSupport ? (stdenv.isx86_64) # Unit tests - only testing x64 assembly
 , vtuneSupport ? false # Vtune profiling instrumentation
 , werrorSupport ? false # Warnings as errors
 }:

@vs49688 vs49688 mentioned this pull request Feb 7, 2022
13 tasks
@SuperSandro2000
Copy link
Member

I suspect that the shebang needs to be fixed of ./test/TestBench.

@vs49688
Copy link
Contributor

vs49688 commented Feb 7, 2022

./test/TestBench is a binary that isn't being built.

The conditions to build it are a bit screwy:

if(ENABLE_ASSEMBLY AND NOT XCODE)
    option(ENABLE_TESTS "Enable Unit Tests" OFF)
    if(ENABLE_TESTS)
        add_subdirectory(test)
    endif()
endif()

ENABLE_ASSEMBLY is enabled when:

if(ARM OR CROSS_COMPILE_ARM)
    option(ENABLE_ASSEMBLY "Enable use of assembly coded primitives" ON)

But that is never set because arm64 isn't in ARM_ALIASES.

-- CMAKE_SYSTEM_PROCESSOR value `arm64` is unknown
-- Please add this value near /tmp/nix-build-x265-3.5.drv-0/x265_3.5/source/CMakeLists.txt:84

(On M1 uname -m returns arm64)

Once added:

set(ARM_ALIASES armv6l armv6l armv7l armv7a aarch64 arm64)

This causes a whole bunch of build errors like so:

/Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/common/aarch64/asm-primitives.cpp:104:33: error: use of undeclared identifier 'x265_12bit_pixel_satd_4x4_neon'; did you mean 'x265_pixel_satd_4x4_neon'?
        p.pu[LUMA_4x4].satd   = PFX(pixel_satd_4x4_neon);
                                ^~~~~~~~~~~~~~~~~~~~~~~~
                                x265_pixel_satd_4x4_neon
/Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/common/cpu.h:32:28: note: expanded from macro 'PFX'
#define PFX(name)          PFX2(X265_NS, name)
                           ^
/Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/common/cpu.h:31:28: note: expanded from macro 'PFX2'
#define PFX2(prefix, name) PFX3(prefix, name)
                           ^
/Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/common/cpu.h:30:28: note: expanded from macro 'PFX3'
#define PFX3(prefix, name) prefix ## _ ## name
                           ^
<scratch space>:63:1: note: expanded from here
x265_12bit_pixel_satd_4x4_neon
^
/Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/common/aarch64/pixel-util.h:28:5: note: 'x265_pixel_satd_4x4_neon' declared here
int x265_pixel_satd_4x4_neon(const pixel* pix1, intptr_t stride_pix1, const pixel* pix2, intptr_t stride_pix2);
    ^
In file included from /Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/encoder/analysis.cpp:33:
In file included from /Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/encoder/analysis.h:30:
In file included from /Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/common/predict.h:30:
In file included from /Users/zane.vaniperen/Documents/Coding/nixpkgs/x/x265_3.5/source/common/quant.h:29:

vs49688 added a commit to vs49688/nixpkgs that referenced this pull request Feb 7, 2022
Temporary to not break its dependencies.

See NixOS#154347 (comment)
@c0bw3b
Copy link
Contributor Author

c0bw3b commented Feb 7, 2022

Looking at Homebrew formula, they don't use nasm on aarch64 Darwin:
https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/x265.rb#L21

Which means no ASM on this platform, which means there is nothing to test.

So I believe simply disabling the unit tests on this platform as done in vs49688@9b93014 in indeed the valid approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

x265: 3.4 -> 3.5

3 participants