Skip to content

ao_coreaudio: cleanup ao on init failure - #18383

Open
Dudemanguy wants to merge 2 commits into
mpv-player:masterfrom
Dudemanguy:coreaudio-unregister-hotplug
Open

ao_coreaudio: cleanup ao on init failure#18383
Dudemanguy wants to merge 2 commits into
mpv-player:masterfrom
Dudemanguy:coreaudio-unregister-hotplug

Conversation

@Dudemanguy

Copy link
Copy Markdown
Member

This matches what other AOs and VOs do and properly releases allocated resources on failure.

Can someone actually verify that my assumptions in the uninit are correct since I'm not a mac user.

@kasper93 kasper93 added this to the Release v0.42.0 milestone Aug 23, 2026
@low-batt

Copy link
Copy Markdown
Contributor

The fix passed my testing.

I do not have a macOS 27 setup. This testing was done under macOS 15.7.9 using mpv master @ 02a595d. To reproduce the crash I added the following statement to the initializer in ao_coreaudio.c:

    goto coreaudio_error;

right before:

    reinit_latency(ao);

In order to simulate a driver failure.

I built mpv with -Db_sanitize=address,undefined and started playing a video:

mpv/build/mpv --no-config --gpu-debug --log-file=output.txt ~/Movies/big_buck_bunny.mp4

The addition of the goto caused a switch to the AVFoundation driver:

[   0.177][v][ao/coreaudio] audio channel layout: tag: <6619138>
[   0.187][v][ao] Trying audio driver 'avfoundation'
[   0.187][v][ao/avfoundation] requested format: 48000 Hz, 5.1 channels, floatp

I powered on my AirPods Max to trigger a hotplug event. When the AirPods connected mpv crashed due to the address sanitizer detecting a heap buffer overflow. Details can be seen in the terminal session:

Terminal session:
low-batt@gag mpv ((e8673660ab...) %)$ emacs audio/out/ao_coreaudio.c
low-batt@gag mpv ((e8673660ab...) *%)$ git diff
diff --git a/audio/out/ao_coreaudio.c b/audio/out/ao_coreaudio.c
index 4b4bf1d48c..01e27c2d59 100644
--- a/audio/out/ao_coreaudio.c
+++ b/audio/out/ao_coreaudio.c
@@ -187,6 +187,7 @@ static int init(struct ao *ao)
     if (!r)
         goto coreaudio_error;
 
+    goto coreaudio_error;
     reinit_latency(ao);
     ao->device_buffer = av_rescale(p->hw_latency_ns, ao->samplerate, 1000000000) * 2;
 
low-batt@gag mpv ((e8673660ab...) *%)$ popd
~/Documents/builds/mpv-build/mpv-build
low-batt@gag mpv-build (master %=)$ ./build-for-testing.sh
…
low-batt@gag mpv-build (master %=)$ mpv/build/mpv --no-config --gpu-debug --log-file=output.txt ~/Movies/big_buck_bunny.mp4
mpv(51265,0x20de0a2c0) malloc: nano zone abandoned due to inability to reserve vm space.
● Video  --vid=1  (av1 3840x2160 60 fps 11394 kbps) [default]
● Audio  --aid=1  (aac 6ch 48000 Hz 386 kbps) [default]
● Subs   --sid=1  'ass' (ass) [external]
AO: [avfoundation] 48000Hz 5.1 6ch float
VO: [gpu-next] 3840x2160 yuv420p
[sub/ass] Neither PlayResX nor PlayResY defined. Assuming 384x288
                                         
Audio/Video desynchronisation detected! Possible reasons include too slow
hardware, temporary CPU spikes, broken drivers, and broken files. Audio
position will not match to the video (see A-V status field).
Consider trying `--profile=fast` and/or `--hwdec=auto` as they may help.

