Skip to content

Commit 954ffb7

Browse files
roderickvdbenface
andcommitted
fix(ios): resolve second review round on session interruption recovery
- Replace the playing/interrupted bool pair with a PlaybackState enum, so pause() during an interruption can't leave a stale flag that restarts the stream on the next interruption end. - Narrow unsafe to the extern-static reads that actually need it. - Drop the redundant field comment, tighten changelog wording. Co-authored-by: Benoît Rouleau <benoit.rouleau@icloud.com>
1 parent c9c02b2 commit 954ffb7

3 files changed

Lines changed: 50 additions & 49 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3030
- **AudioWorklet**: Fix stale audio output when a data callback wrote a partial buffer.
3131
- **CoreAudio**: Default-output streams now report xrun status.
3232
- **CoreAudio**: Fix stale audio output when a data callback wrote a partial buffer.
33-
- **iOS**: Streams stopped by an audio session interruption now resume when it ends, instead of staying silent until rebuilt.
33+
- **iOS**: Streams now resume automatically when an audio session interruption ends.
3434
- **JACK**: Streams with more channels than physical system ports no longer fail to build.
3535
- **JACK**: Channel enumeration is no longer capped at the physical system port count.
3636
- **JACK**: `Device::id` no longer embeds the process ID, so `DeviceId` is now stable across application restarts.

src/host/coreaudio/ios/mod.rs

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,39 @@ impl Device {
138138
}
139139
}
140140

