Commit cca33e15 authored by David Fuhrmann's avatar David Fuhrmann Committed by Jean-Baptiste Kempf

auhal: rework locking and avoid potential deadlock

hopefully closes #11675

(cherry picked from commit 2034ba88d6e1779f6717a16520d1a8b07759b2aa)
Signed-off-by: default avatarJean-Baptiste Kempf <jb@videolan.org>
parent 4d0af56a
...@@ -106,8 +106,12 @@ struct aout_sys_t ...@@ -106,8 +106,12 @@ struct aout_sys_t
int i_bytes_per_sample; int i_bytes_per_sample;
CFArrayRef device_list; CFArrayRef device_list;
vlc_mutex_t device_list_lock; /* protects access to device_list */
vlc_mutex_t var_lock; /* protects access to device_list and i_selected_dev */ vlc_mutex_t selected_device_lock;/* Synchronizes access to i_selected_dev. This is only needed
between VLCs audio thread and the core audio callback thread.
The value is only changed in Start, further access to this variable
within the audio thread (start, stop, close) needs no protection. */
float f_volume; float f_volume;
bool b_mute; bool b_mute;
...@@ -186,7 +190,8 @@ static int Open(vlc_object_t *obj) ...@@ -186,7 +190,8 @@ static int Open(vlc_object_t *obj)
OSStatus err = noErr; OSStatus err = noErr;
vlc_mutex_init(&p_sys->var_lock); vlc_mutex_init(&p_sys->device_list_lock);
vlc_mutex_init(&p_sys->selected_device_lock);
vlc_mutex_init(&p_sys->lock); vlc_mutex_init(&p_sys->lock);
vlc_cond_init(&p_sys->cond); vlc_cond_init(&p_sys->cond);
p_sys->b_digital = false; p_sys->b_digital = false;
...@@ -263,13 +268,21 @@ static void Close(vlc_object_t *obj) ...@@ -263,13 +268,21 @@ static void Close(vlc_object_t *obj)
if (err != noErr) if (err != noErr)
msg_Err(p_aout, "failed to remove listener for default audio device [%4.4s]", (char *)&err); msg_Err(p_aout, "failed to remove listener for default audio device [%4.4s]", (char *)&err);
vlc_mutex_lock(&p_sys->var_lock); /*
* StreamsChangedListener can rebuild the device list and thus held the device_list_lock.
* To avoid a possible deadlock, an array copy is created here.
* In rare cases, this can lead to missing StreamsChangedListener callback deregistration (TODO).
*/
vlc_mutex_lock(&p_sys->device_list_lock);
CFArrayRef device_list_cpy = CFArrayCreateCopy(NULL, p_sys->device_list);
vlc_mutex_unlock(&p_sys->device_list_lock);
/* remove streams callbacks */ /* remove streams callbacks */
CFIndex count = CFArrayGetCount(p_sys->device_list); CFIndex count = CFArrayGetCount(device_list_cpy);
if (count > 0) { if (count > 0) {
for (CFIndex x = 0; x < count; x++) { for (CFIndex x = 0; x < count; x++) {
AudioDeviceID deviceId = 0; AudioDeviceID deviceId = 0;
CFNumberRef cfn_device_id = CFArrayGetValueAtIndex(p_sys->device_list, x); CFNumberRef cfn_device_id = CFArrayGetValueAtIndex(device_list_cpy, x);
if (!cfn_device_id) if (!cfn_device_id)
continue; continue;
...@@ -280,14 +293,15 @@ static void Close(vlc_object_t *obj) ...@@ -280,14 +293,15 @@ static void Close(vlc_object_t *obj)
} }
} }
CFRelease(device_list_cpy);
CFRelease(p_sys->device_list); CFRelease(p_sys->device_list);
vlc_mutex_unlock(&p_sys->var_lock);
char *psz_device = aout_DeviceGet(p_aout); char *psz_device = aout_DeviceGet(p_aout);
config_PutPsz(p_aout, "auhal-audio-device", psz_device); config_PutPsz(p_aout, "auhal-audio-device", psz_device);
free(psz_device); free(psz_device);
vlc_mutex_destroy(&p_sys->var_lock); vlc_mutex_destroy(&p_sys->selected_device_lock);
vlc_mutex_destroy(&p_sys->device_list_lock);
vlc_mutex_destroy(&p_sys->lock); vlc_mutex_destroy(&p_sys->lock);
vlc_cond_destroy(&p_sys->cond); vlc_cond_destroy(&p_sys->cond);
...@@ -318,7 +332,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt) ...@@ -318,7 +332,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
p_sys->b_paused = false; p_sys->b_paused = false;
p_sys->i_device_latency = 0; p_sys->i_device_latency = 0;
vlc_mutex_lock(&p_sys->var_lock); vlc_mutex_lock(&p_sys->selected_device_lock);
p_sys->i_selected_dev = p_sys->i_new_selected_dev; p_sys->i_selected_dev = p_sys->i_new_selected_dev;
aout_FormatPrint(p_aout, "VLC is looking for:", fmt); aout_FormatPrint(p_aout, "VLC is looking for:", fmt);
...@@ -365,7 +379,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt) ...@@ -365,7 +379,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
err = AudioObjectGetPropertyData(kAudioObjectSystemObject, &defaultDeviceAddress, 0, NULL, &propertySize, &defaultDeviceID); err = AudioObjectGetPropertyData(kAudioObjectSystemObject, &defaultDeviceAddress, 0, NULL, &propertySize, &defaultDeviceID);
if (err != noErr) { if (err != noErr) {
msg_Err(p_aout, "could not get default audio device [%4.4s]", (char *)&err); msg_Err(p_aout, "could not get default audio device [%4.4s]", (char *)&err);
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->selected_device_lock);
goto error; goto error;
} }
else else
...@@ -374,7 +388,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt) ...@@ -374,7 +388,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
p_sys->i_selected_dev = defaultDeviceID; p_sys->i_selected_dev = defaultDeviceID;
p_sys->b_selected_dev_is_digital = var_InheritBool(p_aout, "spdif"); p_sys->b_selected_dev_is_digital = var_InheritBool(p_aout, "spdif");
} }
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->selected_device_lock);
// recheck if device still supports digital // recheck if device still supports digital
b_start_digital = p_sys->b_selected_dev_is_digital; b_start_digital = p_sys->b_selected_dev_is_digital;
...@@ -1286,7 +1300,7 @@ static void RebuildDeviceList(audio_output_t * p_aout) ...@@ -1286,7 +1300,7 @@ static void RebuildDeviceList(audio_output_t * p_aout)
free(psz_name); free(psz_name);
} }
vlc_mutex_lock(&p_sys->var_lock); vlc_mutex_lock(&p_sys->device_list_lock);
CFIndex count = 0; CFIndex count = 0;
if (p_sys->device_list) if (p_sys->device_list)
count = CFArrayGetCount(p_sys->device_list); count = CFArrayGetCount(p_sys->device_list);
...@@ -1302,6 +1316,7 @@ static void RebuildDeviceList(audio_output_t * p_aout) ...@@ -1302,6 +1316,7 @@ static void RebuildDeviceList(audio_output_t * p_aout)
if (cfn_device_id) { if (cfn_device_id) {
CFNumberGetValue(cfn_device_id, kCFNumberSInt32Type, &i_device_id); CFNumberGetValue(cfn_device_id, kCFNumberSInt32Type, &i_device_id);
msg_Dbg(p_aout, "Device ID %i is not found in new array, deleting.", i_device_id); msg_Dbg(p_aout, "Device ID %i is not found in new array, deleting.", i_device_id);
ReportDevice(p_aout, i_device_id, NULL); ReportDevice(p_aout, i_device_id, NULL);
} }
} }
...@@ -1311,7 +1326,7 @@ static void RebuildDeviceList(audio_output_t * p_aout) ...@@ -1311,7 +1326,7 @@ static void RebuildDeviceList(audio_output_t * p_aout)
CFRelease(p_sys->device_list); CFRelease(p_sys->device_list);
p_sys->device_list = CFArrayCreateCopy(kCFAllocatorDefault, currentListOfDevices); p_sys->device_list = CFArrayCreateCopy(kCFAllocatorDefault, currentListOfDevices);
CFRelease(currentListOfDevices); CFRelease(currentListOfDevices);
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->device_list_lock);
free(deviceIDs); free(deviceIDs);
} }
...@@ -1582,12 +1597,14 @@ static OSStatus DevicesListener(AudioObjectID inObjectID, UInt32 inNumberAddres ...@@ -1582,12 +1597,14 @@ static OSStatus DevicesListener(AudioObjectID inObjectID, UInt32 inNumberAddres
msg_Dbg(p_aout, "audio device configuration changed, resetting cache"); msg_Dbg(p_aout, "audio device configuration changed, resetting cache");
RebuildDeviceList(p_aout); RebuildDeviceList(p_aout);
vlc_mutex_lock(&p_sys->var_lock); vlc_mutex_lock(&p_sys->selected_device_lock);
vlc_mutex_lock(&p_sys->device_list_lock);
CFNumberRef selectedDevice = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &p_sys->i_selected_dev); CFNumberRef selectedDevice = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &p_sys->i_selected_dev);
if(!CFArrayContainsValue(p_sys->device_list, CFRangeMake(0, CFArrayGetCount(p_sys->device_list)), selectedDevice)) if(!CFArrayContainsValue(p_sys->device_list, CFRangeMake(0, CFArrayGetCount(p_sys->device_list)), selectedDevice))
aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT); aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT);
CFRelease(selectedDevice); CFRelease(selectedDevice);
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->device_list_lock);
vlc_mutex_unlock(&p_sys->selected_device_lock);
return noErr; return noErr;
} }
...@@ -1624,6 +1641,8 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i ...@@ -1624,6 +1641,8 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i
if (!p_aout) if (!p_aout)
return -1; return -1;
aout_sys_t *p_sys = p_aout->sys;
if (!p_aout->sys->b_selected_dev_is_default) if (!p_aout->sys->b_selected_dev_is_default)
return noErr; return noErr;
...@@ -1646,12 +1665,13 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i ...@@ -1646,12 +1665,13 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i
return noErr; return noErr;
} }
vlc_mutex_lock(&p_sys->selected_device_lock);
/* Also ignore events which announce the same device id */ /* Also ignore events which announce the same device id */
if(defaultDeviceID == p_aout->sys->i_selected_dev) if(defaultDeviceID != p_aout->sys->i_selected_dev) {
return noErr; msg_Dbg(p_aout, "default device actually changed, resetting aout");
aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT);
msg_Dbg(p_aout, "default device actually changed, resetting aout"); }
aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT); vlc_mutex_unlock(&p_sys->selected_device_lock);
return noErr; return noErr;
} }
...@@ -1680,37 +1700,39 @@ static OSStatus StreamsChangedListener(AudioObjectID inObjectID, UInt32 inNumbe ...@@ -1680,37 +1700,39 @@ static OSStatus StreamsChangedListener(AudioObjectID inObjectID, UInt32 inNumbe
msg_Dbg(p_aout, "available physical formats for audio device changed"); msg_Dbg(p_aout, "available physical formats for audio device changed");
RebuildDeviceList(p_aout); RebuildDeviceList(p_aout);
vlc_mutex_lock(&p_sys->selected_device_lock);
/* In this case audio has not yet started. Below code will not work and is not needed here. */ /* In this case audio has not yet started. Below code will not work and is not needed here. */
if (p_sys->i_selected_dev == 0) if (p_sys->i_selected_dev == 0) {
vlc_mutex_unlock(&p_sys->selected_device_lock);
return 0; return 0;
}
/* /*
* check if changed stream id belongs to current device * check if changed stream id belongs to current device
*/ */
vlc_mutex_lock(&p_sys->var_lock);
AudioObjectPropertyAddress streamsAddress = { kAudioDevicePropertyStreams, kAudioDevicePropertyScopeOutput, kAudioObjectPropertyElementMaster }; AudioObjectPropertyAddress streamsAddress = { kAudioDevicePropertyStreams, kAudioDevicePropertyScopeOutput, kAudioObjectPropertyElementMaster };
err = AudioObjectGetPropertyDataSize(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size); err = AudioObjectGetPropertyDataSize(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size);
if (err != noErr) { if (err != noErr) {
msg_Err(p_aout, "could not get number of streams for device %i [%4.4s]", p_sys->i_selected_dev, (char *)&err); msg_Err(p_aout, "could not get number of streams for device %i [%4.4s]", p_sys->i_selected_dev, (char *)&err);
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->selected_device_lock);
return VLC_EGENERIC; return VLC_EGENERIC;
} }
i_streams = i_param_size / sizeof(AudioStreamID); i_streams = i_param_size / sizeof(AudioStreamID);
p_streams = (AudioStreamID *)malloc(i_param_size); p_streams = (AudioStreamID *)malloc(i_param_size);
if (p_streams == NULL) { if (p_streams == NULL) {
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->selected_device_lock);
return VLC_ENOMEM; return VLC_ENOMEM;
} }
err = AudioObjectGetPropertyData(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size, p_streams); err = AudioObjectGetPropertyData(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size, p_streams);
if (err != noErr) { if (err != noErr) {
msg_Err(p_aout, "could not get list of streams [%4.4s]", (char *)&err); msg_Err(p_aout, "could not get list of streams [%4.4s]", (char *)&err);
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->selected_device_lock);
free(p_streams); free(p_streams);
return VLC_EGENERIC; return VLC_EGENERIC;
} }
vlc_mutex_unlock(&p_sys->var_lock); vlc_mutex_unlock(&p_sys->selected_device_lock);
for (int i = 0; i < i_streams; i++) { for (int i = 0; i < i_streams; i++) {
if (p_streams[i] == inObjectID) { if (p_streams[i] == inObjectID) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment