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

@@ -23,8 +23,6 @@ This software module is out of scope and not part of the IAMF Final Deliverable.
 **/

 #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).

yeroro added 3 commits July 27, 2026 16:46

Supports building with the Bazel building system

b6ce6d6

Removes obr's redundant licensing info

3240c80

Git to ignore folders created by Bazel

0468beb

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 requested changes Jul 28, 2026

View reviewed changes

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

		@@ -23,8 +23,6 @@ This software module is out of scope and not part of the IAMF Final Deliverable.
		**/

		#define _USE_MATH_DEFINES


		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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants