|
1 | | -From 3a6ea13fd15abffea92fb866a6d2e8905a8a2f4f Mon Sep 17 00:00:00 2001 |
2 | | - |
3 | | -Date: Sat, 4 Mar 2023 20:52:34 -0800 |
4 | | -Subject: [PATCH 1/2] osxaudio: fix race condition when removing AU callback |
5 | | - |
6 | | -If the IO thread has already entered the callback when it is being removed, |
7 | | -the callback function can use memory that is being freed. We address this by |
8 | | -wrapping the callback function with something that holds a lock. |
9 | | ---- |
10 | | - sys/osxaudio/gstosxcoreaudio.c | 18 +++++++++++ |
11 | | - sys/osxaudio/gstosxcoreaudio.h | 1 + |
12 | | - sys/osxaudio/gstosxcoreaudiocommon.c | 41 +++++++++++++++++++++++--- |
13 | | - sys/osxaudio/gstosxcoreaudiocommon.h | 9 ------ |
14 | | - sys/osxaudio/gstosxcoreaudiohal.c | 10 +++++++ |
15 | | - sys/osxaudio/gstosxcoreaudioremoteio.c | 2 ++ |
16 | | - 6 files changed, 68 insertions(+), 13 deletions(-) |
17 | | - |
18 | | -diff --git a/sys/osxaudio/gstosxcoreaudio.c b/sys/osxaudio/gstosxcoreaudio.c |
19 | | -index c432ee630..4fae6aff3 100644 |
20 | | ---- a/sys/osxaudio/gstosxcoreaudio.c |
21 | | -+++ b/sys/osxaudio/gstosxcoreaudio.c |
22 | | -@@ -27,6 +27,7 @@ |
23 | | - GST_DEBUG_CATEGORY_STATIC (osx_audio_debug); |
24 | | - #define GST_CAT_DEFAULT osx_audio_debug |
25 | | - |
26 | | -+#define gst_core_audio_parent_class parent_class |
27 | | - G_DEFINE_TYPE (GstCoreAudio, gst_core_audio, G_TYPE_OBJECT); |
28 | | - |
29 | | - #ifdef HAVE_IOS |
30 | | -@@ -35,10 +36,26 @@ G_DEFINE_TYPE (GstCoreAudio, gst_core_audio, G_TYPE_OBJECT); |
31 | | - #include "gstosxcoreaudiohal.c" |
32 | | - #endif |
33 | | - |
34 | | -+static void |
35 | | -+gst_core_audio_finalize (GObject * object) |
36 | | -+{ |
37 | | -+ GstCoreAudio *core_audio; |
38 | | -+ |
39 | | -+ core_audio = GST_CORE_AUDIO (object); |
40 | | -+ |
41 | | -+ g_mutex_clear (&core_audio->io_proc_lock); |
42 | | -+ |
43 | | -+ G_OBJECT_CLASS (parent_class)->finalize (object); |
44 | | -+} |
45 | | - |
46 | | - static void |
47 | | - gst_core_audio_class_init (GstCoreAudioClass * klass) |
48 | | - { |
49 | | -+ GObjectClass *gobject_class; |
50 | | -+ |
51 | | -+ gobject_class = (GObjectClass *) klass; |
52 | | -+ parent_class = g_type_class_peek_parent (klass); |
53 | | -+ gobject_class->finalize = gst_core_audio_finalize; |
54 | | - } |
55 | | - |
56 | | - static void |
57 | | -@@ -54,6 +71,7 @@ gst_core_audio_init (GstCoreAudio * core_audio) |
58 | | - core_audio->hog_pid = -1; |
59 | | - core_audio->disabled_mixing = FALSE; |
60 | | - #endif |
61 | | -+ g_mutex_init (&core_audio->io_proc_lock); |
62 | | - } |
63 | | - |
64 | | - static gboolean |
65 | | -diff --git a/sys/osxaudio/gstosxcoreaudio.h b/sys/osxaudio/gstosxcoreaudio.h |
66 | | -index ee88e3cf4..8d3f91fa7 100644 |
67 | | ---- a/sys/osxaudio/gstosxcoreaudio.h |
68 | | -+++ b/sys/osxaudio/gstosxcoreaudio.h |
69 | | -@@ -93,6 +93,7 @@ struct _GstCoreAudio |
70 | | - gint stream_idx; |
71 | | - gboolean io_proc_active; |
72 | | - gboolean io_proc_needs_deactivation; |
73 | | -+ GMutex io_proc_lock; |
74 | | - |
75 | | - /* For LPCM in/out */ |
76 | | - AudioUnit audiounit; |
77 | | -diff --git a/sys/osxaudio/gstosxcoreaudiocommon.c b/sys/osxaudio/gstosxcoreaudiocommon.c |
78 | | -index 39d03ac5b..0200cbfbc 100644 |
79 | | ---- a/sys/osxaudio/gstosxcoreaudiocommon.c |
80 | | -+++ b/sys/osxaudio/gstosxcoreaudiocommon.c |
81 | | -@@ -23,7 +23,17 @@ |
82 | | - |
83 | | - #include "gstosxcoreaudiocommon.h" |
84 | | - |
85 | | --void |
86 | | -+static OSStatus gst_core_audio_render_notify (GstCoreAudio * core_audio, |
87 | | -+ AudioUnitRenderActionFlags * ioActionFlags, |
88 | | -+ const AudioTimeStamp * inTimeStamp, |
89 | | -+ unsigned int inBusNumber, |
90 | | -+ unsigned int inNumberFrames, |
91 | | -+ AudioBufferList * ioData); |
92 | | -+ |
93 | | -+/** |
94 | | -+ * core_audio->io_proc_lock must be held before calling! |
95 | | -+*/ |
96 | | -+static void |
97 | | - gst_core_audio_remove_render_callback (GstCoreAudio * core_audio) |
98 | | - { |
99 | | - AURenderCallbackStruct input; |
100 | | -@@ -57,7 +67,7 @@ gst_core_audio_remove_render_callback (GstCoreAudio * core_audio) |
101 | | - core_audio->io_proc_active = FALSE; |
102 | | - } |
103 | | - |
104 | | --OSStatus |
105 | | -+static OSStatus |
106 | | - gst_core_audio_render_notify (GstCoreAudio * core_audio, |
107 | | - AudioUnitRenderActionFlags * ioActionFlags, |
108 | | - const AudioTimeStamp * inTimeStamp, |
109 | | -@@ -71,6 +81,7 @@ gst_core_audio_render_notify (GstCoreAudio * core_audio, |
110 | | - * work around some thread-safety issues in CoreAudio |
111 | | - */ |
112 | | - if ((*ioActionFlags) & kAudioUnitRenderAction_PreRender) { |
113 | | -+ // core_audio->io_proc_lock held before call to AudioUnitRender |
114 | | - if (core_audio->io_proc_needs_deactivation) { |
115 | | - gst_core_audio_remove_render_callback (core_audio); |
116 | | - } |
117 | | -@@ -79,6 +90,22 @@ gst_core_audio_render_notify (GstCoreAudio * core_audio, |
118 | | - return noErr; |
119 | | - } |
| 1 | +diff -Naur a/sys/osxaudio/gstosxcoreaudiocommon.c b/sys/osxaudio/gstosxcoreaudiocommon.c |
| 2 | +--- a/sys/osxaudio/gstosxcoreaudiocommon.c 2021-05-31 16:11:47.000000000 -0700 |
| 3 | ++++ b/sys/osxaudio/gstosxcoreaudiocommon.c 2022-12-24 23:00:48.000000000 -0800 |
| 4 | +@@ -137,15 +137,16 @@ |
| 5 | + "osx ring buffer stop ioproc: %p device_id %lu", |
| 6 | + core_audio->element->io_proc, (gulong) core_audio->device_id); |
120 | 7 |
|
121 | | -+static OSStatus |
122 | | -+gst_core_audio_io_proc_callback (GstCoreAudio * core_audio, |
123 | | -+ AudioUnitRenderActionFlags * ioActionFlags, |
124 | | -+ const AudioTimeStamp * inTimeStamp, |
125 | | -+ UInt32 inBusNumber, UInt32 inNumberFrames, AudioBufferList * bufferList) |
126 | | -+{ |
127 | | -+ OSStatus status = 0; |
128 | | -+ g_mutex_lock (&core_audio->io_proc_lock); |
| 8 | ++ // ###: why is it okay to directly remove from here but not from pause() ? |
129 | 9 | + if (core_audio->io_proc_active) { |
130 | | -+ status = core_audio->element->io_proc (core_audio->osxbuf, ioActionFlags, inTimeStamp, inBusNumber, inNumberFrames, bufferList); |
| 10 | ++ gst_core_audio_remove_render_callback (core_audio); |
131 | 11 | + } |
132 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
133 | 12 | + |
134 | | -+ return status; |
135 | | -+} |
136 | | -+ |
137 | | - gboolean |
138 | | - gst_core_audio_io_proc_start (GstCoreAudio * core_audio) |
139 | | - { |
140 | | -@@ -86,6 +113,7 @@ gst_core_audio_io_proc_start (GstCoreAudio * core_audio) |
141 | | - AURenderCallbackStruct input; |
142 | | - AudioUnitPropertyID callback_type; |
143 | | - |
144 | | -+ g_mutex_lock (&core_audio->io_proc_lock); |
145 | | - GST_DEBUG_OBJECT (core_audio->osxbuf, |
146 | | - "osx ring buffer start ioproc: %p device_id %lu", |
147 | | - core_audio->element->io_proc, (gulong) core_audio->device_id); |
148 | | -@@ -94,8 +122,8 @@ gst_core_audio_io_proc_start (GstCoreAudio * core_audio) |
149 | | - kAudioOutputUnitProperty_SetInputCallback : |
150 | | - kAudioUnitProperty_SetRenderCallback; |
151 | | - |
152 | | -- input.inputProc = (AURenderCallback) core_audio->element->io_proc; |
153 | | -- input.inputProcRefCon = core_audio->osxbuf; |
154 | | -+ input.inputProc = (AURenderCallback) gst_core_audio_io_proc_callback; |
155 | | -+ input.inputProcRefCon = core_audio; |
156 | | - |
157 | | - status = AudioUnitSetProperty (core_audio->audiounit, callback_type, kAudioUnitScope_Global, 0, /* N/A for global */ |
158 | | - &input, sizeof (input)); |
159 | | -@@ -103,6 +131,7 @@ gst_core_audio_io_proc_start (GstCoreAudio * core_audio) |
160 | | - if (status) { |
161 | | - GST_ERROR_OBJECT (core_audio->osxbuf, |
162 | | - "AudioUnitSetProperty failed: %d", (int) status); |
163 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
164 | | - return FALSE; |
165 | | - } |
166 | | - // ### does it make sense to do this notify stuff for input mode? |
167 | | -@@ -112,12 +141,14 @@ gst_core_audio_io_proc_start (GstCoreAudio * core_audio) |
168 | | - if (status) { |
169 | | - GST_ERROR_OBJECT (core_audio->osxbuf, |
170 | | - "AudioUnitAddRenderNotify failed %d", (int) status); |
171 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
172 | | - return FALSE; |
173 | | - } |
174 | | - core_audio->io_proc_active = TRUE; |
175 | | - } |
176 | | - |
177 | | - core_audio->io_proc_needs_deactivation = FALSE; |
178 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
179 | | - |
180 | | - status = AudioOutputUnitStart (core_audio->audiounit); |
| 13 | + status = AudioOutputUnitStop (core_audio->audiounit); |
181 | 14 | if (status) { |
182 | | -@@ -143,9 +174,11 @@ gst_core_audio_io_proc_stop (GstCoreAudio * core_audio) |
| 15 | + GST_WARNING_OBJECT (core_audio->osxbuf, |
183 | 16 | "AudioOutputUnitStop failed: %d", (int) status); |
184 | 17 | } |
185 | | - // ###: why is it okay to directly remove from here but not from pause() ? |
186 | | -+ g_mutex_lock (&core_audio->io_proc_lock); |
187 | | - if (core_audio->io_proc_active) { |
188 | | - gst_core_audio_remove_render_callback (core_audio); |
189 | | - } |
190 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
| 18 | +- // ###: why is it okay to directly remove from here but not from pause() ? |
| 19 | +- if (core_audio->io_proc_active) { |
| 20 | +- gst_core_audio_remove_render_callback (core_audio); |
| 21 | +- } |
191 | 22 | return TRUE; |
192 | 23 | } |
193 | 24 |
|
194 | | -diff --git a/sys/osxaudio/gstosxcoreaudiocommon.h b/sys/osxaudio/gstosxcoreaudiocommon.h |
195 | | -index c4602a6b3..a76b66656 100644 |
196 | | ---- a/sys/osxaudio/gstosxcoreaudiocommon.h |
197 | | -+++ b/sys/osxaudio/gstosxcoreaudiocommon.h |
198 | | -@@ -34,8 +34,6 @@ gboolean gst_core_audio_bind_device (GstCoreAudio *core_au |
199 | | - |
200 | | - void gst_core_audio_dump_channel_layout (AudioChannelLayout * channel_layout); |
201 | | - |
202 | | --void gst_core_audio_remove_render_callback (GstCoreAudio * core_audio); |
203 | | -- |
204 | | - gboolean gst_core_audio_io_proc_start (GstCoreAudio * core_audio); |
205 | | - |
206 | | - gboolean gst_core_audio_io_proc_stop (GstCoreAudio * core_audio); |
207 | | -@@ -54,13 +52,6 @@ gboolean gst_core_audio_open_device (GstCoreAudio *core_au |
208 | | - OSType sub_type, |
209 | | - const gchar *adesc); |
210 | | - |
211 | | --OSStatus gst_core_audio_render_notify (GstCoreAudio * core_audio, |
212 | | -- AudioUnitRenderActionFlags * ioActionFlags, |
213 | | -- const AudioTimeStamp * inTimeStamp, |
214 | | -- unsigned int inBusNumber, |
215 | | -- unsigned int inNumberFrames, |
216 | | -- AudioBufferList * ioData); |
217 | | -- |
218 | | - AudioChannelLabel gst_audio_channel_position_to_core_audio (GstAudioChannelPosition position, int channel); |
219 | | - |
220 | | - GstAudioChannelPosition gst_core_audio_channel_label_to_gst (AudioChannelLabel label, int channel, gboolean warn); |
221 | | -diff --git a/sys/osxaudio/gstosxcoreaudiohal.c b/sys/osxaudio/gstosxcoreaudiohal.c |
222 | | -index f649e4fc7..79fed0d97 100644 |
223 | | ---- a/sys/osxaudio/gstosxcoreaudiohal.c |
224 | | -+++ b/sys/osxaudio/gstosxcoreaudiohal.c |
225 | | -@@ -917,6 +917,9 @@ done: |
226 | | - return ret; |
227 | | - } |
228 | | - |
229 | | -+/** |
230 | | -+ * core_audio->io_proc_lock must be held! |
231 | | -+*/ |
232 | | - static inline void |
233 | | - _remove_render_spdif_callback (GstCoreAudio * core_audio) |
234 | | - { |
235 | | -@@ -950,6 +953,7 @@ _io_proc_spdif_start (GstCoreAudio * core_audio) |
236 | | - "osx ring buffer start ioproc ID: %p device_id %lu", |
237 | | - core_audio->procID, (gulong) core_audio->device_id); |
238 | | - |
239 | | -+ g_mutex_lock (&core_audio->io_proc_lock); |
240 | | - if (!core_audio->io_proc_active) { |
241 | | - /* Add IOProc callback */ |
242 | | - status = AudioDeviceCreateIOProcID (core_audio->device_id, |
243 | | -@@ -958,12 +962,14 @@ _io_proc_spdif_start (GstCoreAudio * core_audio) |
244 | | - if (status != noErr) { |
245 | | - GST_ERROR_OBJECT (core_audio->osxbuf, |
246 | | - ":AudioDeviceCreateIOProcID failed: %d", (int) status); |
247 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
248 | | - return FALSE; |
249 | | - } |
250 | | - core_audio->io_proc_active = TRUE; |
251 | | - } |
252 | | - |
253 | | - core_audio->io_proc_needs_deactivation = FALSE; |
254 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
255 | | - |
256 | | - /* Start device */ |
257 | | - status = AudioDeviceStart (core_audio->device_id, core_audio->procID); |
258 | | -@@ -991,9 +997,11 @@ _io_proc_spdif_stop (GstCoreAudio * core_audio) |
259 | | - "osx ring buffer stop ioproc ID: %p device_id %lu", |
260 | | - core_audio->procID, (gulong) core_audio->device_id); |
261 | | - |
262 | | -+ g_mutex_lock (&core_audio->io_proc_lock); |
263 | | - if (core_audio->io_proc_active) { |
264 | | - _remove_render_spdif_callback (core_audio); |
265 | | - } |
266 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
267 | | - |
268 | | - _close_spdif (core_audio); |
269 | | - |
270 | | -@@ -1054,6 +1062,7 @@ gst_core_audio_start_processing_impl (GstCoreAudio * core_audio) |
271 | | - static gboolean |
272 | | - gst_core_audio_pause_processing_impl (GstCoreAudio * core_audio) |
273 | | - { |
274 | | -+ g_mutex_lock (&core_audio->io_proc_lock); |
275 | | - if (core_audio->is_passthrough) { |
276 | | - GST_DEBUG_OBJECT (core_audio, |
277 | | - "osx ring buffer pause ioproc ID: %p device_id %lu", |
278 | | -@@ -1074,6 +1083,7 @@ gst_core_audio_pause_processing_impl (GstCoreAudio * core_audio) |
279 | | - core_audio->io_proc_needs_deactivation = TRUE; |
280 | | - } |
281 | | - } |
282 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
283 | | - return TRUE; |
284 | | - } |
285 | | - |
286 | | -diff --git a/sys/osxaudio/gstosxcoreaudioremoteio.c b/sys/osxaudio/gstosxcoreaudioremoteio.c |
287 | | -index 76b0eba32..10f5b18e4 100644 |
288 | | ---- a/sys/osxaudio/gstosxcoreaudioremoteio.c |
289 | | -+++ b/sys/osxaudio/gstosxcoreaudioremoteio.c |
290 | | -@@ -38,6 +38,7 @@ gst_core_audio_start_processing_impl (GstCoreAudio * core_audio) |
291 | | - static gboolean |
292 | | - gst_core_audio_pause_processing_impl (GstCoreAudio * core_audio) |
293 | | - { |
294 | | -+ g_mutex_lock (&core_audio->io_proc_lock); |
295 | | - GST_DEBUG_OBJECT (core_audio, |
296 | | - "osx ring buffer pause ioproc: %p device_id %lu", |
297 | | - core_audio->element->io_proc, (gulong) core_audio->device_id); |
298 | | -@@ -49,6 +50,7 @@ gst_core_audio_pause_processing_impl (GstCoreAudio * core_audio) |
299 | | - */ |
300 | | - core_audio->io_proc_needs_deactivation = TRUE; |
301 | | - } |
302 | | -+ g_mutex_unlock (&core_audio->io_proc_lock); |
303 | | - return TRUE; |
304 | | - } |
305 | | - |
306 | | --- |
307 | | -2.37.1 (Apple Git-137.1) |
308 | | - |
309 | 25 | From 8a269018758b52c0df4035ef5989bc0c1f89a685 Mon Sep 17 00:00:00 2001 |
310 | 26 | |
311 | 27 | Date: Sun, 5 Mar 2023 01:28:03 -0800 |
312 | | -Subject: [PATCH 2/2] osxaudio: detect changes to default device |
| 28 | +Subject: [PATCH] osxaudio: detect changes to default device |
313 | 29 |
|
314 | 30 | If the user does not specify a device and we use the default device, then |
315 | 31 | register a property listener for changes to that device and re-bind the AU. |
|
0 commit comments