fix(aaudio): timestamps and buffer sizing (#1165)

Author: roderickvd

CHANGELOG.md

@@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   silently returning an empty list.
 - **AAudio**: Bump MSRV to 1.85.
 - **AAudio**: Buffers with default sizes are now dynamically tuned.
+- **AAudio**: `SupportedBufferSize` now reports `min: 1`.
 - **ALSA**: Device disconnection now stops the stream with `StreamError::DeviceNotAvailable`
   instead of looping.
 - **ALSA**: Polling errors trigger underrun recovery instead of looping.
@@ -81,7 +82,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Reintroduce `audio_thread_priority` feature.
 - Fix numeric overflows in calls to create `StreamInstant` in ASIO, CoreAudio and JACK.
 - **AAudio**: Fix thread lock when a stream is dropped before it fully starts.
-- **AAudio**: Fix invalid capture and playback timestamps.
+- **AAudio**: Fix capture and playback timestamps falling back to time-zero on error.
+- **AAudio**: Fix capture and playback timestamp not accounting for audio pipeline buffer depth.
+- **AAudio**: Fix signed overflow in `buffer_capacity_in_frames` for large fixed buffer sizes.
 - **ALSA**: Fix capture stream hanging or spinning on overruns.
 - **ALSA**: Fix non-monotonic `StreamInstant` during stream startup.
 - **ALSA**: Fix spurious timestamp errors during stream startup.

src/host/aaudio/convert.rs

@@ -18,15 +18,44 @@ pub fn now_stream_instant() -> StreamInstant {
     StreamInstant::new(ts.tv_sec as u64, ts.tv_nsec as u32)
 }
 
-/// Returns the [`StreamInstant`] of the most recent audio frame transferred by `stream`.
-pub fn stream_instant(stream: &ndk::audio::AudioStream) -> StreamInstant {
-    let ts = stream
-        .timestamp(ndk::audio::Clockid::Monotonic)
-        .unwrap_or(ndk::audio::Timestamp {
-            frame_position: 0,
-            time_nanoseconds: 0,
-        });
-    StreamInstant::from_nanos(ts.time_nanoseconds as u64)
+/// Projects a hardware timestamp anchor to the instant of a specific frame position.
+fn stream_instant_from_anchor(
+    anchor_frame: i64,
+    anchor_nanos: i64,
+    app_frame: i64,
+    sample_rate: u32,
+) -> StreamInstant {
+    let offset_nanos =
+        (app_frame as i128 - anchor_frame as i128) * 1_000_000_000 / sample_rate as i128;
+    StreamInstant::from_nanos((anchor_nanos as i128 + offset_nanos).max(0) as u64)
+}
+
+/// Returns the [`StreamInstant`] for when the first frame of the current output callback will
+/// be presented at the DAC.
+pub fn output_stream_instant(stream: &ndk::audio::AudioStream, sample_rate: u32) -> StreamInstant {
+    match stream.timestamp(ndk::audio::Clockid::Monotonic) {
+        Ok(ts) => stream_instant_from_anchor(
+            ts.frame_position,
+            ts.time_nanoseconds,
+            stream.frames_written(),
+            sample_rate,
+        ),
+        Err(_) => now_stream_instant(),
+    }
+}
+
+/// Returns the [`StreamInstant`] for when the first frame of the current input callback was
+/// captured at the ADC.
+pub fn input_stream_instant(stream: &ndk::audio::AudioStream, sample_rate: u32) -> StreamInstant {
+    match stream.timestamp(ndk::audio::Clockid::Monotonic) {
+        Ok(ts) => stream_instant_from_anchor(
+            ts.frame_position,
+            ts.time_nanoseconds,
+            stream.frames_read(),
+            sample_rate,
+        ),
+        Err(_) => now_stream_instant(),
+    }
 }
 
 impl From<ndk::audio::AudioError> for StreamError {

src/host/aaudio/java_interface/audio_manager.rs

@@ -1,20 +1,9 @@
 use super::{
-    utils::{
-        get_context, get_property, get_system_property, get_system_service, with_attached, JNIEnv,
-        JObject, JResult,
-    },
-    AudioManager, Context,
+    utils::{get_context, get_system_property, with_attached, JNIEnv, JResult},
+    AudioManager,
 };
 
 impl AudioManager {
-    /// Get the frames per buffer using Android Java API
-    pub fn get_frames_per_buffer() -> Result<i32, String> {
-        let context = get_context();
-
-        with_attached(context, |env, context| get_frames_per_buffer(env, &context))
-            .map_err(|error| error.to_string())
-    }
-
     /// Get the AAudio mixer burst count from system property
     pub fn get_mixer_bursts() -> Result<i32, String> {
         let context = get_context();
@@ -24,23 +13,6 @@ impl AudioManager {
     }
 }
 
-fn get_frames_per_buffer<'j>(env: &mut JNIEnv<'j>, context: &JObject<'j>) -> JResult<i32> {
-    let audio_manager = get_system_service(env, context, Context::AUDIO_SERVICE)?;
-
-    let frames_per_buffer = get_property(
-        env,
-        &audio_manager,
-        AudioManager::PROPERTY_OUTPUT_FRAMES_PER_BUFFER,
-    )?;
-
-    let frames_per_buffer_string = String::from(env.get_string(&frames_per_buffer)?);
-
-    // TODO: Use jni::errors::Error::ParseFailed instead of jni::errors::Error::JniCall once jni > v0.21.1 is released
-    frames_per_buffer_string
-        .parse::<i32>()
-        .map_err(|_| jni::errors::Error::JniCall(jni::errors::JniError::Unknown))
-}
-
 fn get_mixer_bursts<'j>(env: &mut JNIEnv<'j>) -> JResult<i32> {
     let mixer_bursts = get_system_property(env, "aaudio.mixer_bursts", "2")?;
 

src/host/aaudio/java_interface/definitions.rs

@@ -21,9 +21,6 @@ impl PackageManager {
 pub(crate) struct AudioManager;
 
 impl AudioManager {
-    pub const PROPERTY_OUTPUT_FRAMES_PER_BUFFER: &'static str =
-        "android.media.property.OUTPUT_FRAMES_PER_BUFFER";
-
     pub const GET_DEVICES_INPUTS: i32 = 1 << 0;
     pub const GET_DEVICES_OUTPUTS: i32 = 1 << 1;
     pub const GET_DEVICES_ALL: i32 = Self::GET_DEVICES_INPUTS | Self::GET_DEVICES_OUTPUTS;

src/host/aaudio/java_interface/utils.rs

@@ -96,23 +96,6 @@ pub fn call_method_string_arg_ret_bool<'j>(
     .z()
 }
 
-pub fn call_method_string_arg_ret_string<'j>(
-    env: &mut JNIEnv<'j>,
-    subject: &JObject<'j>,
-    name: &str,
-    arg: impl AsRef<str>,
-) -> JResult<JString<'j>> {
-    Ok(env
-        .call_method(
-            subject,
-            name,
-            "(Ljava/lang/String;)Ljava/lang/String;",
-            &[(&env.new_string(arg)?).into()],
-        )?
-        .l()?
-        .into())
-}
-
 pub fn call_method_string_arg_ret_object<'j>(
     env: &mut JNIEnv<'j>,
     subject: &JObject<'j>,
@@ -157,14 +140,6 @@ pub fn get_system_service<'j>(
     call_method_string_arg_ret_object(env, subject, "getSystemService", name)
 }
 
-pub fn get_property<'j>(
-    env: &mut JNIEnv<'j>,
-    subject: &JObject<'j>,
-    name: &str,
-) -> JResult<JString<'j>> {
-    call_method_string_arg_ret_string(env, subject, "getProperty", name)
-}
-
 /// Read an Android system property
 pub fn get_system_property<'j>(
     env: &mut JNIEnv<'j>,

