audio: Mark disconnected default devices as "zombies".

Zombie devices just sit there doing nothing until a new default device
is chosen, and then they migrate all their logical devices before being
destroyed.

This lets the system deal with the likely outcome of a USB headset being
the default audio device, and when its cable is yanked out, the backend
will likely announce this _before_ it chooses a new default (or, perhaps,
the only device in the system got yanked out and there _isn't_ a new
default to be had until the user plugs the cable back in).

This lets the audio device hold on without disturbing the app until it can
seamlessly migrate audio, and it also means the backend does not have to
be careful in how it announces device events, since SDL will manage the
time between a device loss and its replacement.

Note that this _only_ applies to things opened as the default device
(SDL_AUDIO_DEVICE_DEFAULT_OUTPUT, etc). If those USB headphones are the
default, and one SDL_OpenAudioDevice() call asked for them specifically and
the other just said "give me the system default," the explicitly requested
open will get a device-lost notification immediately. The other open will
live on as a zombie until it can migrate to the new default.

This drops the complexity of the PulseAudio hotplug thread dramatically,
back to what it was previously, since it no longer needs to fight against
Pulse's asychronous nature, but just report device disconnects and new
default choices as they arrive.

loopwave has been updated to not check for device removals anymore; since
it opens the default device, this is now managed for it; it no longer
needs to close and reopen a device, and as far as it knows, the device
is never lost in the first place.

src/audio/SDL_audio.c

@@ -236,6 +236,7 @@ static SDL_AudioDevice *CreatePhysicalAudioDevice(const char *name, SDL_bool isc
 
     SDL_AtomicSet(&device->shutdown, 0);
     SDL_AtomicSet(&device->condemned, 0);
+    SDL_AtomicSet(&device->zombie, 0);
     device->iscapture = iscapture;
     SDL_memcpy(&device->spec, spec, sizeof (SDL_AudioSpec));
     SDL_memcpy(&device->default_spec, spec, sizeof (SDL_AudioSpec));
@@ -296,9 +297,27 @@ SDL_AudioDevice *SDL_AddAudioDevice(const SDL_bool iscapture, const char *name,
             SDL_PushEvent(&event);
         }
     }
+
     return device;
 }
 
+// this _also_ destroys the logical device!
+static void DisconnectLogicalAudioDevice(SDL_LogicalAudioDevice *logdev)
+{
+    if (SDL_EventEnabled(SDL_EVENT_AUDIO_DEVICE_REMOVED)) {
+        SDL_Event event;
+        SDL_zero(event);
+SDL_Log("Sending event about loss of logical device #%u", (unsigned int) logdev->instance_id);
+        event.type = SDL_EVENT_AUDIO_DEVICE_REMOVED;
+        event.common.timestamp = 0;
+        event.adevice.which = logdev->instance_id;
+        event.adevice.iscapture = logdev->physical_device->iscapture ? 1 : 0;
+        SDL_PushEvent(&event);
+    }
+
+    DestroyLogicalAudioDevice(logdev);
+}
+
 // Called when a device is removed from the system, or it fails unexpectedly, from any thread, possibly even the audio device's thread.
 void SDL_AudioDeviceDisconnected(SDL_AudioDevice *device)
 {
@@ -306,8 +325,21 @@ void SDL_AudioDeviceDisconnected(SDL_AudioDevice *device)
         return;
     }
 
-    // !!! FIXME: if this was the default device, we should figure out how to migrate the appropriate logical devices instead of declaring them dead.
-    // !!! FIXME: (right now we rely on the backends to change the default device before disconnecting this one, but that's probably not practical.)
+    // if the current default device is going down, mark it as dead but keep it around until a replacement is decided upon, so we can migrate logical devices to it.
+    if ((device->instance_id == current_audio.default_output_device_id) || (device->instance_id == current_audio.default_capture_device_id)) {
+        SDL_AtomicSet(&device->zombie, 1);
+        SDL_AtomicSet(&device->shutdown, 1);  // tell audio thread to terminate, but don't mark it condemned, so the thread won't destroy the device. We'll join on the audio thread later.
+
+        // dump any logical devices that explicitly opened this device. Things that opened the system default can stay.
+        SDL_LogicalAudioDevice *next = NULL;
+        for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev != NULL; logdev = next) {
+            next = logdev->next;
+            if (!logdev->is_default) {  // if opened as a default, leave it on the zombie device for later migration.
+                DisconnectLogicalAudioDevice(logdev);
+            }
+        }
+        return;  // done for now. Come back when a new default device is chosen!
+    }
 
     SDL_bool was_live = SDL_FALSE;
 
