Fix rustfmt on bindings, enable cross-language LTO, and improve Opus build configuration - #23
Merged
Merged
Conversation
DoumanAsh
reviewed
Jul 23, 2026
| .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; _]; |
Owner
There was a problem hiding this comment.
Please use simpler construct of initializing every variable individually
DoumanAsh
reviewed
Jul 23, 2026
| 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; |
Owner
There was a problem hiding this comment.
We need to apply here result of DO_RUNTIME_DETECTION since else branch will not set OPUS_*_MAY flags
Contributor
Author
There was a problem hiding this comment.
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
Owner
There was a problem hiding this comment.
Yeah, that's fine I doubt this branch could happen so this approach is the best to take
DoumanAsh
approved these changes
Jul 24, 2026
Owner
|
Thank you |
Owner
|
Release 0.7.4 with changes introduced by your PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.tomlapplies to the formatting done onlib.rsduringbindgen, and it looks like a one-line mess. Specify emptyrustfmtconfig to be applied to the generated file so it is formatted normally and trim thePREPEND_LIBto make newly-generatedlib.rsidentical 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_FEATUREsPass
TARGET_FEATUREs from rust to cmake, allowing the user to control guaranteed CPU features using standard rust methods (liketarget-cpu). Also added "no-runtime-feature-detection" feature to disable runtime feature detection in opus.