AV: 00:00:22 / 00:10:34 (4%) A-V:  0.000 Dropped: 51=================================================================
==51265==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6120000fad88 at pc 0x000105b403ac bp 0x00016b259940 sp 0x00016b259938
READ of size 8 at 0x6120000fad88 thread T17
AV: 00:00:23 / 00:10:34 (4%) A-V:  0.000 Dropped: 51    #0 0x000105b403a8 in hotplug_cb+0xd8 (mpv:arm64+0x100ce43a8)
    #1 0x0001a2dc305c in HALObject::PropertiesChanged(unsigned int, AudioObjectPropertyAddress const*)+0x77c (CoreAudio:arm64e+0x27f05c)
    #2 0x0001a2c1ce48 in HALSystem::PropertiesChanged(unsigned int, AudioObjectPropertyAddress const*)+0x184 (CoreAudio:arm64e+0xd8e48)
    #3 0x0001a2c1cbcc in HALSystem::ObjectsPublishedAndDied(unsigned int, unsigned int const*, unsigned int, unsigned int const*)+0x888 (CoreAudio:arm64e+0xd8bcc)
    #4 0x0001a2c23570 in HALSystem::AudioObjectsPublishedAndDied(AudioHardwarePlugInInterface**, unsigned int, unsigned int, unsigned int const*, unsigned int, unsigned int const*)+0xc4 (CoreAudio:arm64e+0xdf570)
    #5 0x0001a2cf5b68 in HALC_ShellPlugIn::ProxyObject_PropertiesChanged(unsigned int, unsigned int, AudioObjectPropertyAddress const*)+0x394 (CoreAudio:arm64e+0x1b1b68)
    #6 0x0001a2d57970 in void applesauce::dispatch::v1::async<HALC_ProxyNotifications::_SendPropertiesChanged(unsigned int, unsigned int, AudioObjectPropertyAddress const*, bool, std::__1::vector<HALC_ProxyNotifications::Listener, std::__1::allocator<HALC_ProxyNotifications::Listener>> const&)::$_0&>(dispatch_queue_s*, HALC_ProxyNotifications::_SendPropertiesChanged(unsigned int, unsigned int, AudioObjectPropertyAddress const*, bool, std::__1::vector<HALC_ProxyNotifications::Listener, std::__1::allocator<HALC_ProxyNotifications::Listener>> const&)::$_0&)::'lambda'(void*)::__invoke(void*)+0x64 (CoreAudio:arm64e+0x213970)
    #7 0x00010a9ebc70 in asan_dispatch_call_block_and_release+0xe0 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3bc70)
    #8 0x00019fca9858 in _dispatch_client_callout+0xc (libdispatch.dylib:arm64e+0x1b858)
    #9 0x00019fc9834c in _dispatch_lane_serial_drain+0x2e0 (libdispatch.dylib:arm64e+0xa34c)
    #10 0x00019fc98e5c in _dispatch_lane_invoke+0x1b4 (libdispatch.dylib:arm64e+0xae5c)
    #11 0x00019fc9a16c in _dispatch_workloop_invoke+0x648 (libdispatch.dylib:arm64e+0xc16c)
    #12 0x00019fca3260 in _dispatch_root_queue_drain_deferred_wlh+0x120 (libdispatch.dylib:arm64e+0x15260)
    #13 0x00019fca2ae4 in _dispatch_workloop_worker_thread+0x218 (libdispatch.dylib:arm64e+0x14ae4)
    #14 0x00019fe43e1c in _pthread_wqthread+0x120 (libsystem_pthread.dylib:arm64e+0x2e1c)
    #15 0x00019fe42b70 in start_wqthread+0x4 (libsystem_pthread.dylib:arm64e+0x1b70)

