vlc | branch: master | David Fuhrmann <dfuhrm...@videolan.org> | Wed Jul 30 18:45:04 2014 +0200| [2034ba88d6e1779f6717a16520d1a8b07759b2aa] | committer: David Fuhrmann
auhal: rework locking and avoid potential deadlock hopefully closes #11675 > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=2034ba88d6e1779f6717a16520d1a8b07759b2aa --- modules/audio_output/auhal.c | 72 +++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/modules/audio_output/auhal.c b/modules/audio_output/auhal.c index 6253052..9bdd4fd 100644 --- a/modules/audio_output/auhal.c +++ b/modules/audio_output/auhal.c @@ -106,8 +106,12 @@ struct aout_sys_t int i_bytes_per_sample; 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; bool b_mute; @@ -186,7 +190,8 @@ static int Open(vlc_object_t *obj) 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_cond_init(&p_sys->cond); p_sys->b_digital = false; @@ -263,13 +268,21 @@ static void Close(vlc_object_t *obj) if (err != noErr) 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 */ - CFIndex count = CFArrayGetCount(p_sys->device_list); + CFIndex count = CFArrayGetCount(device_list_cpy); if (count > 0) { for (CFIndex x = 0; x < count; x++) { 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) continue; @@ -280,14 +293,15 @@ static void Close(vlc_object_t *obj) } } + CFRelease(device_list_cpy); CFRelease(p_sys->device_list); - vlc_mutex_unlock(&p_sys->var_lock); char *psz_device = aout_DeviceGet(p_aout); config_PutPsz(p_aout, "auhal-audio-device", 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_cond_destroy(&p_sys->cond); @@ -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->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; 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) err = AudioObjectGetPropertyData(kAudioObjectSystemObject, &defaultDeviceAddress, 0, NULL, &propertySize, &defaultDeviceID); if (err != noErr) { 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; } else @@ -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->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 b_start_digital = p_sys->b_selected_dev_is_digital; @@ -1286,7 +1300,7 @@ static void RebuildDeviceList(audio_output_t * p_aout) free(psz_name); } - vlc_mutex_lock(&p_sys->var_lock); + vlc_mutex_lock(&p_sys->device_list_lock); CFIndex count = 0; if (p_sys->device_list) count = CFArrayGetCount(p_sys->device_list); @@ -1302,6 +1316,7 @@ static void RebuildDeviceList(audio_output_t * p_aout) if (cfn_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); + ReportDevice(p_aout, i_device_id, NULL); } } @@ -1311,7 +1326,7 @@ static void RebuildDeviceList(audio_output_t * p_aout) CFRelease(p_sys->device_list); p_sys->device_list = CFArrayCreateCopy(kCFAllocatorDefault, currentListOfDevices); CFRelease(currentListOfDevices); - vlc_mutex_unlock(&p_sys->var_lock); + vlc_mutex_unlock(&p_sys->device_list_lock); free(deviceIDs); } @@ -1582,12 +1597,14 @@ static OSStatus DevicesListener(AudioObjectID inObjectID, UInt32 inNumberAddres msg_Dbg(p_aout, "audio device configuration changed, resetting cache"); 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); if(!CFArrayContainsValue(p_sys->device_list, CFRangeMake(0, CFArrayGetCount(p_sys->device_list)), selectedDevice)) aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT); 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; } @@ -1624,6 +1641,8 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i if (!p_aout) return -1; + aout_sys_t *p_sys = p_aout->sys; + if (!p_aout->sys->b_selected_dev_is_default) return noErr; @@ -1646,12 +1665,13 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i return noErr; } + vlc_mutex_lock(&p_sys->selected_device_lock); /* Also ignore events which announce the same device id */ - 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); + if(defaultDeviceID != p_aout->sys->i_selected_dev) { + 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; } @@ -1680,37 +1700,39 @@ static OSStatus StreamsChangedListener(AudioObjectID inObjectID, UInt32 inNumbe msg_Dbg(p_aout, "available physical formats for audio device changed"); 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. */ - if (p_sys->i_selected_dev == 0) + if (p_sys->i_selected_dev == 0) { + vlc_mutex_unlock(&p_sys->selected_device_lock); return 0; + } /* * check if changed stream id belongs to current device */ - vlc_mutex_lock(&p_sys->var_lock); AudioObjectPropertyAddress streamsAddress = { kAudioDevicePropertyStreams, kAudioDevicePropertyScopeOutput, kAudioObjectPropertyElementMaster }; err = AudioObjectGetPropertyDataSize(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size); 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); - vlc_mutex_unlock(&p_sys->var_lock); + vlc_mutex_unlock(&p_sys->selected_device_lock); return VLC_EGENERIC; } i_streams = i_param_size / sizeof(AudioStreamID); p_streams = (AudioStreamID *)malloc(i_param_size); if (p_streams == NULL) { - vlc_mutex_unlock(&p_sys->var_lock); + vlc_mutex_unlock(&p_sys->selected_device_lock); return VLC_ENOMEM; } err = AudioObjectGetPropertyData(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size, p_streams); if (err != noErr) { 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); 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++) { if (p_streams[i] == inObjectID) { _______________________________________________ vlc-commits mailing list vlc-commits@videolan.org https://mailman.videolan.org/listinfo/vlc-commits