@@ -332,6 +364,9 @@ void SDL_AudioDeviceDisconnected(SDL_AudioDevice *device)
         was_live = SDL_TRUE;
     }
 
+    device->next = NULL;
+    device->prev = NULL;
+
     if (was_live) {
         SDL_AtomicDecRef(device->iscapture ? &current_audio.capture_device_count : &current_audio.output_device_count);
     }
@@ -342,6 +377,13 @@ void SDL_AudioDeviceDisconnected(SDL_AudioDevice *device)
     SDL_AtomicSet(&device->condemned, 1);
     SDL_AtomicSet(&device->shutdown, 1);  // tell audio thread to terminate.
 
+    // disconnect each attached logical device, so apps won't find their streams still bound if they get the REMOVED event before the device thread cleans up.
+    SDL_LogicalAudioDevice *next;
+    for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev != NULL; logdev = next) {
+        next = logdev->next;
+        DisconnectLogicalAudioDevice(logdev);
+    }
+
     // if there's an audio thread, don't free until thread is terminating, otherwise free stuff now.
     const SDL_bool should_destroy = (device->thread == NULL);
     SDL_UnlockMutex(device->lock);
@@ -355,16 +397,6 @@ void SDL_AudioDeviceDisconnected(SDL_AudioDevice *device)
         event.adevice.which = device->instance_id;
         event.adevice.iscapture = device->iscapture ? 1 : 0;
         SDL_PushEvent(&event);
-
-        // post an event for each logical device, too.
-        for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev != NULL; logdev = logdev->next) {
-            SDL_zero(event);
-            event.type = SDL_EVENT_AUDIO_DEVICE_REMOVED;
-            event.common.timestamp = 0;
-            event.adevice.which = logdev->instance_id;
-            event.adevice.iscapture = device->iscapture ? 1 : 0;
-            SDL_PushEvent(&event);
-        }
     }
 
     if (should_destroy) {
@@ -1113,6 +1145,11 @@ static int OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec
 {
     SDL_assert(device->logical_devices == NULL);
 
+    // Just pretend to open a zombie device. It can still collect logical devices on the assumption they will all migrate when the default device is officially changed.
+    if (SDL_AtomicGet(&device->zombie)) {
+        return 0;  // Braaaaaaaaains.
+    }
+
     SDL_AudioSpec spec;
     SDL_memcpy(&spec, inspec, sizeof (SDL_AudioSpec));
     PrepareAudioFormat(&spec);
@@ -1195,28 +1232,24 @@ SDL_AudioDeviceID SDL_OpenAudioDevice(SDL_AudioDeviceID devid, const SDL_AudioSp
     SDL_AudioDeviceID retval = 0;
 
     if (device) {
-        SDL_LogicalAudioDevice *logdev = (SDL_LogicalAudioDevice *) SDL_calloc(1, sizeof (SDL_LogicalAudioDevice));
-        if (!logdev) {
+        SDL_LogicalAudioDevice *logdev = NULL;
+        if (!is_default && SDL_AtomicGet(&device->zombie)) {
+            // uhoh, this device is undead, and just waiting for a new default device to be declared so it can hand off to it. Refuse explicit opens.
+            SDL_SetError("Device was already lost and can't accept new opens");
+        } else if ((logdev = (SDL_LogicalAudioDevice *) SDL_calloc(1, sizeof (SDL_LogicalAudioDevice))) == NULL) {
             SDL_OutOfMemory();
+        } else if (!device->is_opened && OpenPhysicalAudioDevice(device, spec) == -1) {  // first thing using this physical device? Open at the OS level...
+            SDL_free(logdev);
         } else {
-            if (device->logical_devices == NULL) {  // first thing using this physical device? Open at the OS level...
-                if (OpenPhysicalAudioDevice(device, spec) == -1) {
-                    SDL_free(logdev);
-                    logdev = NULL;
-                }
-            }
-
-            if (logdev != NULL) {
-                SDL_AtomicSet(&logdev->paused, 0);
-                retval = logdev->instance_id = assign_audio_device_instance_id(device->iscapture, /*islogical=*/SDL_TRUE);
-                logdev->physical_device = device;
-                logdev->is_default = is_default;
-                logdev->next = device->logical_devices;
-                if (device->logical_devices) {
-                    device->logical_devices->prev = logdev;
-                }
-                device->logical_devices = logdev;
+            SDL_AtomicSet(&logdev->paused, 0);
+            retval = logdev->instance_id = assign_audio_device_instance_id(device->iscapture, /*islogical=*/SDL_TRUE);
+            logdev->physical_device = device;
+            logdev->is_default = is_default;
+            logdev->next = device->logical_devices;
+            if (device->logical_devices) {
+                device->logical_devices->prev = logdev;
             }
+            device->logical_devices = logdev;
         }
         SDL_UnlockMutex(device->lock);
     }
@@ -1584,4 +1617,9 @@ void SDL_DefaultAudioDeviceChanged(SDL_AudioDevice *new_default_device)
     }
 
     SDL_UnlockMutex(new_default_device->lock);
+
+    // was current device already dead and just kept around to migrate to a new default device? Now we can kill it. Aim for the brain.
+    if (current_default_device && SDL_AtomicGet(&current_default_device->zombie)) {
+        SDL_AudioDeviceDisconnected(current_default_device);  // Call again, now that we're not the default; this will remove from device list, send removal events, and destroy the SDL_AudioDevice.
+    }
 }

src/audio/SDL_sysaudio.h

@@ -238,6 +238,9 @@ struct SDL_AudioDevice
     /* non-zero if we want the device to be destroyed (so audio thread knows to do it on termination). */
     SDL_AtomicInt condemned;
 
+    /* non-zero if this was a disconnected default device and we're waiting for its replacement. */
+    SDL_AtomicInt zombie;
+
     /* SDL_TRUE if this is a capture device instead of an output device */
     SDL_bool iscapture;
 

src/audio/pulseaudio/SDL_pulseaudio.c

@@ -810,53 +810,47 @@ static void ServerInfoCallback(pa_context *c, const pa_server_info *i, void *dat
     PULSEAUDIO_pa_threaded_mainloop_signal(pulseaudio_threaded_mainloop, 0);
 }
 
-typedef struct PulseHotplugEvent
+// This is called when PulseAudio has a device connected/removed/changed. */
+static void HotplugCallback(pa_context *c, pa_subscription_event_type_t t, uint32_t idx, void *data)
 {
-    pa_subscription_event_type_t t;
-    uint32_t idx;
-} PulseHotplugEvent;
+    const SDL_bool added = ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_NEW);
+    const SDL_bool removed = ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_REMOVE);
+    const SDL_bool changed = ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_CHANGE);
 
