casaroli opened a new pull request, #3684:
URL: https://github.com/apache/nuttx-apps/pull/3684

   ## 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](https://sourceforge.net/p/lame/svn/6655/) (2026-07-25) — "Detect 
AVX2 and offer it to the vector routines"
   * [r6718](https://sourceforge.net/p/lame/svn/6718/) (2026-07-30) — "Add an 
AVX-512 tier for the constant-bitrate vector routines"
   * [r6720](https://sourceforge.net/p/lame/svn/6720/) (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](https://github.com/apache/nuttx/actions/runs/30580951557/job/91001313430)):
   
   ```text
   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](https://sourceforge.net/p/lame/svn/6720/tree/trunk/lame/libmp3lame/vector/),
 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
   
   - [x] This PR introduces one focused functional change.
   - [x] All required PR description fields are completed.
   - [x] The commit has a descriptive topic/body, `Signed-off-by`, and 
`Assisted-by` trailer.
   - [x] The modified CMake file passes `cmake-format` and `git diff --check`.
   - [x] This PR is ready for review.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to