Supports building with the Bazel building system by yeroro · Pull Request #16 · AOMediaCodec/oar

yeroro · 2026-07-27T09:19:50Z

Added BUILD.bazel files in all required folders.
Added MODULE.bazel to handle external dependencies.
Updated the README.md file.
Fixed some missing/unused include headers.

Minor changes:

Added some files to .gitignore.
Added a .bazelversion, because OBR (part of liboar) has only been tested with bazel v7.4.1.
Pulled the license info for OBR to the top-level liboar folder.

jingbo-marquis · 2026-07-27T10:21:35Z


 #define _USE_MATH_DEFINES
-#include <math.h>
-#include <string.h>


the code still uses sqrt(), memset(), tanf(), and M_PI inside #if DISABLE_LFE_HOA == 0 conditional blocks.
refer to #15

jingbo-marquis · 2026-07-28T01:42:50Z

+        "pffft.h",
+    ],
+    visibility = ["//visibility:public"],
+)


‎src/renderer/obr/obr_capi/obr/external/pffft.BUILD

The pffft BUILD files are located in two places; does pffft need to be built twice?

pffft is only built once: the root MODULE.bazel declares no pffft extension, so no @pffft repository exists in the root module. The only instantiation is in obr's extensions.bzl, where build_file = Label("//external:pffft.BUILD") resolves within the @obr module to src/renderer/obr/obr_capi/obr/external/pffft.BUILD. The top-level external/pffft.BUILD added by this PR is referenced by nothing — it should be deleted (along with the then-empty external/ directory).

jingbo-marquis · 2026-07-28T01:50:17Z

+build:arm64 --action_env=CXX=aarch64-linux-gnu-g++
+build:arm64 --cpu=aarch64
+
+# The folllowing are required by obr.


"folllowing" is a typo.

jingbo-marquis · 2026-07-28T02:01:19Z

+# This is to tell GCC to not generate .sframe sections, which does not work
+# with the LLVM linker (LLD).
+build --copt="-Wa,--gsframe=no"
+build --cxxopt="-Wa,--gsframe=no"


Not all GCC versions support --gsframe=no; early versions do not support a value immediately following the --gsframe parameter.
If this parameter is required, a more robust approach is needed.

trsonic · 2026-07-28T11:16:55Z

#15 has been merged and modifies src/renderer/ear/ear.c, h2m_rdr.c, ae_rdr.h, and tests/examples/, so this branch needs a rebase. It also adds a new test, tests/examples/test_hoa_lfe_rendering.c, built behind the new OAR_ENABLE_HOA_LFE CMake option (default OFF), which is not yet covered by the Bazel build files in this PR.

trsonic · 2026-07-28T11:26:48Z

+build --copt="-Wa,--gsframe=no"
+build --cxxopt="-Wa,--gsframe=no"


