Skip to content

QVAC-27588 fix[audiogen-ggml]: return raw ACE-Step output when normalizeLoudness is off - #5603

Open
RamazTs wants to merge 4 commits into
mainfrom
fix/QVAC-27588-audiogen-normalize-loudness
Open

RamazTs wants to merge 4 commits into
mainfrom
fix/QVAC-27588-audiogen-normalize-loudness

Conversation

@RamazTs

@RamazTs RamazTs commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

🎯 What problem does this PR solve?

  • ACE-Step normalizeLoudness: false did not return raw output. The option reaches the engine, but the addon then rescaled every non-edit result to a 0.9 peak before the int16 conversion, whatever the option said. The flag only switched off the engine's percentile pass.
  • The same rescale was applied to lego stems. The engine never normalizes a stem, so that it keeps its mix gain relative to the source; the addon raised or lowered each stem to a 0.9 peak.

📝 How does it solve it?

  • The gain decision moves out of AcestepModel::generate into addon/src/model-interface/acestep/OutputGain.hpp. It follows the engine's rule: only a generation that asked for normalization is rescaled. normalizeLoudness: false, edit plans and lego stems keep gain 1.0.
  • A normalized generation is unchanged: still scaled to a 0.9 peak, still left alone when near-silent.
  • With normalization off, samples the VAE leaves outside [-1, 1] now saturate at int16 full scale. That is what raw output means at an int16 boundary; the README and the option's doc comment say so.
  • The cpp-tests lane now runs npm run test:cpp (the addon's own gtests, this one and the budget and runtime-option tests) after the native build's caches are saved and before the fuzz build; it only printed a stub before. The workflow policy test expects the new step.
  • No JS API change. index.d.ts is regenerated for the doc comment. CHANGELOG entry under Unreleased, no version bump (package is at 0.10.0, released today).

Scope note: QVAC-27588 also lists ACE-Step backend / lmBackend and MiniMax coremlConditionerPath. Those are already implemented in #5495, so this PR leaves them alone. It touches a different part of AcestepModel.cpp and should merge cleanly on either side of it.

🧪 How was it tested?

  • New gtest addon/tests/test_output_gain.cpp (6 cases: normalized generation, normalization off, lego stem, edit, near-silent and empty input, quiet input), added to audiogen_ggml_tests. Compiled and run locally together with the existing budget tests against an existing vcpkg tree: 11/11 pass.
  • AcestepModel.cpp syntax-checked with the package's compile flags against the engine header at the pinned speech-cpp commit; clang-format clean.
  • npm run lint:js, npm run lint:ts, npm run test:types (typecheck, d.ts, generated-wrapper check) pass; JS unit tests 185/185; test:node 47/47.
  • Not done locally: a full native build and a real generation. The machine did not have the disk for a clean vcpkg build, so the changed code path has not produced audio here; CI's native build and integration lanes are the first real run. test:package was not completed locally (it downloads model files).

…off and keep lego stem gain

The addon rescaled every non-edit result to a 0.9 peak before the int16
quantisation, whatever normalizeLoudness said, and did the same to lego
stems. The engine normalizes neither, so the flag only switched off its
percentile pass and a stem lost its gain relative to the source.

The gain decision moves to OutputGain.hpp, where it is unit-tested.
@RamazTs
RamazTs requested review from a team as code owners November 5, 2026 19:01
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Review Status

Current Status: ✅ APPROVED
Approvals so far: Team Lead: 1, Member: 1

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

License compliance — clean

No new dependency license findings in this PR.

Warn-only (shadow) mode — this check does not block merges yet.

Updated automatically by the canonical license compliance workflow.

NOTICE presence (advisory)

Missing NOTICE (advisory, does not block):

  • ./docs/website
  • ./packages/asr-ggml/benchmarks/server
  • ./packages/fabric/test/integration
  • ./packages/llm-llamacpp/benchmarks/performance
  • ./packages/llm-llamacpp/benchmarks/server
  • ./packages/inference-addon-cpp/mobile
  • ./packages/sdk-e2e
  • ./packages/embed-llamacpp/benchmarks/performance
  • ./packages/embed-llamacpp/benchmarks/server
  • ./packages/vla-ggml/sim/server
  • ./.github/actions/release-merge-guard
  • ./.github/bare-toolchain

@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues were found.

Summary

ACE-Step now keeps the engine's gain when normalizeLoudness is off or the output is a lego stem.

  • ACE-Step keeps raw gain for edits, stems, and unnormalized audio.
  • The AudioGen C++ workflow runs addon tests before its fuzz checks.

Review checks were based on source inspection; no native build or audio generation was run.

Reviews (4) · Last reviewed commit: "Merge branch 'main' into fix/QVAC-27588-..." · Reviewed by Greptile

Comment thread packages/audiogen-ggml/CMakeLists.txt Resolved
mexxik
mexxik previously approved these changes Nov 6, 2026
…lane

The lane built the addon, printed a stub notice and ran only the fuzz
target, so audiogen_ggml_tests never ran in CI. It now runs test:cpp
after the native build's caches are saved and before the fuzz build
reconfigures the tree.
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Workflow security (shadow mode)

zizmor found 782 finding(s) in .github/ (highest severity: high). This check is warn-only and does not block the merge.

Findings are annotated inline on the changed files and listed in the job summary.

Reproduce locally:

pipx run zizmor==1.27.0 --offline .github/

Keeps the new gain test beside the tests #5495 added and expects the
C++ test step in the workflow policy test.

This branch was previously deployed

1 inactive deployment
release — d41c3c7d Deployed Nov 9, 2026 by GustavoA1604 via cpp-lint (audiogen-ggml) / cpp-lint #7018
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.

4 participants