Skip to content

Fix rustfmt on bindings, enable cross-language LTO, and improve Opus build configuration - #23

Merged
DoumanAsh merged 7 commits into
DoumanAsh:masterfrom
Andrew-Morozko:master
Jul 24, 2026
Merged

Fix rustfmt on bindings, enable cross-language LTO, and improve Opus build configuration#23
DoumanAsh merged 7 commits into
DoumanAsh:masterfrom
Andrew-Morozko:master

Conversation

@Andrew-Morozko

@Andrew-Morozko Andrew-Morozko commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Messed around with opus for my project and came up with a few improvements to this library. Can break the PR up if you would want to accept the changes partially.

  • Re-enable rustfmt for bindings
    "disable_all_formatting = true" in .rustfmt.toml applies to the formatting done on lib.rs during bindgen, and it looks like a one-line mess. Specify empty rustfmt config to be applied to the generated file so it is formatted normally and trim the PREPEND_LIB to make newly-generated lib.rs identical to the current one.

  • Allow cross-language LTO
    Now we detect if the library is compiled with cross-language LTO (using linker-plugin-lto) and pass it onward to cmake. If linker-plugin-lto is not used - keeps behavior as-is. I've checked, and this is actually working, for example a call to opus_encoder_get_size(2) is replaced with a constant in the final binary.

  • Explicitly control opus options
    Always set feature-controlled options as either "ON" or "OFF", do not rely on defaults of the opus.

  • Set CMAKE_BUILD_TYPE from OPT_LEVEL
    Explicitly set CMAKE_BUILD_TYPE to "Release", except when built with rust opt level "s" or "z", then "MinSizeRel". I tried setting it to "Debug" when OPT_LEVEL="0", but this causes more rebuilds and doesn't make the builds much faster.

  • CPU feature detection from TARGET_FEATUREs
    Pass TARGET_FEATUREs from rust to cmake, allowing the user to control guaranteed CPU features using standard rust methods (like target-cpu). Also added "no-runtime-feature-detection" feature to disable runtime feature detection in opus.

Comment thread build.rs Outdated
.define("OPUS_ARM_MAY_HAVE_NEON", to_opt(DO_RUNTIME_DETECTION));
}
Ok("x86_64") | Ok("x86") => {
let [mut has_sse, mut has_sse2, mut has_sse41, mut has_avx2, mut has_fma] = [false; _];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please use simpler construct of initializing every variable individually

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread build.rs Outdated
const DO_RUNTIME_DETECTION: bool = cfg!(not(feature = "no-runtime-feature-detection"));

let Ok(target_features) = std::env::var("CARGO_CFG_TARGET_FEATURE") else {
return;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We need to apply here result of DO_RUNTIME_DETECTION since else branch will not set OPUS_*_MAY flags

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How about this? I'd like to apply the OPUS_*_MAY only for corresponding architectures, and this will work in the unlikely case if we fail to get CARGO_CFG_TARGET_FEATURE. Also added an error message for missing CARGO_CFG_TARGET_ARCH

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yeah, that's fine I doubt this branch could happen so this approach is the best to take

@DoumanAsh
DoumanAsh merged commit e615989 into DoumanAsh:master Jul 24, 2026
3 checks passed
@DoumanAsh

Copy link
Copy Markdown
Owner

Thank you

@DoumanAsh

Copy link
Copy Markdown
Owner

Release 0.7.4 with changes introduced by your PR

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.

2 participants