These flags are GCC/GAS-specific; clang fails with unsupported argument '--gsframe=no' to option '-Wa,', so the Bazel build currently fails on macOS (MSVC doesn't know the flag either). With these two lines removed, the full build succeeds on Apple Silicon (//:liboar including the @obr module, plus all tests/examples targets, and the binaries run correctly). Since --enable_platform_specific_config is already set, move both lines into the build:linux section.

trsonic · 2026-07-28T11:26:48Z

+build --copt="-Wa,--gsframe=no"
+build --cxxopt="-Wa,--gsframe=no"


Same issue as the top-level .bazelrc: GCC-specific flags that break clang — move into the build:linux section.

trsonic · 2026-07-28T11:26:48Z

 * @date Created 03/03/2023
 **/

 #define _USE_MATH_DEFINES


This removal was correct against the base this branch is on, but #15 (now on main) uses tanf, M_PI, and memset in this file's LFE filter init, so after rebasing, a build with OAR_ENABLE_HOA_LFE=ON fails to compile without <math.h> and <string.h>. It also leaves this #define _USE_MATH_DEFINES with no <math.h> include following it. Suggest dropping this hunk during the rebase.

trsonic · 2026-07-28T11:26:48Z

+# in the MODULE.bazel file using `local_path_override()`.
+# To build targets in the nested obr package, use `bazel build @obr//...`
+# instead of `bazel build //src/renderer/obr/obr_capi/obr/...`.
+src/renderer/obr/obr_capi/obr


bazel build //... fails when a CMake build/ directory exists — Bazel recurses into build/_deps and errors on abseil's fetched sources. The README's first build instruction is cmake -B build, so mixed CMake/Bazel use hits this immediately. Add build/ (and other common CMake output dirs like out/) here.

trsonic · 2026-07-28T11:26:48Z

+          cartesian_pos.x = 0.0f;
+          cartesian_pos.y = 0.0f;
+          cartesian_pos.z = 0.0f;


This is a genuine bug fix — the unsupported-animation-type branch previously fell through to cartesian_to_polar_sector_float32(cartesian_pos) with the variable uninitialized — but it's a behavior change inside a build-system PR. Please mention it in the PR description (or split it into its own commit) so it stays visible when bisecting.

trsonic · 2026-07-28T11:26:48Z

+exports_files(["LICENSE"])
+
+cc_library(
+    name = "liboar",


Same rename here: name = "oar", making the target //:oar (plus the four //:liboar references in tests/examples/BUILD.bazel). cc_library adds the lib prefix itself, so the built artifact is still liboar.a/liboar.so. Nit: this file is missing a trailing newline.

trsonic · 2026-07-28T11:26:48Z

+    $ bazel build <build_target>
+~~~
+
+The `<build_target>` may be `src:oar` for building the `oar` library or `tests/examples/...` for building the example binaries, for example.


With the top-level wrapper in place, consider documenting //:oar (post-rename) as the canonical build target here rather than src:oar — it's the target that carries the public headers.

trsonic · 2026-07-28T11:26:48Z

+
+The `<build_target>` may be `src:oar` for building the `oar` library or `tests/examples/...` for building the example binaries, for example.
+
+Add `--config=arm64` at the end of the `bazel build` command when building for ARM CPUs.


--config=arm64 sets CC=aarch64-linux-gnu-gcc, so it is a Linux cross-compilation config — as written, this instruction breaks native Apple Silicon builds, which need no flag at all. Suggest: "Add --config=arm64 when cross-compiling for aarch64 Linux."

trsonic · 2026-07-28T11:26:48Z

+Add `--config=arm64` at the end of the `bazel build` command when building for ARM CPUs.
+
+Note: the Open Binaural Renderer (`obr`) is enabled by default. To disable it,
+defines `OAR_ENABLE_BINAURALIZER` as `0`:


Typo: "defines" → "define".

trsonic · 2026-07-28T11:35:13Z

+        "pffft.h",
+    ],
+    visibility = ["//visibility:public"],
+)


pffft is only built once: the root MODULE.bazel declares no pffft extension, so no @pffft repository exists in the root module. The only instantiation is in obr's extensions.bzl, where build_file = Label("//external:pffft.BUILD") resolves within the @obr module to src/renderer/obr/obr_capi/obr/external/pffft.BUILD. The top-level external/pffft.BUILD added by this PR is referenced by nothing — it should be deleted (along with the then-empty external/ directory).

In metadata_constant_polar_positions_create(), the else branch that warns about an unsupported Cartesian animation type left `cartesian_pos` uninitialized and then fell through to cartesian_to_polar_sector_float32(), converting an indeterminate value. Zero it before the warning so the fallback position is the origin. This is a behavior fix rather than a build-system change; it is kept in its own commit so it stays visible when bisecting.

Dropping these includes was correct against the base this branch started from, but the LFE filter added in #15 uses tanf(), M_PI, sqrt() and memset() in this file, so a build with -DOAR_ENABLE_HOA_LFE=ON no longer compiles without them. It also left `#define _USE_MATH_DEFINES` with no <math.h> following it, which is what that define exists for on MSVC.

@obr

- `-Wa,--gsframe=no` is a GNU-assembler flag. clang's integrated assembler rejects it ("unsupported argument '--gsframe=no' to option '-Wa,'"), so the build failed outright on macOS, and MSVC does not know it either. Older GNU assemblers also reject the `=no` value form. Move it out of the unconditional `build` lines into an opt-in `--config=no_sframe` so the GCC + LLD combination that needs it can ask for it and no other toolchain trips over it. Same change in obr's nested .bazelrc. - Ignore the CMake output directories. The README's first build instruction is `cmake -B build`, and `bazel build //...` then fails trying to load abseil's fetched sources under build/_deps. - Point obr's pffft extension at obr's own BUILD shim. The bare `Label("//external:pffft.BUILD")` resolved against the *main* repository, not against @obr where extensions.bzl lives, which is why the shim had to be duplicated at the OAR root for the build to work at all. Qualifying it as `@obr//external:pffft.BUILD` makes it resolve inside the module, so the root copy can go and pffft keeps its single BUILD file in the obr subtree where the rest of obr's external shims live. - Fix two typos and three missing trailing newlines.

The CMake project and target are already named `oar` (`set(PROJECT_N oar)`); the `lib` prefix only ever appears in the built artifact name, which cc_library still adds on its own, so //:oar continues to produce liboar.a and liboar.so. It also matches the nested module's `module(name = "obr")`. Worth settling in this PR rather than later: the module name becomes public API for downstream `bazel_dep(name = ...)` consumers the moment anyone depends on it.