141+
#[derive(Clone, Copy, Default, PartialEq, Eq)]
142+
enum PlaybackState {
143+
#[default]
144+
Stopped,
145+
Playing,
146+
Interrupted,
147+
}
148+
149+
struct StreamInner {
150+
state: PlaybackState,
151+
audio_unit: AudioUnit,
152+
}
153+
154+
impl StreamInner {
155+
fn stop_for_interruption(&mut self) {
156+
if self.state != PlaybackState::Playing {
157+
return;
158+
}
159+
// The OS may have already stopped the unit; stop() is idempotent.
160+
let _ = self.audio_unit.stop();
161+
self.state = PlaybackState::Interrupted;
162+
}
163+
164+
fn resume_after_interruption(&mut self) {
165+
if self.state != PlaybackState::Interrupted {
166+
return;
167+
}
168+
if self.audio_unit.start().is_ok() {
169+
self.state = PlaybackState::Playing;
170+
}
171+
}
172+
}
173+
141174
impl DeviceTrait for Device {
142175
type SupportedInputConfigs = SupportedInputConfigs;
143176
type SupportedOutputConfigs = SupportedOutputConfigs;
@@ -206,8 +239,7 @@ impl DeviceTrait for Device {
206239
)?;
207240

208241
let inner = Arc::new(Mutex::new(StreamInner {
209-
playing: false,
210-
interrupted: false,
242+
state: PlaybackState::default(),
211243
audio_unit,
212244
}));
213245
let session_manager = SessionEventManager::new(
@@ -266,8 +298,7 @@ impl DeviceTrait for Device {
266298
)?;
267299

268300
let inner = Arc::new(Mutex::new(StreamInner {
269-
playing: false,
270-
interrupted: false,
301+
state: PlaybackState::default(),
271302
audio_unit,
272303
}));
273304
let session_manager = SessionEventManager::new(
@@ -308,12 +339,12 @@ impl StreamTrait for Stream {
308339
let mut stream = self.inner.lock().map_err(|_| {
309340
Error::with_message(ErrorKind::StreamInvalidated, "Stream lock poisoned")
310341
})?;
311-
if !stream.playing {
342+
if stream.state != PlaybackState::Playing {
312343
stream
313344
.audio_unit
314345
.start()
315346
.context("Failed to start audio unit")?;
316-
stream.playing = true;
347+
stream.state = PlaybackState::Playing;
317348
}
318349
Ok(())
319350
}
@@ -322,13 +353,13 @@ impl StreamTrait for Stream {
322353
let mut stream = self.inner.lock().map_err(|_| {
323354
Error::with_message(ErrorKind::StreamInvalidated, "Stream lock poisoned")
324355
})?;
325-
if stream.playing {
356+
if stream.state == PlaybackState::Playing {
326357
stream
327358
.audio_unit
328359
.stop()
329360
.context("Failed to stop audio unit")?;
330-
stream.playing = false;
331361
}
362+
stream.state = PlaybackState::Stopped;
332363
Ok(())
333364
}
334365

@@ -342,35 +373,6 @@ impl StreamTrait for Stream {
342373
}
343374
}
344375

345-
struct StreamInner {
346-
playing: bool,
347-
/// Set while an interruption is what stopped the unit, so only those resume on its end.
348-
interrupted: bool,
349-
audio_unit: AudioUnit,
350-
}
351-
352-
impl StreamInner {
353-
/// The OS already stopped the unit; resync `playing` so a later `play` actually starts it.
354-
fn stop_for_interruption(&mut self) {
355-
if !self.playing {
356-
return;
357-
}
358-
let _ = self.audio_unit.stop();
359-
self.playing = false;
360-
self.interrupted = true;
361-
}
362-
363-
/// Only resumes what the interruption stopped; a stream the caller had paused stays paused.
364-
fn resume_after_interruption(&mut self) {
365-
if !std::mem::take(&mut self.interrupted) {
366-
return;
367-
}
368-
if self.audio_unit.start().is_ok() {
369-
self.playing = true;
370-
}
371-
}
372-
}
373-
374376
fn create_audio_unit() -> Result<AudioUnit, coreaudio::Error> {
375377
AudioUnit::new_uninitialized(coreaudio::audio_unit::IOType::RemoteIO)
376378
}

src/host/coreaudio/ios/session_event_manager.rs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ fn with_stream(stream: &Weak<Mutex<StreamInner>>, f: impl FnOnce(&mut StreamInne
3535
}
3636

3737
/// Reads the number stored under `key` in a notification's `userInfo`.
38-
unsafe fn user_info_number(notification: &NSNotification, key: Option<&NSString>) -> Option<usize> {
38+
fn user_info_number(notification: &NSNotification, key: Option<&NSString>) -> Option<usize> {
3939
let user_info = notification.userInfo()?;
4040
let value = user_info.objectForKey(key?)?;
4141
Some(value.downcast::<NSNumber>().ok()?.unsignedIntegerValue())
@@ -45,10 +45,9 @@ unsafe fn user_info_number(notification: &NSNotification, key: Option<&NSString>
4545
/// input or output latency. `true` means an input stream.
4646
type LatencyRefresh = (Arc<AtomicUsize>, bool);
4747

48-
unsafe fn route_change_error(notification: &NSNotification) -> Option<Error> {
49-
let reason = AVAudioSessionRouteChangeReason(unsafe {
50-
user_info_number(notification, AVAudioSessionRouteChangeReasonKey)?
51-
});
48+
fn route_change_error(notification: &NSNotification) -> Option<Error> {
49+
let key = unsafe { AVAudioSessionRouteChangeReasonKey };
50+
let reason = AVAudioSessionRouteChangeReason(user_info_number(notification, key)?);
5251
match reason {
5352
AVAudioSessionRouteChangeReason::OldDeviceUnavailable => Some(Error::with_message(
5453
ErrorKind::DeviceChanged,
@@ -103,18 +102,17 @@ impl SessionEventManager {
103102
return;
104103
}
105104
let notif = unsafe { notif.as_ref() };
106-
let Some(kind) =
107-
(unsafe { user_info_number(notif, AVAudioSessionInterruptionTypeKey) })
108-
else {
105+
let interruption_type_key = unsafe { AVAudioSessionInterruptionTypeKey };
106+
let Some(kind) = user_info_number(notif, interruption_type_key) else {
109107
return;
110108
};
111109
if AVAudioSessionInterruptionType(kind) == AVAudioSessionInterruptionType::Began {
112110
with_stream(&stream, StreamInner::stop_for_interruption);
113111
return;
114112
}
113+
let interruption_option_key = unsafe { AVAudioSessionInterruptionOptionKey };
115114
let options = AVAudioSessionInterruptionOptions(
116-
unsafe { user_info_number(notif, AVAudioSessionInterruptionOptionKey) }
117-
.unwrap_or(0),
115+
user_info_number(notif, interruption_option_key).unwrap_or(0),
118116
);
119117
if !options.contains(AVAudioSessionInterruptionOptions::ShouldResume) {
120118
return;
@@ -147,7 +145,8 @@ impl SessionEventManager {
147145
};
148146
frames.store(depth, Ordering::Relaxed);
149147
}
150-
if let Some(err) = unsafe { route_change_error(notif.as_ref()) } {
148+
let notif = unsafe { notif.as_ref() };
149+
if let Some(err) = route_change_error(notif) {
151150
emit_error(&cb, err);
152151
}
153152
}

0 commit comments

Comments
 (0)