-typedef struct PulseHotplugEvents
-{
-    PulseHotplugEvent *events;
-    int num_events;
-    int allocated_events;
-    SDL_bool need_server_info_refresh;
-} PulseHotplugEvents;
+    if (added || removed || changed) { /* we only care about add/remove events. */
+        const SDL_bool sink = ((t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) == PA_SUBSCRIPTION_EVENT_SINK);
+        const SDL_bool source = ((t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) == PA_SUBSCRIPTION_EVENT_SOURCE);
 
-// This is called when PulseAudio has a device connected/removed/changed.
-static void HotplugCallback(pa_context *c, pa_subscription_event_type_t t, uint32_t idx, void *data)
-{
-    // We can't block in here for new ServerInfo, but we need that info to resolve first, since we can
-    // migrate default devices during a removal, as long as we know the new default. So just queue up
-    // the data and let HotplugThread handle it, where we can call WaitForPulseOperation.
-    PulseHotplugEvents *events = (PulseHotplugEvents *) data;
-    if (events->allocated_events <= events->num_events) {
-        const int new_allocation = events->num_events + 16;
-        void *ptr = SDL_realloc(events->events, new_allocation * sizeof (PulseHotplugEvent));
-        if (!ptr) {
-            return;  // oh well.
+        if (changed) {
+            PULSEAUDIO_pa_operation_unref(PULSEAUDIO_pa_context_get_server_info(pulseaudio_context, ServerInfoCallback, NULL));
         }
-        events->events = (PulseHotplugEvent *) ptr;
-        events->allocated_events = new_allocation;
-    }
-    events->events[events->num_events].t = t;
-    events->events[events->num_events].idx = idx;
-    events->num_events++;
 
-    const SDL_bool changed = ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_CHANGE);
-    if (changed) {
-        events->need_server_info_refresh = SDL_TRUE;
+        /* adds need sink details from the PulseAudio server. Another callback...
+           (just unref all these operations right away, because we aren't going to wait on them
+           and their callbacks will handle any work, so they can free as soon as that happens.) */
+        if ((added || changed) && sink) {
+            PULSEAUDIO_pa_operation_unref(PULSEAUDIO_pa_context_get_sink_info_by_index(pulseaudio_context, idx, SinkInfoCallback, (void *)((intptr_t)added)));
+        } else if ((added || changed) && source) {
+            PULSEAUDIO_pa_operation_unref(PULSEAUDIO_pa_context_get_source_info_by_index(pulseaudio_context, idx, SourceInfoCallback, (void *)((intptr_t)added)));
+        } else if (removed && (sink || source)) {
+            // removes we can handle just with the device index.
+            SDL_AudioDevice *device = SDL_ObtainPhysicalAudioDeviceByHandle((void *)((intptr_t)idx + 1));
+            if (device) {
+                SDL_UnlockMutex(device->lock);  // AudioDeviceDisconnected will relock and verify it's still in the list, but in case this is destroyed, unlock now.
+                SDL_AudioDeviceDisconnected(device);
+            }
+        }
     }
     PULSEAUDIO_pa_threaded_mainloop_signal(pulseaudio_threaded_mainloop, 0);
 }
 
-static void CheckDefaultDevice(int prev_default, int new_default)
+static void CheckDefaultDevice(uint32_t *prev_default, uint32_t new_default)
 {
-    if (prev_default != new_default) {
+    if (*prev_default != new_default) {
         SDL_AudioDevice *device = SDL_ObtainPhysicalAudioDeviceByHandle((void *)((intptr_t)new_default + 1));
         if (device) {
             SDL_UnlockMutex(device->lock);
+            *prev_default = new_default;
             SDL_DefaultAudioDeviceChanged(device);
         }
     }
@@ -865,78 +859,25 @@ static void CheckDefaultDevice(int prev_default, int new_default)
 // this runs as a thread while the Pulse target is initialized to catch hotplug events.
 static int SDLCALL HotplugThread(void *data)
 {
-    pa_operation *op;
+    uint32_t prev_default_sink_index = default_sink_index;
+    uint32_t prev_default_source_index = default_source_index;
 
     SDL_SetThreadPriority(SDL_THREAD_PRIORITY_LOW);
-
-    PulseHotplugEvents events;
-    SDL_zero(events);
-
     PULSEAUDIO_pa_threaded_mainloop_lock(pulseaudio_threaded_mainloop);
-    PULSEAUDIO_pa_context_set_subscribe_callback(pulseaudio_context, HotplugCallback, &events);
-    WaitForPulseOperation(PULSEAUDIO_pa_context_subscribe(pulseaudio_context, PA_SUBSCRIPTION_MASK_SINK | PA_SUBSCRIPTION_MASK_SOURCE | PA_SUBSCRIPTION_MASK_SERVER, NULL, NULL));
+    PULSEAUDIO_pa_context_set_subscribe_callback(pulseaudio_context, HotplugCallback, NULL);
+    WaitForPulseOperation(PULSEAUDIO_pa_context_subscribe(pulseaudio_context, PA_SUBSCRIPTION_MASK_SINK | PA_SUBSCRIPTION_MASK_SOURCE, NULL, NULL));
     while (SDL_AtomicGet(&pulseaudio_hotplug_thread_active)) {
         PULSEAUDIO_pa_threaded_mainloop_wait(pulseaudio_threaded_mainloop);
-        if (events.need_server_info_refresh) {
-            events.need_server_info_refresh = SDL_FALSE;
-            WaitForPulseOperation(PULSEAUDIO_pa_context_get_server_info(pulseaudio_context, ServerInfoCallback, NULL));
-        } else if (events.num_events) {  // don't process events until we didn't get anything new that needs a server info update.
-            const uint32_t prev_default_sink_index = default_sink_index;
-            const uint32_t prev_default_source_index = default_source_index;
-            PulseHotplugEvents eventscpy;
-            SDL_memcpy(&eventscpy, &events, sizeof (PulseHotplugEvents));
-            SDL_zero(events);  // make sure the current array isn't touched in case new events fire.
-
-            // process adds and changes first, so we definitely have default device changes processed. Then remove devices after.
-            for (int i = 0; i < eventscpy.num_events; i++) {
-                const pa_subscription_event_type_t t = eventscpy.events[i].t;
-                const uint32_t idx = eventscpy.events[i].idx;
-                const SDL_bool added = ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_NEW);
-                const SDL_bool changed = ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_CHANGE);
-                const SDL_bool sink = ((t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) == PA_SUBSCRIPTION_EVENT_SINK);
-                const SDL_bool source = ((t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) == PA_SUBSCRIPTION_EVENT_SOURCE);
-                if (added || changed) {
-                    if (sink) {
-                        WaitForPulseOperation(PULSEAUDIO_pa_context_get_sink_info_by_index(pulseaudio_context, idx, SinkInfoCallback, (void *)((intptr_t)added)));
-                    } else if (source) {
-                        WaitForPulseOperation(PULSEAUDIO_pa_context_get_source_info_by_index(pulseaudio_context, idx, SourceInfoCallback, (void *)((intptr_t)added)));
-                    }
-                }
-            }
-
-            // don't hold the pulse lock during this, since it could deadlock vs a playing device that we're about to lock here.
-            PULSEAUDIO_pa_threaded_mainloop_unlock(pulseaudio_threaded_mainloop);
-
-            CheckDefaultDevice(prev_default_sink_index, default_sink_index);
-            CheckDefaultDevice(prev_default_source_index, default_source_index);
-
-            // okay, default devices are sane, migrations were made if necessary...now remove lost devices.
-            for (int i = 0; i < eventscpy.num_events; i++) {
-                const pa_subscription_event_type_t t = eventscpy.events[i].t;
-                const uint32_t idx = eventscpy.events[i].idx;
-                const SDL_bool removed = ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_REMOVE);
-                const SDL_bool sink = ((t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) == PA_SUBSCRIPTION_EVENT_SINK);
-                const SDL_bool source = ((t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) == PA_SUBSCRIPTION_EVENT_SOURCE);
-                if (removed && (sink || source)) {
-                    SDL_AudioDevice *device = SDL_ObtainPhysicalAudioDeviceByHandle((void *)((intptr_t)idx + 1));
-                    if (device) {
-                        SDL_UnlockMutex(device->lock);  // AudioDeviceDisconnected will relock and verify it's still in the list, but in case this is destroyed, unlock now.
-                        SDL_AudioDeviceDisconnected(device);
-                    }
-                }
-            }
-
-            PULSEAUDIO_pa_threaded_mainloop_lock(pulseaudio_threaded_mainloop);
 
-            SDL_free(eventscpy.events);
-        }
+        // Update default devices; don't hold the pulse lock during this, since it could deadlock vs a playing device that we're about to lock here.
+        PULSEAUDIO_pa_threaded_mainloop_unlock(pulseaudio_threaded_mainloop);
+        CheckDefaultDevice(&prev_default_sink_index, default_sink_index);
+        CheckDefaultDevice(&prev_default_source_index, default_source_index);
+        PULSEAUDIO_pa_threaded_mainloop_lock(pulseaudio_threaded_mainloop);
     }
 
     PULSEAUDIO_pa_context_set_subscribe_callback(pulseaudio_context, NULL, NULL);  // Don't fire HotplugCallback again.
     PULSEAUDIO_pa_threaded_mainloop_unlock(pulseaudio_threaded_mainloop);
-
-    SDL_free(events.events);  // should be NULL, but just in case.
-
     return 0;
 }
 

test/loopwave.c

@@ -103,14 +103,6 @@ open_audio(void)
     SDL_SetAudioStreamGetCallback(stream, fillerup, NULL);
 }
 
-#ifndef __EMSCRIPTEN__
-static void reopen_audio(void)
-{
-    close_audio();
-    open_audio();
-}
-#endif
-
 
 static int done = 0;
 
@@ -191,8 +183,6 @@ int main(int argc, char *argv[])
 
     open_audio();
 
-    SDL_FlushEvents(SDL_EVENT_AUDIO_DEVICE_ADDED, SDL_EVENT_AUDIO_DEVICE_REMOVED);
-
 #ifdef __EMSCRIPTEN__
     emscripten_set_main_loop(loop, 0, 1);
 #else
@@ -203,10 +193,6 @@ int main(int argc, char *argv[])
             if (event.type == SDL_EVENT_QUIT) {
                 done = 1;
             }
-            if ((event.type == SDL_EVENT_AUDIO_DEVICE_ADDED && !event.adevice.iscapture) ||
-                (event.type == SDL_EVENT_AUDIO_DEVICE_REMOVED && !event.adevice.iscapture && event.adevice.which == device)) {
-                reopen_audio();
-            }
         }
 
         SDL_Delay(100);