0x6120000fad88 is located 664 bytes after 304-byte region [0x6120000fa9c0,0x6120000faaf0)
freed by thread T7 here:
    #0 0x00010a9ed424 in free+0x7c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3d424)
    #1 0x000105a64798 in ta_free+0x1e0 (mpv:arm64+0x100c08798)
    #2 0x000105a6529c in ta_free_children+0x250 (mpv:arm64+0x100c0929c)
    #3 0x000105a6477c in ta_free+0x1c4 (mpv:arm64+0x100c0877c)
    #4 0x000105a6529c in ta_free_children+0x250 (mpv:arm64+0x100c0929c)
    #5 0x000105a6477c in ta_free+0x1c4 (mpv:arm64+0x100c0877c)
    #6 0x0001054934c8 in mp_cmd_ctx_complete+0x638 (mpv:arm64+0x1006374c8)
    #7 0x000105496ba4 in run_command+0x36c8 (mpv:arm64+0x10063aba4)
    #8 0x0001054743e4 in run_client_command+0xa64 (mpv:arm64+0x1006183e4)
    #9 0x000105474770 in mpv_command_node+0x220 (mpv:arm64+0x100618770)
    #10 0x000105ae5d40 in script_command_native+0x224 (mpv:arm64+0x100c89d40)
    #11 0x000105ae4338 in script_autofree_call+0x398 (mpv:arm64+0x100c88338)
    #12 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #13 0x00010926c1e8 in lua_pcall+0x94 (libluajit-5.1.2.1.1787165859.dylib:arm64+0x101e8)
    #14 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #15 0x000105ae3758 in load_scripts+0x25c (mpv:arm64+0x100c87758)
    #16 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #17 0x00010926c1e8 in lua_pcall+0x94 (libluajit-5.1.2.1.1787165859.dylib:arm64+0x101e8)
    #18 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #19 0x00010926c228 in lua_cpcall+0x18 (libluajit-5.1.2.1.1787165859.dylib:arm64+0x10228)
    #20 0x000105601528 in run_script+0x3cc (mpv:arm64+0x1007a5528)
    #21 0x000105601914 in script_thread+0x1c (mpv:arm64+0x1007a5914)
    #22 0x00010a9ea418 in asan_thread_start(void*)+0x4c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3a418)
    #23 0x00019fe47bc4 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x6bc4)
    #24 0x00019fe42b7c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1b7c)

previously allocated by thread T7 here:
    #0 0x00010a9ed520 in realloc+0x80 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3d520)
    #1 0x000105a63b24 in ta_realloc_size+0x178 (mpv:arm64+0x100c07b24)
    #2 0x000105a6a35c in ta_xrealloc_size+0x24 (mpv:arm64+0x100c0e35c)
    #3 0x000105311934 in finish_cmd+0x1990 (mpv:arm64+0x1004b5934)
    #4 0x00010530c948 in mp_input_parse_cmd_node+0x330 (mpv:arm64+0x1004b0948)
    #5 0x000105474738 in mpv_command_node+0x1e8 (mpv:arm64+0x100618738)
    #6 0x000105ae5d40 in script_command_native+0x224 (mpv:arm64+0x100c89d40)
    #7 0x000105ae4338 in script_autofree_call+0x398 (mpv:arm64+0x100c88338)
    #8 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #9 0x00010926c1e8 in lua_pcall+0x94 (libluajit-5.1.2.1.1787165859.dylib:arm64+0x101e8)
    #10 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #11 0x000105ae3758 in load_scripts+0x25c (mpv:arm64+0x100c87758)
    #12 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #13 0x00010926c1e8 in lua_pcall+0x94 (libluajit-5.1.2.1.1787165859.dylib:arm64+0x101e8)
    #14 0x00010925f1c8  (libluajit-5.1.2.1.1787165859.dylib:arm64+0x31c8)
    #15 0x00010926c228 in lua_cpcall+0x18 (libluajit-5.1.2.1.1787165859.dylib:arm64+0x10228)
    #16 0x000105601528 in run_script+0x3cc (mpv:arm64+0x1007a5528)
    #17 0x000105601914 in script_thread+0x1c (mpv:arm64+0x1007a5914)
    #18 0x00010a9ea418 in asan_thread_start(void*)+0x4c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3a418)
    #19 0x00019fe47bc4 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x6bc4)
    #20 0x00019fe42b7c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1b7c)