#15 added the OAR_ENABLE_HOA_LFE CMake option and a test that exercises the 120 Hz LFE low-pass, neither of which the Bazel build knew about. `--define OAR_ENABLE_HOA_LFE=1` now sets DISABLE_LFE_HOA=0 on ae_rdr, whose header defaults it to 1, matching what the CMake option does globally. The test target is gated with target_compatible_with, so `bazel build //...` skips it instead of building a test whose subject was compiled out.

`src:oar` is the internal library; //:oar is the target that carries the public headers, so document that one. `--config=arm64` sets CC to aarch64-linux-gnu-gcc, making it a Linux cross-compilation config rather than the "building for ARM CPUs" flag it was described as — as written it broke native Apple Silicon builds, which need no flag at all. Document the two remaining knobs, --config=no_sframe and --define OAR_ENABLE_HOA_LFE=1, and fix a typo.

The Bazel build had no CI coverage at all, while CMake gets three operating systems and both library types. That is how `-Wa,--gsframe=no`, which breaks every clang and MSVC build, stayed in this branch through four rounds of review. Mirrors ci-cmake.yml: same three-OS matrix, same triggers, `bazel build //...` followed by `bazel test` over the examples, which are declared cc_test so a newly added example runs without the workflow needing an edit. The HOA LFE path gets a second pass because it is compiled out by default and its test target is skipped without the flag; only that test's dependency cone is rebuilt with the define. Bazelisk is preinstalled on the GitHub-hosted runner images and reads the version pinned in .bazelversion, so there is no toolchain setup step and no third-party action. No result cache either: if a cold build per job turns out to be too slow, a disk cache is the obvious follow-up.

`--define` silently ignores values with no matching config_setting, so `--define OAR_ENABLE_HOA_LFE=ON` enabled nothing while looking like it had. Recognize the common CMake spellings (ON/OFF, TRUE/FALSE, YES/NO) for both options via bazel_skylib's config_setting_group.

The root .bazelrc duplicated the vendored obr tree's .bazelrc nearly wholesale, so every toolchain fix had to be made twice and the two copies could silently drift. Move the shared flags into one toolchain.bazelrc, imported by both files, and drop the no_sframe `--cxxopt` duplicate: `--copt` already reaches C++ compile actions.

CMake unconditionally compiles with -O2, but Bazel's default fastbuild mode is -O0, so the documented `bazel build` shipped a materially slower render path. Set `-c opt` as the default; `-c dbg` remains available on the command line.

Every test function returned void and bailed out early on error, and main unconditionally returned 0, so the binary always exited successfully and CI could not detect a failure. Return -1 from each failed test and propagate it through main's exit code, as the other examples already do.

Bazel applies module overrides only in the root module, so oar's local_path_override for the vendored obr module does not propagate to consumers, and neither do the toolchain flags obr needs. Describe the git_override and .bazelrc setup a consuming project must repeat.

all points addressed in the new batch of commits

trsonic

Approved on behalf of @yeroro

yeroro requested review from trsonic and yilun-zhangs July 27, 2026 09:20

jingbo-marquis reviewed Jul 27, 2026

View reviewed changes

jingbo-marquis reviewed Jul 28, 2026

View reviewed changes

trsonic previously requested changes Jul 28, 2026

View reviewed changes

yeroro and others added 15 commits August 20, 2026 01:52

Supports building with the Bazel building system

3583661

Removes obr's redundant licensing info

47beea7

Git to ignore folders created by Bazel

feb3aad

Defaults Bazel builds to optimized compilation

7c9a676

CMake unconditionally compiles with -O2, but Bazel's default fastbuild mode is -O0, so the documented `bazel build` shipped a materially slower render path. Set `-c opt` as the default; `-c dbg` remains available on the command line.

trsonic force-pushed the bazel branch from 0468beb to ea0f77c Compare August 20, 2026 01:09

trsonic requested review from jingbo-marquis and trsonic August 20, 2026 01:14

jingbo-marquis approved these changes Aug 20, 2026

View reviewed changes

trsonic approved these changes Aug 20, 2026

View reviewed changes

trsonic merged commit dfe4c57 into main Aug 20, 2026
9 checks passed

trsonic deleted the bazel branch August 20, 2026 08:37

		build --copt="-Wa,--gsframe=no"
		build --cxxopt="-Wa,--gsframe=no"


		The `<build_target>` may be `src:oar` for building the `oar` library or `tests/examples/...` for building the example binaries, for example.

		Add `--config=arm64` at the end of the `bazel build` command when building for ARM CPUs.

Uh oh!

Conversation

yeroro commented Jul 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trsonic commented Jul 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trsonic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants