Skip to content

Android native: tighten Oboe build, fix getLastError race, cut audio-thread syscalls - #208

Merged
winnerspiros merged 2 commits into
masterfrom
copilot/investigate-failed-build-issues
Apr 19, 2026
Merged

Android native: tighten Oboe build, fix getLastError race, cut audio-thread syscalls#208
winnerspiros merged 2 commits into
masterfrom
copilot/investigate-failed-build-issues

Conversation

Copilot AI commented Apr 19, 2026

Copy link
Copy Markdown

Follow-up to the Oboe/Veldrid integration: apply the deferred optimizations from the previous PR and harden one race-y P/Invoke path discovered on a second pass. No game logic touched.

CMake (osu.Android/Native/CMakeLists.txt)

  • Strip -ffast-math from Oboe's compile options (kept on our own bridge). Avoids __FINITE_MATH_ONLY__ symbol-version drift against system libm.
  • OBOE_BUILD_TESTS / EXAMPLES / DOCS = OFF and BUILD_TESTING = OFF — FetchContent stops compiling artifacts we never ship.
  • GIT_SHALLOW TRUE on the Oboe FetchContent_Declare for faster CI checkout.

Audio bridge (oboe_bridge.{h,cpp})

  • Explicit #include <unistd.h> next to <android/performance_hint.h> so gettid() no longer rides on transitive Oboe/NDK includes.
  • Throttle updateLatency() to every 256 callbacks while leaving LatencyTuner::tune() at 128 — halves AAudio syscall pressure on the audio thread.
  • Fix TOCTOU in nOboeGetLastErrorMessage: previous code returned lastError_.c_str() after releasing the mutex, so a concurrent Oboe error callback could mutate the std::string before the P/Invoke marshaller copied it. getLastError() now returns by value (copy under lock); the C export stages it into a thread_local snapshot whose lifetime covers the marshal step. Managed ABI unchanged.
// oboe_bridge.cpp — race-safe export
OSU_EXPORT const char* nOboeGetLastErrorMessage(intptr_t ptr) {
    auto* bridge = reinterpret_cast<OboeBridge*>(ptr);
    if (!bridge) return nullptr;
    thread_local std::string snapshot;
    snapshot = bridge->getLastError();   // copy under mutex, by value
    return snapshot.empty() ? nullptr : snapshot.c_str();
}

Not touched

vulkan_bridge.cpp (runs once at startup), the C# wrapper, and both submodule forks — none need changes for these wins.

Copilot AI and others added 2 commits April 19, 2026 18:23
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/bfd3fec5-2132-4425-949d-b693a7eee8ba

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/9b5aafe3-d606-42af-93bc-8af83d6a31f2

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
@gitar-bot

gitar-bot Bot commented Apr 19, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@winnerspiros
winnerspiros marked this pull request as ready for review April 19, 2026 18:37
Copilot AI review requested due to automatic review settings April 19, 2026 18:37
@winnerspiros
winnerspiros merged commit 6f5a1e0 into master Apr 19, 2026
1 of 15 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes and hardens the Android native audio integration by reducing unnecessary Oboe build work, lowering audio-thread syscall pressure, and fixing a race in the native-to-managed error-message path.

Changes:

  • CMake: disable Oboe tests/examples/docs and use shallow FetchContent to speed up CI/build; adjust Oboe compile options to avoid problematic flags.
  • Audio callback: throttle updateLatency() to every 256 callbacks while keeping LatencyTuner::tune() at 128.
  • Error ABI hardening: make getLastError() return by value (copy under lock) and return a stable thread_local snapshot from nOboeGetLastErrorMessage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
osu.Android/Native/CMakeLists.txt Disables non-shipping Oboe artifacts, shallow-fetches Oboe, and patches Oboe compile flags for compatibility.
osu.Android/Native/oboe_bridge.h Changes getLastError() to return std::string by value with updated contract comment.
osu.Android/Native/oboe_bridge.cpp Fixes AllowedCapturePolicy enum, reduces audio-thread syscalls, and makes last-error export race-safe via thread_local snapshot.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +528 to +530
// Explicit dependency for gettid() used in nADPFCreateSession below — do not
// rely on transitive includes from Oboe / NDK headers, which may change.
#include <unistd.h>

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

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

<unistd.h> is already included at the top of this file, so the new mid-file #include <unistd.h> is redundant and the “don’t rely on transitive includes” comment is now misleading. Consider removing the duplicate include (and ideally keeping all includes in one place).

Suggested change
// Explicit dependency for gettid() used in nADPFCreateSession below — do not
// rely on transitive includes from Oboe / NDK headers, which may change.
#include <unistd.h>

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +40
/// Returns a copy of the most recent error message under lock. We return
/// by value (not a pointer to internal storage) so callers can't observe a
/// torn or freed `std::string` if another thread mutates `lastError_`
/// concurrently (Oboe error callbacks fire from an internal thread).
std::string getLastError() const;

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

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

oboe_bridge.h uses std::string in the public API (getLastError()) but the header doesn’t explicitly include <string>. It currently works only if a transitive include happens to pull it in; please add #include <string> to make the header self-contained.

Copilot uses AI. Check for mistakes.

uint32_t count = callbackCount_.fetch_add(1, std::memory_order_relaxed);

// LatencyTuner once every 128 callbacks (~1.5s @ 192 burst, 48 kHz).

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

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

The new comment says “~1.5s @ 192 burst, 48 kHz”, but 128 callbacks * 192 frames / 48kHz is ~0.51s (and the callback frame count may not even be fixed since framesPerDataCallback is unspecified). Please correct or soften the timing estimate to avoid misleading future readers.

Suggested change
// LatencyTuner once every 128 callbacks (~1.5s @ 192 burst, 48 kHz).
// Run the LatencyTuner once every 128 callbacks. The wall-clock interval
// depends on the callback frame count and sample rate (for example, at
// 192 frames/callback and 48 kHz this is about 0.5s).

Copilot uses AI. Check for mistakes.
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.

3 participants