Thread T17 created by unknown thread
Thread T7 created by T3 here:
    #0 0x00010a9e59f8 in pthread_create+0x5c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x359f8)
    #1 0x0001055fbe5c in mp_load_script+0x1774 (mpv:arm64+0x10079fe5c)
    #2 0x0001055fd618 in load_builtin_script+0x5f8 (mpv:arm64+0x1007a1618)
    #3 0x0001055fc230 in mp_load_builtin_scripts+0x1d0 (mpv:arm64+0x1007a0230)
    #4 0x0001054d80cc in mp_option_run_callback+0xb70 (mpv:arm64+0x10067c0cc)
    #5 0x0001055ceb58 in handle_option_callbacks+0x28c (mpv:arm64+0x100772b58)
    #6 0x0001055af02c in mp_initialize+0x1750 (mpv:arm64+0x10075302c)
    #7 0x0001055b10e8 in mpv_main+0x2b8 (mpv:arm64+0x1007550e8)
    #8 0x000104e74fc0 in closure #1 in variable initialization expression of Application.playbackThread+0x150 (mpv:arm64+0x100018fc0)
    #9 0x000104e74fec in @objc closure #1 in variable initialization expression of Application.playbackThread+0x8 (mpv:arm64+0x100018fec)
    #10 0x00010a9ea418 in asan_thread_start(void*)+0x4c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3a418)
    #11 0x00019fe47bc4 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64e+0x6bc4)
    #12 0x00019fe42b7c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1b7c)

Thread T3 created by T0 here:
    #0 0x00010a9e59f8 in pthread_create+0x5c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x359f8)
    #1 0x000104e75514 in Application.main(_:_:)+0x3fc (mpv:arm64+0x100019514)
    #2 0x000104e758a4 in @objc Application.main(_:_:)+0x38 (mpv:arm64+0x1000198a4)
    #3 0x000105a6b810 in cocoa_main+0x30 (mpv:arm64+0x100c0f810)
    #4 0x000105bd0650 in main+0x20 (mpv:arm64+0x100d74650)
    #5 0x00019faa6b94  (<unknown module>)