src/host/aaudio/mod.rs

@@ -11,14 +11,14 @@ use std::vec::IntoIter as VecIntoIter;
 
 extern crate ndk;
 
-use convert::{now_stream_instant, stream_instant};
+use convert::{input_stream_instant, now_stream_instant, output_stream_instant};
 use java_interface::{AudioDeviceInfo, AudioManager};
 
 use crate::traits::{DeviceTrait, HostTrait, StreamTrait};
 use crate::{
     BackendSpecificError, BufferSize, BuildStreamError, Data, DefaultStreamConfigError,
     DeviceDescription, DeviceDescriptionBuilder, DeviceDirection, DeviceId, DeviceIdError,
-    DeviceNameError, DeviceType, DevicesError, InputCallbackInfo, InputStreamTimestamp,
+    DeviceNameError, DeviceType, DevicesError, FrameCount, InputCallbackInfo, InputStreamTimestamp,
     InterfaceType, OutputCallbackInfo, OutputStreamTimestamp, PauseStreamError, PlayStreamError,
     SampleFormat, StreamConfig, StreamError, SupportedBufferSize, SupportedStreamConfig,
     SupportedStreamConfigRange, SupportedStreamConfigsError,
@@ -199,13 +199,9 @@ impl HostTrait for Host {
 }
 
 fn buffer_size_range() -> SupportedBufferSize {
-    if let Ok(min_buffer_size) = AudioManager::get_frames_per_buffer() {
-        SupportedBufferSize::Range {
-            min: min_buffer_size as u32,
-            max: i32::MAX as u32,
-        }
-    } else {
-        SupportedBufferSize::Unknown
+    SupportedBufferSize::Range {
+        min: 1,
+        max: i32::MAX as FrameCount,
     }
 }
 
@@ -296,8 +292,8 @@ fn configure_for_device(
     if let BufferSize::Fixed(size) = config.buffer_size {
         // For fixed sizes, the user explicitly wants control over the callback size.
         builder = builder
-            .frames_per_data_callback(size as i32)
-            .buffer_capacity_in_frames(2 * size as i32);
+            .frames_per_data_callback(size.min(i32::MAX as FrameCount) as i32)
+            .buffer_capacity_in_frames(size.saturating_mul(2).min(i32::MAX as FrameCount) as i32);
     }
 
     builder
@@ -317,12 +313,13 @@ where
 {
     let builder = configure_for_device(builder, device, config);
     let channel_count = config.channels as i32;
+    let sample_rate = config.sample_rate;
     let stream = builder
         .data_callback(Box::new(move |stream, data, num_frames| {
             let cb_info = InputCallbackInfo {
                 timestamp: InputStreamTimestamp {
                     callback: now_stream_instant(),
-                    capture: stream_instant(stream),
+                    capture: input_stream_instant(stream, sample_rate),
                 },
             };
             (data_callback)(
@@ -366,6 +363,7 @@ where
 {
     let builder = configure_for_device(builder, device, config);
     let channel_count = config.channels as i32;
+    let sample_rate = config.sample_rate;
     let tune_dynamically = config.buffer_size == BufferSize::Default;
 
     let tuning = Arc::new(BufferTuningState::default());
@@ -377,7 +375,7 @@ where
             let cb_info = OutputCallbackInfo {
                 timestamp: OutputStreamTimestamp {
                     callback: now_stream_instant(),
-                    playback: stream_instant(stream),
+                    playback: output_stream_instant(stream, sample_rate),
                 },
             };
             (data_callback)(