Skip to content

libvmaf/svm.cpp: use std:: min,max,swap instead of custom defined ones - #1617

Open
AdelKS wants to merge 1 commit into
Netflix:masterfrom
AdelKS:master
Open

AdelKS wants to merge 1 commit into
Netflix:masterfrom
AdelKS:master

Conversation

@AdelKS

@AdelKS AdelKS commented Sep 27, 2026

Copy link
Copy Markdown

Instead of fighting the STL, embrace it 🔥

Avoids ambiguity with STL functions, as reported when building with clang.

Closes: #1616

@lusoris

lusoris commented Oct 1, 2026

Copy link
Copy Markdown

Tested at d3eb8f4 merged onto master 6ec23e8 (clean), x86-64 Linux, gcc 16.2.1 with libstdc++, and a mingw-w64 cross build (g++ 16.2.0). Both build; release meson test 23/23, ASan+UBSan 21 pass plus the two LeakSanitizer failures master has as well. The three Netflix pairs (frames and pooled values; vmaf_v0.6.1, psnr, float_ssim, motion, float_motion) are identical to master with the default CPU mask, --cpumask 16 and all SIMD masked.

On the libc++ 23 swap ambiguity this PR is the same fix as the one described on #1616; I could only build with libc++ 21.1 there, not 23.

The one behavioural difference to know about is that std::min / std::max return the first argument when the operands compare equal, while the libsvm templates they replace return the second. For equal numbers the result is the same value; it only differs for -0.0 vs +0.0 and for NaN. I went through the call sites: libvmaf only calls svm_predict() (predict.c:402), which goes through svm_predict_values(), and that function has no min / max in it. All uses are elsewhere in svm.cpp: Cache::Cache, Solver::Solve / calculate_rho, Solver_NU::select_working_set / do_shrinking / calculate_rho, the solve_* helpers (lines 1501-1506, 1618), multiclass_probability and svm_predict_probability (lines 1836, 2615), and svm_check_parameter (line 2906). libvmaf calls none of those, so the replacement cannot change prediction results, which matches the identical scores above.

chromium-full-mirror-sync-agent Bot pushed a commit to chromium-full-mirror/chromiumos_codesearch that referenced this pull request Oct 7, 2026
Apply Netflix/vmaf#1617 to fix an ambiguous call
to swap when building with newer libc++ (__split_buffer).

The pull requests has not yet been merged in upstream or Gentoo,
so carry a local patch for now.

BUG=b:568189402
TEST=emerge libvmaf with llvm-r614150

Change-Id: Ib854b62ac77b9d64f0715e85205abb56bebf1c10
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/8528106
Reviewed-by: Ryan Neph <ryanneph@google.com>
Commit-Queue: Bob Haarman <inglorion@chromium.org>
Tested-by: Bob Haarman <inglorion@chromium.org>
chromium-full-mirror-sync-agent Bot pushed a commit to chromium-full-mirror/chromiumos_codesearch that referenced this pull request Oct 7, 2026
Apply Netflix/vmaf#1617 to fix an ambiguous call
to swap when building with newer libc++ (__split_buffer).

The pull requests has not yet been merged in upstream or Gentoo,
so carry a local patch for now.

BUG=b:568189402
TEST=emerge libvmaf with llvm-r614150

Change-Id: Ib854b62ac77b9d64f0715e85205abb56bebf1c10
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/8528106
Reviewed-by: Ryan Neph <ryanneph@google.com>
Commit-Queue: Bob Haarman <inglorion@chromium.org>
Tested-by: Bob Haarman <inglorion@chromium.org>
chromium-full-mirror-sync-agent Bot pushed a commit to chromium-full-mirror/chromiumos_codesearch that referenced this pull request Oct 7, 2026
Apply Netflix/vmaf#1617 to fix an ambiguous call
to swap when building with newer libc++ (__split_buffer).

The pull requests has not yet been merged in upstream or Gentoo,
so carry a local patch for now.

BUG=b:568189402
TEST=emerge libvmaf with llvm-r614150

Change-Id: Ib854b62ac77b9d64f0715e85205abb56bebf1c10
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/8528106
Reviewed-by: Ryan Neph <ryanneph@google.com>
Commit-Queue: Bob Haarman <inglorion@chromium.org>
Tested-by: Bob Haarman <inglorion@chromium.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build failure with llvm-23.1.2 and clang-23.1.2 [error: call to 'swap' is ambiguous]

2 participants