SUMMARY: AddressSanitizer: heap-buffer-overflow (mpv:arm64+0x100ce43a8) in hotplug_cb+0xd8
Shadow bytes around the buggy address:
  0x6120000fab00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000fab80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000fac00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000fac80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000fad00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x6120000fad80: fa[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000fae00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000fae80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000faf00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6120000faf80: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x6120000fb000: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==51265==ABORTING
Abort trap: 6
low-batt@gag mpv-build (master %=)$ cp output.txt  ~/Documents/builds/mpv/prs/18383/without-patch/
low-batt@gag mpv-build (master %=)$ 

I then repeated the test, this time patching ao_coreaudio.c with the fix in this PR and then adding the goto to simulate a driver failure.

As before the addition of the goto caused a switch to the AVFoundation driver:

[   0.175][v][ao/coreaudio] audio channel layout: tag: <6619138>
[   0.187][v][ao] Trying audio driver 'avfoundation'
[   0.187][v][ao/avfoundation] requested format: 48000 Hz, 5.1 channels, floatp

This time when I connected my AirPods Max mpv successfully handled the hotplug even and changed the audio to the AirPods:

[  20.246][v][ao/coreaudio] selected audio output device: LG UltraFine Display Audio (89)
[  21.255][v][ao/coreaudio] Handling potential hotplug event...
[  21.255][v][ao/coreaudio] selected audio output device: AirPods Max (130)
Terminal session:
low-batt@gag mpv ((02a595ddc1...) %)$ patch -i 18383.patch
patching file 'audio/out/ao_coreaudio.c'
patching file 'audio/out/ao_coreaudio.c'
low-batt@gag mpv ((02a595ddc1...) *%)$ emacs audio/out/ao_coreaudio.c
low-batt@gag mpv ((02a595ddc1...) *%)$ git diff
diff --git a/audio/out/ao_coreaudio.c b/audio/out/ao_coreaudio.c
index 4b4bf1d48c..5fb2ee4a52 100644
--- a/audio/out/ao_coreaudio.c
+++ b/audio/out/ao_coreaudio.c
@@ -141,6 +141,7 @@ static void init_physical_format(struct ao *ao);
 static void reinit_latency(struct ao *ao);
 static bool register_hotplug_cb(struct ao *ao);
 static void unregister_hotplug_cb(struct ao *ao);
+static void uninit(struct ao *ao);
 
 static bool reinit_device(struct ao *ao) {
     struct priv *p = ao->priv;
@@ -167,9 +168,6 @@ static int init(struct ao *ao)
     if (!reinit_device(ao))
         goto coreaudio_error;
 
-    if (!register_hotplug_cb(ao))
-        goto coreaudio_error;
-
     if (p->change_physical_format)
         init_physical_format(ao);
 
@@ -187,6 +185,10 @@ static int init(struct ao *ao)
     if (!r)
         goto coreaudio_error;
 
+    if (!register_hotplug_cb(ao))
+        goto coreaudio_error;
+
+    goto coreaudio_error;
     reinit_latency(ao);
     ao->device_buffer = av_rescale(p->hw_latency_ns, ao->samplerate, 1000000000) * 2;
 
@@ -196,6 +198,7 @@ static int init(struct ao *ao)
     return CONTROL_OK;
 
 coreaudio_error:
+    uninit(ao);
     return CONTROL_ERROR;
 }
 
@@ -436,14 +439,18 @@ static void uninit(struct ao *ao)
 {
     struct priv *p = ao->priv;
 
-    dispatch_sync(p->queue, ^{
-        cancel_and_release_idle_work(p);
-    });
-    dispatch_release(p->queue);
+    if (p->queue) {
+        dispatch_sync(p->queue, ^{
+            cancel_and_release_idle_work(p);
+        });
+        dispatch_release(p->queue);
+    }
 
-    AudioOutputUnitStop(p->audio_unit);
-    AudioUnitUninitialize(p->audio_unit);
-    AudioComponentInstanceDispose(p->audio_unit);
+    if (p->audio_unit) {
+        AudioOutputUnitStop(p->audio_unit);
+        AudioUnitUninitialize(p->audio_unit);
+        AudioComponentInstanceDispose(p->audio_unit);
+    }
 
     if (p->original_asbd.mFormatID) {
         OSStatus err = CA_SET(p->original_asbd_stream,
low-batt@gag mpv ((02a595ddc1...) *%)$ popd
~/Documents/builds/mpv-build/mpv-build
low-batt@gag mpv-build (master %=)$ ./build-for-testing.sh
…
low-batt@gag mpv-build (master %=)$ mpv/build/mpv --no-config --gpu-debug --log-file=output.txt ~/Movies/big_buck_bunny.mp4
mpv(96698,0x20de0a2c0) malloc: nano zone abandoned due to inability to reserve vm space.
● Video  --vid=1  (av1 3840x2160 60 fps 11394 kbps) [default]
● Audio  --aid=1  (aac 6ch 48000 Hz 386 kbps) [default]
● Subs   --sid=1  'ass' (ass) [external]
AO: [avfoundation] 48000Hz 5.1 6ch float
VO: [gpu-next] 3840x2160 yuv420p
[sub/ass] Neither PlayResX nor PlayResY defined. Assuming 384x288
                                         
Audio/Video desynchronisation detected! Possible reasons include too slow
hardware, temporary CPU spikes, broken drivers, and broken files. Audio
position will not match to the video (see A-V status field).
Consider trying `--profile=fast` and/or `--hwdec=auto` as they may help.

[ao/avfoundation] restarting due to system notification; this will cause desync
AV: 00:00:40 / 00:10:34 (6%) A-V:  0.000 Dropped: 146 
Exiting... (Quit)
low-batt@gag mpv-build (master %=)$ cp output.txt  ~/Documents/builds/mpv/prs/18383/with-patch/
low-batt@gag mpv-build (master %=)$ 

Full logs and the crash report:
18383.zip

To test that uninit properly handles errors being detected at other locations within init, I repeated moving the goto that simulates a failure to each point at which init detects a failure, rebuilding and playing the video. In all cases mpv successfully switched to the AVFoundation driver and continued to play the video.

Changes look good to me.

This matches what other AOs and VOs do and properly releases allocated
resources on failure.
No real point in registering the hotplug if we don't even know if the
actual ao is going to work or not yet. Just move it down.
@Dudemanguy
Dudemanguy force-pushed the coreaudio-unregister-hotplug branch from 92ab324 to 3371b90 Compare August 31, 2026 15:06
@Dudemanguy

Copy link
Copy Markdown
Member Author

Thanks for testing. Rebased on master with the fixed macos CI

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