diff --git a/src/renderer/obr/obr_capi/obr/CMakeLists.txt b/src/renderer/obr/obr_capi/obr/CMakeLists.txt index 083e73c..383376a 100644 --- a/src/renderer/obr/obr_capi/obr/CMakeLists.txt +++ b/src/renderer/obr/obr_capi/obr/CMakeLists.txt @@ -154,6 +154,7 @@ set(SourceFiles obr/audio_buffer/simd_utils.cc obr/audio_buffer/simd_utils.h obr/common/ambisonic_utils.h + obr/common/atomic_rotation.h obr/common/constants.h obr/common/status_macros.h obr/common/misc_math.h @@ -315,6 +316,10 @@ add_executable(ambisonic_utils_test obr/common/tests/ambisonic_utils_test.cc) target_link_libraries(ambisonic_utils_test PRIVATE obr GTest::gtest_main) add_test(NAME ambisonic_utils_test COMMAND ambisonic_utils_test) +add_executable(atomic_rotation_test obr/common/tests/atomic_rotation_test.cc) +target_link_libraries(atomic_rotation_test PRIVATE obr GTest::gtest_main) +add_test(NAME atomic_rotation_test COMMAND atomic_rotation_test) + add_executable(misc_math_test obr/common/tests/misc_math_test.cc) target_link_libraries(misc_math_test PRIVATE obr GTest::gtest_main) add_test(NAME misc_math_test COMMAND misc_math_test) @@ -347,6 +352,7 @@ set_tests_properties( ambisonic_rotator_test spherical_angle_test ambisonic_utils_test + atomic_rotation_test misc_math_test test_util_test PROPERTIES LABELS "obr" diff --git a/src/renderer/obr/obr_capi/obr/obr/common/BUILD b/src/renderer/obr/obr_capi/obr/obr/common/BUILD index b20ef17..1b34057 100644 --- a/src/renderer/obr/obr_capi/obr/obr/common/BUILD +++ b/src/renderer/obr/obr_capi/obr/obr/common/BUILD @@ -6,6 +6,7 @@ cc_library( name = "common", hdrs = [ "ambisonic_utils.h", + "atomic_rotation.h", "constants.h", "misc_math.h", "status_macros.h", diff --git a/src/renderer/obr/obr_capi/obr/obr/common/atomic_rotation.h b/src/renderer/obr/obr_capi/obr/obr/common/atomic_rotation.h new file mode 100644 index 0000000..535bf6c --- /dev/null +++ b/src/renderer/obr/obr_capi/obr/obr/common/atomic_rotation.h @@ -0,0 +1,116 @@ +/* + * Copyright (c) 2026, Alliance for Open Media. All rights reserved + * + * This source code is subject to the terms of the BSD 3-Clause Clear License + * and the Alliance for Open Media Patent License 1.0. If the BSD 3-Clause Clear + * License was not distributed with this source code in the LICENSE file, you + * can obtain it at www.aomedia.org/license/software-license/bsd-3-c-c. If the + * Alliance for Open Media Patent License 1.0 was not distributed with this + * source code in the PATENTS file, you can obtain it at + * www.aomedia.org/license/patent. + */ + +#ifndef OBR_COMMON_ATOMIC_ROTATION_H_ +#define OBR_COMMON_ATOMIC_ROTATION_H_ + +#include +#include + +#include "obr/common/misc_math.h" + +namespace obr { + +/*!\brief A `WorldRotation` written by one or more threads and read by + * another without a lock. + * + * The four quaternion components are atomic floats behind a sequence + * counter that is odd while a write is in progress. `std::atomic` of the + * whole 16-byte quaternion is not guaranteed to be lock-free on every + * platform OBR targets. + * `Store()` waits for a concurrent `Store()` to finish; `Load()` never waits: + * after `kMaxLoadAttempts` attempts that overlap a write it returns the + * caller's previous value. + */ +class AtomicWorldRotation { + public: + /*!\brief The number of times `Load()` reads the components before it + * returns the fallback. + * + * `Load()` runs on the render thread, so it must not wait for a writer: a + * writer preempted while the sequence is odd would keep an unbounded retry + * loop spinning until that writer runs again. An attempt fails only when a + * write overlaps it, so with 4 the fallback needs writes to overlap four + * consecutive attempts. `ObrImpl` passes the rotation it rendered the + * previous block with as the fallback. Any small bound behaves the same + * way; 4 is not tuned against a measurement. + */ + static constexpr int kMaxLoadAttempts = 4; + + /*!\brief Constructs an identity rotation. */ + AtomicWorldRotation() : AtomicWorldRotation(WorldRotation()) {} + + /*!\brief Constructs a rotation holding `rotation`. */ + explicit AtomicWorldRotation(const WorldRotation& rotation) + : w_(rotation.w()), + x_(rotation.x()), + y_(rotation.y()), + z_(rotation.z()) {} + + AtomicWorldRotation(const AtomicWorldRotation&) = delete; + AtomicWorldRotation& operator=(const AtomicWorldRotation&) = delete; + + /*!\brief Publishes `rotation`. Safe to call from several threads. */ + void Store(const WorldRotation& rotation) { + // Make the sequence odd. Another writer holding it odd is storing four + // floats, so the wait is short while that writer runs; it spins for as + // long as that writer is preempted. The acquire on success orders this + // store after the previous writer's, whose final sequence store it reads. + uint32_t sequence = sequence_.load(std::memory_order_relaxed); + do { + while (sequence & 1u) { + sequence = sequence_.load(std::memory_order_relaxed); + } + } while (!sequence_.compare_exchange_weak(sequence, sequence + 1, + std::memory_order_acquire, + std::memory_order_relaxed)); + // Orders the odd sequence before the component stores, so a reader that + // sees a new component also sees a changed sequence. + std::atomic_thread_fence(std::memory_order_release); + w_.store(rotation.w(), std::memory_order_relaxed); + x_.store(rotation.x(), std::memory_order_relaxed); + y_.store(rotation.y(), std::memory_order_relaxed); + z_.store(rotation.z(), std::memory_order_relaxed); + sequence_.store(sequence + 2, std::memory_order_release); + } + + /*!\brief Returns the last stored rotation, or `fallback` when every one of + * `kMaxLoadAttempts` reads overlapped a `Store()`. + */ + WorldRotation Load(const WorldRotation& fallback) const { + for (int attempt = 0; attempt < kMaxLoadAttempts; ++attempt) { + const uint32_t before = sequence_.load(std::memory_order_acquire); + if (before & 1u) continue; + const float w = w_.load(std::memory_order_relaxed); + const float x = x_.load(std::memory_order_relaxed); + const float y = y_.load(std::memory_order_relaxed); + const float z = z_.load(std::memory_order_relaxed); + // Orders the component loads before the second sequence load. + std::atomic_thread_fence(std::memory_order_acquire); + if (sequence_.load(std::memory_order_relaxed) == before) { + return WorldRotation(w, x, y, z); + } + } + return fallback; + } + + private: + std::atomic sequence_{0}; + std::atomic w_; + std::atomic x_; + std::atomic y_; + std::atomic z_; +}; + +} // namespace obr + +#endif // OBR_COMMON_ATOMIC_ROTATION_H_ diff --git a/src/renderer/obr/obr_capi/obr/obr/common/tests/BUILD b/src/renderer/obr/obr_capi/obr/obr/common/tests/BUILD index 1bc40de..6c3de6a 100644 --- a/src/renderer/obr/obr_capi/obr/obr/common/tests/BUILD +++ b/src/renderer/obr/obr_capi/obr/obr/common/tests/BUILD @@ -9,6 +9,15 @@ cc_test( ], ) +cc_test( + name = "atomic_rotation_test", + srcs = ["atomic_rotation_test.cc"], + deps = [ + "//obr/common", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "misc_math_test", srcs = ["misc_math_test.cc"], diff --git a/src/renderer/obr/obr_capi/obr/obr/common/tests/atomic_rotation_test.cc b/src/renderer/obr/obr_capi/obr/obr/common/tests/atomic_rotation_test.cc new file mode 100644 index 0000000..8a2b697 --- /dev/null +++ b/src/renderer/obr/obr_capi/obr/obr/common/tests/atomic_rotation_test.cc @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2026, Alliance for Open Media. All rights reserved + * + * This source code is subject to the terms of the BSD 3-Clause Clear License + * and the Alliance for Open Media Patent License 1.0. If the BSD 3-Clause Clear + * License was not distributed with this source code in the LICENSE file, you + * can obtain it at www.aomedia.org/license/software-license/bsd-3-c-c. If the + * Alliance for Open Media Patent License 1.0 was not distributed with this + * source code in the PATENTS file, you can obtain it at + * www.aomedia.org/license/patent. + */ + +#include "obr/common/atomic_rotation.h" + +#include +#include +#include + +#include "gtest/gtest.h" +#include "obr/common/misc_math.h" + +namespace obr { +namespace { + +// A rotation whose components are `k`, `k + 1`, `k + 2` and `k + 3`, so a +// load that mixes two stores shows as unequal steps. Integers below 2^24 are +// exact in a float. +WorldRotation Numbered(int k) { + return WorldRotation(static_cast(k), static_cast(k + 1), + static_cast(k + 2), static_cast(k + 3)); +} + +bool IsNumbered(const WorldRotation& r) { + return r.x() - r.w() == 1.0f && r.y() - r.w() == 2.0f && + r.z() - r.w() == 3.0f; +} + +const WorldRotation kFallback(-7.0f, -7.0f, -7.0f, -7.0f); + +bool IsFallback(const WorldRotation& r) { + return r.w() == kFallback.w() && r.x() == kFallback.x() && + r.y() == kFallback.y() && r.z() == kFallback.z(); +} + +TEST(AtomicWorldRotationTest, DefaultIsIdentity) { + const AtomicWorldRotation rotation; + const WorldRotation loaded = rotation.Load(kFallback); + EXPECT_EQ(loaded.w(), 1.0f); + EXPECT_EQ(loaded.x(), 0.0f); + EXPECT_EQ(loaded.y(), 0.0f); + EXPECT_EQ(loaded.z(), 0.0f); +} + +TEST(AtomicWorldRotationTest, LoadReturnsTheLastStore) { + AtomicWorldRotation rotation; + rotation.Store(WorldRotation(0.5f, -0.5f, 0.25f, 0.75f)); + rotation.Store(Numbered(10)); + const WorldRotation loaded = rotation.Load(kFallback); + EXPECT_EQ(loaded.w(), 10.0f); + EXPECT_EQ(loaded.x(), 11.0f); + EXPECT_EQ(loaded.y(), 12.0f); + EXPECT_EQ(loaded.z(), 13.0f); +} + +// One writer stores increasing numbered rotations while the test thread +// loads. Every load returns a whole store or the fallback, and the stores it +// returns never go back in time. +TEST(AtomicWorldRotationTest, NoTornLoadsUnderAConcurrentWriter) { + AtomicWorldRotation rotation(Numbered(0)); + std::atomic stop{false}; + std::thread writer([&] { + for (int k = 1; !stop.load(std::memory_order_relaxed); ++k) { + rotation.Store(Numbered(k & ((1 << 20) - 1))); + } + }); + + float last_w = 0.0f; + int torn = 0; + int backwards = 0; + for (int i = 0; i < 200000; ++i) { + const WorldRotation loaded = rotation.Load(kFallback); + if (IsFallback(loaded)) continue; + if (!IsNumbered(loaded)) { + ++torn; + continue; + } + // The writer wraps at 2^20; a smaller value after a large one is a wrap. + if (loaded.w() < last_w && last_w - loaded.w() < (1 << 19)) ++backwards; + last_w = loaded.w(); + } + stop.store(true, std::memory_order_relaxed); + writer.join(); + + EXPECT_EQ(torn, 0); + EXPECT_EQ(backwards, 0); +} + +// Several writers store as fast as they can. `Load()` returns after at most +// `kMaxLoadAttempts` attempts each time, with a whole store or the fallback. +TEST(AtomicWorldRotationTest, LoadIsBoundedUnderAWriteStorm) { + AtomicWorldRotation rotation(Numbered(0)); + std::atomic stop{false}; + std::vector writers; + for (int t = 0; t < 4; ++t) { + writers.emplace_back([&rotation, &stop, t] { + for (int k = t; !stop.load(std::memory_order_relaxed); k += 4) { + rotation.Store(Numbered(k & ((1 << 20) - 1))); + } + }); + } + + int torn = 0; + int fallbacks = 0; + for (int i = 0; i < 100000; ++i) { + const WorldRotation loaded = rotation.Load(kFallback); + if (IsFallback(loaded)) { + ++fallbacks; + } else if (!IsNumbered(loaded)) { + ++torn; + } + } + stop.store(true, std::memory_order_relaxed); + for (auto& writer : writers) writer.join(); + + EXPECT_EQ(torn, 0); + // Whether a load fell back depends on scheduling; record it. + RecordProperty("fallbacks", fallbacks); +} + +} // namespace +} // namespace obr diff --git a/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.cc b/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.cc index 825cb84..80038b6 100644 --- a/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.cc +++ b/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.cc @@ -13,6 +13,7 @@ #include "obr/renderer/obr_impl.h" #include +#include #include #include #include @@ -41,7 +42,6 @@ ObrImpl::ObrImpl(int buffer_size_per_channel, int sampling_rate) sampling_rate_(sampling_rate), head_tracking_enabled_(false), limiter_enabled_(true), - world_rotation_(WorldRotation()), fft_manager_(buffer_size_per_channel_), group_output_(kNumBinauralChannels, buffer_size_per_channel_), planar_input_(0, buffer_size_per_channel_), @@ -179,6 +179,13 @@ void ObrImpl::ProcessPlanar(const float* input, float* output, void ObrImpl::ProcessLocked(const AudioBuffer& input_buffer, AudioBuffer* output_buffer) { + // Read the control state once for the whole block; other threads set it + // without the lock. + const bool head_tracking_enabled = + head_tracking_enabled_.load(std::memory_order_relaxed); + const bool limiter_enabled = limiter_enabled_.load(std::memory_order_relaxed); + latched_world_rotation_ = world_rotation_.Load(latched_world_rotation_); + // Clear the output buffer. output_buffer->Clear(); @@ -229,7 +236,7 @@ void ObrImpl::ProcessLocked(const AudioBuffer& input_buffer, // (May still have passthrough elements processed above.) if (processing_groups_.empty()) { // Apply peak limiter even for passthrough-only configurations. - if (limiter_enabled_) { + if (limiter_enabled) { peak_limiter_->Process(*output_buffer, output_buffer); } return; @@ -238,8 +245,8 @@ void ObrImpl::ProcessLocked(const AudioBuffer& input_buffer, // Process each processing group. for (auto& group : processing_groups_) { group_output_.Clear(); - group.Process(input_buffer, audio_elements_, head_tracking_enabled_, - world_rotation_, &group_output_); + group.Process(input_buffer, audio_elements_, head_tracking_enabled, + latched_world_rotation_, &group_output_); // Add this group's binaural output to the main output buffer. (*output_buffer)[0] += group_output_[0]; @@ -247,7 +254,7 @@ void ObrImpl::ProcessLocked(const AudioBuffer& input_buffer, } // Peak limit the output if enabled. - if (limiter_enabled_) { + if (limiter_enabled) { peak_limiter_->Process(*output_buffer, output_buffer); } } @@ -460,26 +467,20 @@ absl::Status ObrImpl::UpdateObjectChannelPosition(size_t audio_element_index, } void ObrImpl::EnableHeadTracking(bool enable_head_tracking) { - // Acquire mutex: `head_tracking_enabled_` is read by `Process()` on the - // audio thread. - absl::MutexLock lock(&mutex_); - head_tracking_enabled_ = enable_head_tracking; + // No lock: `Process()` reads the flag once per block on the audio thread. + head_tracking_enabled_.store(enable_head_tracking, std::memory_order_relaxed); } void ObrImpl::EnableLimiter(bool enable_limiter) { - // Acquire mutex: `limiter_enabled_` is read by `Process()` on the audio - // thread. - absl::MutexLock lock(&mutex_); - limiter_enabled_ = enable_limiter; + // No lock: `Process()` reads the flag once per block on the audio thread. + limiter_enabled_.store(enable_limiter, std::memory_order_relaxed); } absl::Status ObrImpl::SetHeadRotation(float w, float x, float y, float z) { - // Acquire mutex: `world_rotation_` is read by `Process()` on the audio - // thread, and head rotation updates typically arrive from a sensor thread. - absl::MutexLock lock(&mutex_); - + // No lock: head rotation typically arrives from a sensor thread on every + // block, and `Process()` reads it once per block on the audio thread. // Apply counter-rotation for head tracking. - world_rotation_ = WorldRotation(w, -x, -y, -z); + world_rotation_.Store(WorldRotation(w, -x, -y, -z)); return absl::OkStatus(); } diff --git a/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.h b/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.h index bd479a1..a73ece8 100644 --- a/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.h +++ b/src/renderer/obr/obr_capi/obr/obr/renderer/obr_impl.h @@ -13,6 +13,7 @@ #ifndef OBR_RENDERER_OBR_IMPL_H_ #define OBR_RENDERER_OBR_IMPL_H_ +#include #include #include #include @@ -23,6 +24,7 @@ #include "obr/ambisonic_binaural_decoder/fft_manager.h" #include "obr/ambisonic_binaural_decoder/resampler.h" #include "obr/audio_buffer/audio_buffer.h" +#include "obr/common/atomic_rotation.h" #include "obr/common/misc_math.h" #include "obr/peak_limiter/peak_limiter.h" #include "obr/renderer/audio_element_config.h" @@ -150,6 +152,9 @@ class ObrImpl { float elevation, float distance); /*! \brief Enables or disables head tracking. + * + * Takes no lock, so it can be called from any thread while `Process()` + * runs; the change takes effect at the next `Process()` call. * * \param enable_head_tracking True to enable head tracking, false to disable. */ @@ -188,6 +193,9 @@ class ObrImpl { bool head_locked); /*! \brief Enables or disables the output peak limiter. + * + * Takes no lock, so it can be called from any thread while `Process()` + * runs; the change takes effect at the next `Process()` call. * * \param enable_limiter True to enable the limiter (default), false to * disable. @@ -203,6 +211,12 @@ class ObrImpl { * Y - front (positive front, negative back) * Z - up (positive up, negative down) * + * Takes no lock, so it can be called from any thread, such as a sensor + * thread, while `Process()` runs; the rotation takes effect at the next + * `Process()` call. If a call overlaps the rotation's read at the start of + * `Process()` every time it retries, that block keeps the previous block's + * rotation. + * * @param w * @param x * @param y @@ -264,13 +278,20 @@ class ObrImpl { const int buffer_size_per_channel_; const int sampling_rate_; - bool head_tracking_enabled_; - bool limiter_enabled_; - WorldRotation world_rotation_; + // Control state set from any thread without `mutex_`. `Process()` reads + // each once per call, so a change takes effect at the next block. + std::atomic head_tracking_enabled_; + std::atomic limiter_enabled_; + AtomicWorldRotation world_rotation_; + + // The rotation `Process()` read for its last block, which it keeps when a + // read of `world_rotation_` overlaps a write. Accessed under `mutex_`. + WorldRotation latched_world_rotation_; - // Mutex to protect data accessed in different threads, including the - // group output and planar buffers. `Process()` and `ProcessPlanar()` hold - // it for the whole call. + // Guards the audio elements and processing groups, the planar input and + // output buffers, the group output buffer, the peak limiter and + // `latched_world_rotation_`. `Process()` and `ProcessPlanar()` hold it for + // the whole call; the control state above does not use it. mutable absl::Mutex mutex_; std::vector audio_elements_; diff --git a/src/renderer/obr/obr_capi/obr/obr/renderer/tests/obr_impl_test.cc b/src/renderer/obr/obr_capi/obr/obr/renderer/tests/obr_impl_test.cc index 32a1b35..57c6556 100644 --- a/src/renderer/obr/obr_capi/obr/obr/renderer/tests/obr_impl_test.cc +++ b/src/renderer/obr/obr_capi/obr/obr/renderer/tests/obr_impl_test.cc @@ -12,8 +12,11 @@ #include "obr/renderer/obr_impl.h" +#include +#include #include #include +#include #include #include @@ -1183,5 +1186,41 @@ TEST(ObrImplTest, TestProcessPlanarMatchesProcess) { expect_same_output(30); } +// Renders while a second thread sweeps the head rotation and toggles head +// tracking and the limiter. None of those calls takes the mutex `Process()` +// holds. The test makes no assertion on the output; a ThreadSanitizer build +// checks the concurrent accesses. +TEST(ObrImplTest, TestConcurrentHeadRotationDuringProcessing) { + const size_t kNumFrames = 256; + ObrImpl renderer(kNumFrames, kSamplingRate); + EXPECT_THAT(renderer.AddAudioElement(AudioElementType::k3OA), IsOk()); + EXPECT_THAT(renderer.AddAudioElement(AudioElementType::kLayout7_1_4_ch), + IsOk()); + + AudioBuffer input_buffer(renderer.GetNumberOfInputChannels(), kNumFrames); + for (size_t ch = 0; ch < input_buffer.num_channels(); ++ch) { + GenerateWhiteNoise(0.1f, &input_buffer[ch], static_cast(100 + ch)); + } + AudioBuffer output_buffer(2, kNumFrames); + + std::atomic stop{false}; + std::thread control([&] { + for (int i = 0; !stop.load(std::memory_order_relaxed); ++i) { + const float half_angle = 0.005f * static_cast(i % 1257); + renderer + .SetHeadRotation(std::cos(half_angle), 0.0f, 0.0f, + std::sin(half_angle)) + .IgnoreError(); + renderer.EnableHeadTracking(i % 2 == 0); + renderer.EnableLimiter(i % 3 != 0); + } + }); + for (int block = 0; block < 500; ++block) { + renderer.Process(input_buffer, &output_buffer); + } + stop.store(true, std::memory_order_relaxed); + control.join(); +} + } // namespace } // namespace obr