Skip to content

audioutils/lame: pin the checkout and build its AVX-512 sources - #3684

Open
casaroli wants to merge 1 commit into
apache:masterfrom
casaroli:lame-pin-r6720
Open

audioutils/lame: pin the checkout and build its AVX-512 sources#3684
casaroli wants to merge 1 commit into
apache:masterfrom
casaroli:lame-pin-r6720

Conversation

@casaroli

Copy link
Copy Markdown
Contributor

Summary

The bundled LAME encoder is checked out from trunk with no revision, so every build takes whatever trunk happens to be at that moment. LAME's trunk grows vector tiers over time and each one adds sources that the two build files have to name, so an unpinned checkout stops linking the day upstream adds the next one. That has now happened twice in six days:

  • r6655 (2026-07-25) — "Detect AVX2 and offer it to the vector routines"
  • r6718 (2026-07-30) — "Add an AVX-512 tier for the constant-bitrate vector routines"
  • r6720 (2026-07-30) — "Bring back the AVX-512 band-noise routine, behind a second switch"

The AVX-512 sources were never listed, so sim:alsa stopped linking on x86 hosts (CI job):

takehiro.c:332:    undefined reference to `quantize_lines_xrpow_avx512'
takehiro.c:533:    undefined reference to `ix_max_avx512'
takehiro.c:569:    undefined reference to `count_bit_esc_avx512'
vbrquantize.c:261: undefined reference to `calc_sfb_noise_x34_avx512'

The first three come from r6718, the fourth from r6720. r6718 is also what starts defining HAVE_AVX512_INTRINSICS in config.h, which is why the call sites and the definitions appeared together upstream — only this repository's source list was left behind.

This change:

  1. Pins the checkout to r6720 through a LAME_VERSION variable in both build files, matching how the rest of apps/ handles third-party sources. The pin is what keeps the two in step: the source list is maintained by hand, so it can only be correct for a known revision. Raising it is now a deliberate act, taken together with the source list.
  2. Adds the three AVX-512 sources that r6720 provides — avx512_choose_table.c, avx512_quantize_lines.c, avx512_calc_sfb_noise.c.
  3. Fixes the same gap in Makefile, which named only vector/xmm_quantize_sub.c and none of the other vector sources. A Make build on an x86 host fails the same way with a longer symbol list, from SSE2 upwards. CI builds sim:alsa through CMake only, so that half was latent rather than visible. Both files now list the same nine sources under the same host condition.

Worth noting for reviewers: the checkout is only performed when lame/configure is absent and is never updated afterwards. Before this change a tree was therefore frozen at whatever trunk was on the day it was first built — anyone who checked out before 2026-07-30 still links and cannot reproduce the failure, which is why this surfaced only in CI.

Impact

  • New feature: No.
  • User adaptation: No. Existing trees with a lame/ checkout are untouched; the pin applies to fresh checkouts. Delete apps/audioutils/lame/lame to move an existing tree to r6720.
  • Build impact: Yes. Fixes the sim:alsa link failure on x86 hosts and makes the LAME source revision reproducible.
  • Hardware impact: No. Host-simulator build integration; the x86 vector routines are still selected only by LAME's runtime CPU dispatch.
  • Documentation impact: No.
  • Security impact: No.
  • Compatibility impact: No. Non-x86 hosts continue to compile and use LAME's scalar path.
  • Dependency: none.

Testing

Verified:

  • cmake-format --check audioutils/lame/CMakeLists.txt — clean.
  • git diff --check — clean.
  • tools/checkpatch.sh -c -u -m -g <base>..HEAD — ✔️ All checks pass.
  • The new ifneq ($(CONFIG_HOST_X86)$(CONFIG_HOST_X86_64),) condition evaluated with GNU make for all three cases — neither symbol set, CONFIG_HOST_X86=y, and CONFIG_HOST_X86_64=y — selecting the vector group in the latter two only.
  • The nine source names checked against the libmp3lame/vector listing at r6720, and the four missing symbols traced to their defining files (ix_max_avx512 and count_bit_esc_avx512 in avx512_choose_table.c, quantize_lines_xrpow_avx512 in avx512_quantize_lines.c, calc_sfb_noise_x34_avx512 in avx512_calc_sfb_noise.c).

Not verified by me: I have no x86 host, so I have not linked sim:alsa with this change — my only machines are arm64. The source list and the pin are verified by inspection against r6720 as described above, and CI covers the actual link. If a maintainer with an x86 box can confirm the build before merge, that would close the gap.

One thing this change does not address: neon_choose_table.c exists at r6720 and is not listed for any host. If LAME's configure enables NEON dispatch on an arm64 simulator host, that host would fail the same way. I have not confirmed whether it does, and did not want to add an untested condition here.

PR verification self-check

  • This PR introduces one focused functional change.
  • All required PR description fields are completed.
  • The commit has a descriptive topic/body, Signed-off-by, and Assisted-by trailer.
  • The modified CMake file passes cmake-format and git diff --check.
  • This PR is ready for review.

The bundled encoder was checked out from lame's trunk with no revision, so
every build took whatever trunk happened to be at that moment.  lame's trunk
grows vector tiers over time, and each one adds sources that the two build
files have to name:  r6655 offered AVX2 to the vector routines on 2026-07-25,
and r6718 and r6720 added an AVX-512 tier on 2026-07-30.  The AVX-512 sources
were never listed, so sim:alsa stopped linking on x86 hosts:

  takehiro.c:332:    undefined reference to `quantize_lines_xrpow_avx512'
  takehiro.c:533:    undefined reference to `ix_max_avx512'
  takehiro.c:569:    undefined reference to `count_bit_esc_avx512'
  vbrquantize.c:261: undefined reference to `calc_sfb_noise_x34_avx512'

Pin the checkout to r6720 through a LAME_VERSION variable, as the rest of
apps/ pins its third-party sources, and list the three AVX-512 files that
revision provides.  The pin is what keeps the two in step:  the source list is
maintained by hand, so it can only be correct for a known revision.

The checkout is also only performed when lame/configure is absent and is never
updated afterwards, so before this an unpinned tree was frozen at whatever
trunk was on the day it was first built.  Anyone who checked out before
2026-07-30 still links and cannot reproduce the failure, which is why this
surfaced only in CI.

Makefile named just vector/xmm_quantize_sub.c and none of the other vector
sources, so a Make build on an x86 host fails the same way with a longer list
of symbols, from SSE2 upwards.  CI builds sim:alsa through CMake only, so that
half was latent rather than visible.  Both files now list the same nine
sources under the same host condition.

Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants