fix(android): client loads again on Android < 13 — dlsym the API-33 render callback
ci / rust (push) Failing after 41s
ci / web (push) Successful in 52s
ci / docs-site (push) Successful in 53s
decky / build-publish (push) Successful in 18s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 7s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 30s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 12s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 6s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 7s
apple / swift (push) Successful in 1m11s
ci / bench (push) Successful in 6m46s
apple / screenshots (push) Successful in 5m45s
android / android (push) Successful in 12m38s
deb / build-publish (push) Successful in 11m58s
arch / build-publish (push) Successful in 14m5s
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Successful in 15m52s
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Successful in 13m33s
docker / deploy-docs (push) Successful in 7s
ci / rust (push) Failing after 41s
ci / web (push) Successful in 52s
ci / docs-site (push) Successful in 53s
decky / build-publish (push) Successful in 18s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 7s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 30s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 12s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 6s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 7s
apple / swift (push) Successful in 1m11s
ci / bench (push) Successful in 6m46s
apple / screenshots (push) Successful in 5m45s
android / android (push) Successful in 12m38s
deb / build-publish (push) Successful in 11m58s
arch / build-publish (push) Successful in 14m5s
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Successful in 15m52s
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Successful in 13m33s
docker / deploy-docs (push) Successful in 7s
0.9.0's HUD display stage hard-linked AMediaCodec_setOnFrameRenderedCallback via
ndk-sys believing it API 26; the symbol is API 33 ("Available since Android T").
A cdylib links fine with the dangling import, so it only exploded at
System.loadLibrary on every pre-13 device: UnsatisfiedLinkError, then every
NativeBridge touch throws NoClassDefFoundError — surfacing as "Identity
unavailable: io.unom.punktfunk.kit.NativeBridge", a dead pair button, and no
discovery (reported on a Y700 / Android 12; 13+ devices unaffected).
- decode::install_render_callback now dlsym-resolves the entry point from
libmediandk.so, mirroring try_set_frame_rate; on API < 33 the HUD simply has
no display stage (the pre-0.9.0 behaviour) and the .so loads.
- New scripts/ci/check-android-jni-imports.sh, wired as checkJniImports* gradle
tasks the APK build depends on: fails the build if libpunktfunk_android.so
imports any symbol absent from the NDK's API-28 stubs — `--platform 28` never
enforced this (cdylib links permit undefined symbols), despite the old
comment's claim. Verified: 3 ABIs clean at the 28 floor, and the check flags
the known API-28 symbols when pointed at a 27 floor.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -91,9 +91,12 @@ fun registerCargoNdk(taskName: String, release: Boolean) =
|
|||||||
val cmd = mutableListOf(
|
val cmd = mutableListOf(
|
||||||
"$cargoBin/cargo", "ndk",
|
"$cargoBin/cargo", "ndk",
|
||||||
"-t", "arm64-v8a", "-t", "armeabi-v7a", "-t", "x86_64",
|
"-t", "arm64-v8a", "-t", "armeabi-v7a", "-t", "x86_64",
|
||||||
// Link against the minSdk-28 sysroot: libaaudio (API 26) is present, and building at the
|
// Link against the minSdk-28 sysroot (libaaudio, API 26, is present). NOTE: this does
|
||||||
// floor makes the linker reject any accidental >28 hard import (the one API-30 call we
|
// NOT reject an accidental >28 hard import — a cdylib link permits undefined symbols,
|
||||||
// make, ANativeWindow_setFrameRate, is dlsym-resolved — see decode::try_set_frame_rate).
|
// which then fail at System.loadLibrary on every device below the symbol's API level
|
||||||
|
// (the 0.9.0 Android-≤12 regression). The checkJniImports* task after this build is
|
||||||
|
// what actually enforces the floor; >28 entry points must be dlsym-resolved (see
|
||||||
|
// decode::try_set_frame_rate, decode::install_render_callback, adpf).
|
||||||
"--platform", "28",
|
"--platform", "28",
|
||||||
"-o", file("src/main/jniLibs").absolutePath,
|
"-o", file("src/main/jniLibs").absolutePath,
|
||||||
"build", "-p", "punktfunk-client-android",
|
"build", "-p", "punktfunk-client-android",
|
||||||
@@ -102,8 +105,28 @@ fun registerCargoNdk(taskName: String, release: Boolean) =
|
|||||||
commandLine(cmd)
|
commandLine(cmd)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Post-link floor check: every undefined symbol in the built .so must exist in the API-28 stubs,
|
||||||
|
// else System.loadLibrary fails on devices at the minSdk floor (see the script header for the
|
||||||
|
// 0.9.0 incident this guards against). Runs right after its cargo-ndk task; the APK build depends
|
||||||
|
// on this task (not the cargo one directly), so a violation fails the build, local and CI alike.
|
||||||
|
fun registerCheckJniImports(taskName: String, cargoTask: TaskProvider<Exec>) =
|
||||||
|
tasks.register<Exec>(taskName) {
|
||||||
|
group = "rust"
|
||||||
|
description = "verify libpunktfunk_android.so imports stay within the API-28 floor"
|
||||||
|
dependsOn(cargoTask)
|
||||||
|
workingDir = repoRoot
|
||||||
|
commandLine(
|
||||||
|
"sh", File(repoRoot, "scripts/ci/check-android-jni-imports.sh").absolutePath,
|
||||||
|
"${androidSdkDir()}/ndk/$ndkVer",
|
||||||
|
file("src/main/jniLibs").absolutePath,
|
||||||
|
"28",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
val cargoNdkDebug = registerCargoNdk("cargoNdkDebug", release = false)
|
val cargoNdkDebug = registerCargoNdk("cargoNdkDebug", release = false)
|
||||||
val cargoNdkRelease = registerCargoNdk("cargoNdkRelease", release = true)
|
val cargoNdkRelease = registerCargoNdk("cargoNdkRelease", release = true)
|
||||||
|
val checkJniImportsDebug = registerCheckJniImports("checkJniImportsDebug", cargoNdkDebug)
|
||||||
|
val checkJniImportsRelease = registerCheckJniImports("checkJniImportsRelease", cargoNdkRelease)
|
||||||
|
|
||||||
afterEvaluate {
|
afterEvaluate {
|
||||||
// `-PskipRustBuild` skips the cargo-ndk native build — for JVM-only tasks (the Roborazzi
|
// `-PskipRustBuild` skips the cargo-ndk native build — for JVM-only tasks (the Roborazzi
|
||||||
@@ -120,8 +143,9 @@ afterEvaluate {
|
|||||||
// debug build); only the cargo profile changes. `-PrustDebug` restores a debug-profile native
|
// debug build); only the cargo profile changes. `-PrustDebug` restores a debug-profile native
|
||||||
// build for the rare session that actually steps through Rust.
|
// build for the rare session that actually steps through Rust.
|
||||||
if (!project.hasProperty("skipRustBuild")) {
|
if (!project.hasProperty("skipRustBuild")) {
|
||||||
val debugRust = if (project.hasProperty("rustDebug")) cargoNdkDebug else cargoNdkRelease
|
val debugRust =
|
||||||
|
if (project.hasProperty("rustDebug")) checkJniImportsDebug else checkJniImportsRelease
|
||||||
tasks.named("preDebugBuild").configure { dependsOn(debugRust) }
|
tasks.named("preDebugBuild").configure { dependsOn(debugRust) }
|
||||||
tasks.named("preReleaseBuild").configure { dependsOn(cargoNdkRelease) }
|
tasks.named("preReleaseBuild").configure { dependsOn(checkJniImportsRelease) }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -43,15 +43,20 @@ tracing = { version = "0.1", default-features = false, features = ["std", "log"]
|
|||||||
# Pure-Rust FFI to libmediandk/libnativewindow/libaaudio — no C++/libc++_shared to bundle. Decode +
|
# Pure-Rust FFI to libmediandk/libnativewindow/libaaudio — no C++/libc++_shared to bundle. Decode +
|
||||||
# audio run entirely in Rust on native threads (the "no async on the hot path" invariant).
|
# audio run entirely in Rust on native threads (the "no async on the hot path" invariant).
|
||||||
# api-level-28 matches the app's minSdk floor (Android 9). AAudio (26), AMediaCodec (21) and
|
# api-level-28 matches the app's minSdk floor (Android 9). AAudio (26), AMediaCodec (21) and
|
||||||
# ANativeWindow_setBuffersDataSpace (28) are all ≤28; the one API-30 call we make
|
# ANativeWindow_setBuffersDataSpace (28) are all ≤28; every call above the floor —
|
||||||
# (ANativeWindow_setFrameRate) is dlsym-resolved at runtime (see decode::try_set_frame_rate), not
|
# ANativeWindow_setFrameRate (30), …WithChangeStrategy (31), AMediaCodec_setOnFrameRenderedCallback
|
||||||
# linked, so the .so still loads on API 28/29.
|
# (33), the ADPF hints — is dlsym-resolved at runtime (decode::try_set_frame_rate,
|
||||||
|
# decode::install_render_callback, adpf), never linked, so the .so still loads on API 28+.
|
||||||
ndk = { version = "0.9", features = ["media", "audio", "nativewindow", "api-level-28"] }
|
ndk = { version = "0.9", features = ["media", "audio", "nativewindow", "api-level-28"] }
|
||||||
# Raw FFI for the one AMediaCodec entry point the `ndk` wrapper lacks:
|
# Raw FFI *types* for the one AMediaCodec entry point the `ndk` wrapper lacks:
|
||||||
# `AMediaCodec_setOnFrameRenderedCallback` (API 26, under the minSdk-28 floor ⇒ hard-linked) — the
|
# `AMediaCodec_setOnFrameRenderedCallback` — the per-frame render-timestamp callback behind the
|
||||||
# per-frame render-timestamp callback behind the HUD's `display` stage (see decode::DisplayTracker).
|
# HUD's `display` stage (see decode::DisplayTracker). The symbol is API 33 ("Available since
|
||||||
# Reaching it needs the codec's raw pointer, which is why the workspace pins `ndk` to the vendored
|
# Android T"), ABOVE the minSdk-28 floor, so it is dlsym-resolved at runtime
|
||||||
# copy whose only patch makes `MediaCodec::as_ptr` public (vendor/ndk, wired in the root Cargo.toml).
|
# (decode::install_render_callback), NEVER hard-linked: 0.9.0 hard-linked it and
|
||||||
|
# `System.loadLibrary` failed on every pre-Android-13 device. Only ndk-sys's pointer/typedef
|
||||||
|
# types are referenced, which creates no import. Reaching it needs the codec's raw pointer, which
|
||||||
|
# is why the workspace pins `ndk` to the vendored copy whose only patch makes `MediaCodec::as_ptr`
|
||||||
|
# public (vendor/ndk, wired in the root Cargo.toml).
|
||||||
ndk-sys = { version = "0.6", features = ["media"] }
|
ndk-sys = { version = "0.6", features = ["media"] }
|
||||||
# setpriority/gettid to raise the decode thread toward URGENT_DISPLAY (see decode::boost_thread_priority).
|
# setpriority/gettid to raise the decode thread toward URGENT_DISPLAY (see decode::boost_thread_priority).
|
||||||
libc = "0.2"
|
libc = "0.2"
|
||||||
|
|||||||
@@ -449,27 +449,49 @@ impl DisplayTracker {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Register [`on_frame_rendered`] on the codec (`AMediaCodec_setOnFrameRenderedCallback`, API 26 —
|
/// Register [`on_frame_rendered`] on the codec (`AMediaCodec_setOnFrameRenderedCallback`,
|
||||||
/// under the minSdk-28 floor, so hard-linked via `ndk-sys`; the `ndk` wrapper has no binding, which
|
/// **API 33** — "Available since Android T" per the NDK header; only the *Java* listener dates
|
||||||
/// is what the vendored crate's public `as_ptr` patch is for). Returns the userdata pointer holding
|
/// back further). That sits above the API-28 floor, so the entry point is dlsym-resolved at
|
||||||
/// a leaked `Arc<DisplayTracker>` refcount; the caller MUST reclaim it with
|
/// runtime like [`try_set_frame_rate`] — hard-linking it (as 0.9.0 shipped) made
|
||||||
/// [`release_render_callback`] AFTER dropping the codec (`AMediaCodec_delete` is what guarantees no
|
/// `System.loadLibrary` fail on every pre-Android-13 device, taking down all of `NativeBridge`.
|
||||||
/// further callback can fire). `None` (nothing to reclaim) if the platform refused — the HUD then
|
/// The `ndk` wrapper has no binding and the call needs the raw codec pointer, which is what the
|
||||||
/// simply has no `display` stage, exactly the pre-callback behaviour.
|
/// vendored crate's public `as_ptr` patch is for. Returns the userdata pointer holding a leaked
|
||||||
|
/// `Arc<DisplayTracker>` refcount; the caller MUST reclaim it with [`release_render_callback`]
|
||||||
|
/// AFTER dropping the codec (`AMediaCodec_delete` is what guarantees no further callback can
|
||||||
|
/// fire). `None` (nothing to reclaim) if the symbol is absent (API < 33) or the platform refused —
|
||||||
|
/// the HUD then simply has no `display` stage, exactly the pre-callback behaviour.
|
||||||
fn install_render_callback(
|
fn install_render_callback(
|
||||||
codec: &MediaCodec,
|
codec: &MediaCodec,
|
||||||
tracker: &Arc<DisplayTracker>,
|
tracker: &Arc<DisplayTracker>,
|
||||||
) -> Option<*const DisplayTracker> {
|
) -> Option<*const DisplayTracker> {
|
||||||
|
// media_status_t AMediaCodec_setOnFrameRenderedCallback(
|
||||||
|
// AMediaCodec*, AMediaCodecOnFrameRendered, void*) (API 33)
|
||||||
|
type SetOnFrameRenderedFn = unsafe extern "C" fn(
|
||||||
|
*mut ndk_sys::AMediaCodec,
|
||||||
|
ndk_sys::AMediaCodecOnFrameRendered,
|
||||||
|
*mut c_void,
|
||||||
|
) -> ndk_sys::media_status_t;
|
||||||
|
// SAFETY: `dlopen` of `libmediandk.so`, which the `ndk` media wrapper already links — always
|
||||||
|
// mapped, so this only bumps its refcount (never closed — process-lifetime handle). `dlsym`
|
||||||
|
// returns null when the symbol is absent (device below API 33), checked before transmuting the
|
||||||
|
// non-null pointer to its fn-pointer type.
|
||||||
|
let set_on_frame_rendered = unsafe {
|
||||||
|
let lib = libc::dlopen(c"libmediandk.so".as_ptr(), libc::RTLD_NOW);
|
||||||
|
if lib.is_null() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let sym = libc::dlsym(lib, c"AMediaCodec_setOnFrameRenderedCallback".as_ptr());
|
||||||
|
if sym.is_null() {
|
||||||
|
log::info!("decode: no render callback on this API level (<33) — no display stage");
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
std::mem::transmute::<*mut c_void, SetOnFrameRenderedFn>(sym)
|
||||||
|
};
|
||||||
let ud = Arc::into_raw(tracker.clone());
|
let ud = Arc::into_raw(tracker.clone());
|
||||||
// SAFETY: `codec.as_ptr()` is the live codec this thread owns; `ud` outlives the registration
|
// SAFETY: `codec.as_ptr()` is the live codec this thread owns; `ud` outlives the registration
|
||||||
// (reclaimed only after the codec is deleted, per this function's contract).
|
// (reclaimed only after the codec is deleted, per this function's contract).
|
||||||
let status = unsafe {
|
let status =
|
||||||
ndk_sys::AMediaCodec_setOnFrameRenderedCallback(
|
unsafe { set_on_frame_rendered(codec.as_ptr(), Some(on_frame_rendered), ud as *mut c_void) };
|
||||||
codec.as_ptr(),
|
|
||||||
Some(on_frame_rendered),
|
|
||||||
ud as *mut c_void,
|
|
||||||
)
|
|
||||||
};
|
|
||||||
if status == ndk_sys::media_status_t::AMEDIA_OK {
|
if status == ndk_sys::media_status_t::AMEDIA_OK {
|
||||||
Some(ud)
|
Some(ud)
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
Executable
+74
@@ -0,0 +1,74 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# Post-link floor check for libpunktfunk_android.so: every undefined dynamic symbol must be
|
||||||
|
# defined in the NDK's minSdk-level stub libraries, for every built ABI.
|
||||||
|
#
|
||||||
|
# Why this exists: a Rust cdylib links fine with dangling undefined symbols (lld only rejects them
|
||||||
|
# under --no-undefined, which rustc does not pass), so a hard import of an above-floor NDK entry
|
||||||
|
# point sails through `cargo ndk --platform 28` and only explodes at `System.loadLibrary` on every
|
||||||
|
# device below the symbol's introduction level — 0.9.0 shipped exactly this
|
||||||
|
# (AMediaCodec_setOnFrameRenderedCallback, API 33) and bricked the client on all pre-Android-13
|
||||||
|
# devices. Above-floor entry points must be dlsym-resolved at runtime instead (see
|
||||||
|
# decode::try_set_frame_rate / decode::install_render_callback / adpf). This script makes the
|
||||||
|
# violation a build failure. Wired into clients/android/kit/build.gradle.kts after each cargo-ndk
|
||||||
|
# build (local + CI).
|
||||||
|
#
|
||||||
|
# Usage: check-android-jni-imports.sh <ndk-dir> <jniLibs-dir> <api-level>
|
||||||
|
set -eu
|
||||||
|
|
||||||
|
NDK=$1
|
||||||
|
JNILIBS=$2
|
||||||
|
API=$3
|
||||||
|
|
||||||
|
# Host-agnostic prebuilt dir (linux-x86_64 on CI and the dev box, darwin-* on a Mac).
|
||||||
|
PREBUILT=$(echo "$NDK"/toolchains/llvm/prebuilt/*)
|
||||||
|
NM="$PREBUILT/bin/llvm-nm"
|
||||||
|
[ -x "$NM" ] || { echo "check-android-jni-imports: llvm-nm not found under $NDK" >&2; exit 1; }
|
||||||
|
|
||||||
|
triple_for_abi() {
|
||||||
|
case "$1" in
|
||||||
|
arm64-v8a) echo aarch64-linux-android ;;
|
||||||
|
armeabi-v7a) echo arm-linux-androideabi ;;
|
||||||
|
x86_64) echo x86_64-linux-android ;;
|
||||||
|
x86) echo i686-linux-android ;;
|
||||||
|
*) echo "" ;;
|
||||||
|
esac
|
||||||
|
}
|
||||||
|
|
||||||
|
TMP=$(mktemp -d)
|
||||||
|
trap 'rm -rf "$TMP"' EXIT
|
||||||
|
|
||||||
|
checked=0
|
||||||
|
fail=0
|
||||||
|
for so in "$JNILIBS"/*/libpunktfunk_android.so; do
|
||||||
|
[ -f "$so" ] || continue
|
||||||
|
abi=$(basename "$(dirname "$so")")
|
||||||
|
triple=$(triple_for_abi "$abi")
|
||||||
|
if [ -z "$triple" ]; then
|
||||||
|
echo "check-android-jni-imports: unknown ABI dir '$abi' — extend triple_for_abi" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
stubs="$PREBUILT/sysroot/usr/lib/$triple/$API"
|
||||||
|
[ -d "$stubs" ] || { echo "check-android-jni-imports: no stub dir $stubs" >&2; exit 1; }
|
||||||
|
|
||||||
|
# Strong undefined imports only ('U'; weak 'w'/'v' resolve to null and are fine), and the
|
||||||
|
# stubs' exports, both with the @VERSION / @@VERSION suffix stripped.
|
||||||
|
"$NM" -D --undefined-only "$so" | awk '$1 == "U" { print $2 }' | sed 's/@.*//' | sort -u \
|
||||||
|
> "$TMP/undef"
|
||||||
|
for stub in "$stubs"/*.so; do
|
||||||
|
"$NM" -D --defined-only "$stub" 2>/dev/null | awk '{ print $NF }' | sed 's/@.*//'
|
||||||
|
done | sort -u > "$TMP/defined"
|
||||||
|
|
||||||
|
missing=$(comm -23 "$TMP/undef" "$TMP/defined")
|
||||||
|
if [ -n "$missing" ]; then
|
||||||
|
echo "check-android-jni-imports: $abi libpunktfunk_android.so imports symbols absent from" >&2
|
||||||
|
echo "the API-$API sysroot — System.loadLibrary would fail on devices at the minSdk floor." >&2
|
||||||
|
echo "dlsym-resolve these at runtime instead of hard-linking them:" >&2
|
||||||
|
echo "$missing" | sed 's/^/ /' >&2
|
||||||
|
fail=1
|
||||||
|
fi
|
||||||
|
checked=$((checked + 1))
|
||||||
|
done
|
||||||
|
|
||||||
|
[ "$checked" -gt 0 ] || { echo "check-android-jni-imports: no libpunktfunk_android.so under $JNILIBS" >&2; exit 1; }
|
||||||
|
[ "$fail" -eq 0 ] && echo "check-android-jni-imports: $checked ABI(s) clean at the API-$API floor"
|
||||||
|
exit "$fail"
|
||||||
Reference in New Issue
Block a user