[vlc-devel] [PATCH] aout: add device cycling helper

Rémi Denis-Courmont remi at remlab.net
Fri Nov 30 13:32:25 CET 2018


You don't need half of the code to factor a race-prone next-device function like this.

Le 30 novembre 2018 10:29:56 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>On Thu, Nov 29, 2018, at 14:46, Rémi Denis-Courmont wrote:
>> since there is anyway a ToCToU situation now, I don't see the point
>of
>> the helper, and even less of the unlocked function.
>Factorize it between several interfaces modules ?
>
>> Le 28 novembre 2018 16:47:22 GMT+02:00, Victorien Le Couviour--Tuffet
>> <victorien.lecouviour.tuffet at gmail.com> a écrit :>> 
>include/vlc_aout.h        |  1 +
>>> 
>>>  src/audio_output/output.c | 94
>++++++++++++++++++++++++++++++++-------
>>>>>  src/libvlccore.sym        |  1 +
>>> 
>>>  3 files changed, 81 insertions(+), 15 deletions(-)
>>> 
>>> 
>>> 
>>> diff --git a/include/vlc_aout.h b/include/vlc_aout.h
>>> 
>>> index aa49884001..ef78127c13 100644
>>> 
>>> --- a/include/vlc_aout.h
>>> 
>>> +++ b/include/vlc_aout.h
>>> 
>>> @@ -373,6 +373,7 @@ VLC_API int aout_MuteGet (audio_output_t *);
>>>>>  VLC_API int aout_MuteSet (audio_output_t *, bool);
>>> 
>>>  VLC_API char *aout_DeviceGet (audio_output_t *);
>>> 
>>>  VLC_API int aout_DeviceSet (audio_output_t *, const char *);
>>> 
>>> +VLC_API int aout_DeviceNext (audio_output_t *, char **);
>>> 
>>>  VLC_API int aout_DevicesList (audio_output_t *, char ***, char
>***);
>>>>>  
>>> 
>>>  /**
>>> 
>>> diff --git a/src/audio_output/output.c b/src/audio_output/output.c
>>>>> index 019797c5ef..ef643260b0 100644
>>> 
>>> --- a/src/audio_output/output.c
>>> 
>>> +++ b/src/audio_output/output.c
>>> 
>>> @@ -701,24 +701,15 @@ int aout_DeviceSet (audio_output_t *aout,
>const
>>> char *id)
>>>>>      return ret ? -1 : 0;
>>> 
>>>  }
>>> 
>>>  
>>> 
>>> -/**
>>> 
>>> - * Enumerates possible audio output devices.
>>> 
>>> - *
>>> 
>>> - * The function will heap-allocate two tables of heap-allocated
>>>   strings;
>>>>> - * the caller is responsible for freeing all strings and both
>>>   tables.
>>>>> - *
>>> 
>>> - * \param ids pointer to a table of device identifiers [OUT]
>>> 
>>> - * \param names pointer to a table of device human-readable
>>>   descriptions [OUT]
>>>>> - * \return the number of devices, or negative on error.
>>> 
>>> - * \note In case of error, *ids and *names are undefined.
>>> 
>>> - */
>>> 
>>> -int aout_DevicesList (audio_output_t *aout, char ***ids, char
>>> ***names)
>>>>> +static int aout_DevicesListLocked(audio_output_t *aout,
>>> 
>>> +                                  char ***ids, char ***names)
>>> 
>>>  {
>>> 
>>>      aout_owner_t *owner = aout_owner (aout);
>>> 
>>> +    vlc_mutex_assert(&owner->lock);
>>> 
>>> +    vlc_mutex_assert(&owner->dev.lock);
>>> 
>>>      char **tabid, **tabname;
>>> 
>>>      unsigned i = 0;
>>> 
>>>  
>>> 
>>> -    vlc_mutex_lock (&owner->dev.lock);
>>> 
>>>      tabid = vlc_alloc (owner->dev.count, sizeof (*tabid));
>>> 
>>>      tabname = vlc_alloc (owner->dev.count, sizeof (*tabname));
>>> 
>>>  
>>> 
>>> @@ -743,12 +734,10 @@ int aout_DevicesList (audio_output_t *aout,
>>> char ***ids, char ***names)
>>>>>  
>>> 
>>>          i++;
>>> 
>>>      }
>>> 
>>> -    vlc_mutex_unlock (&owner->dev.lock);
>>> 
>>>  
>>> 
>>>      return i;
>>> 
>>>  
>>> 
>>>  error:
>>> 
>>> -    vlc_mutex_unlock(&owner->dev.lock);
>>> 
>>>      while (i > 0)
>>> 
>>>      {
>>> 
>>>          i--;
>>> 
>>> @@ -760,6 +749,81 @@ error:
>>> 
>>>      return -1;
>>> 
>>>  }
>>> 
>>>  
>>> 
>>> +/**
>>> 
>>> + * Enumerates possible audio output devices.
>>> 
>>> + *
>>> 
>>> + * The function will heap-allocate two tables of heap-allocated
>>>   strings;
>>>>> + * the caller is responsible for freeing all strings and both
>>>   tables.
>>>>> + *
>>> 
>>> + * \param ids pointer to a table of device identifiers [OUT]
>>> 
>>> + * \param names pointer to a table of device human-readable
>>>   descriptions [OUT]
>>>>> + * \return the number of devices, or negative on error.
>>> 
>>> + * \note In case of error, *ids and *names are undefined.
>>> 
>>> + */
>>> 
>>> +int aout_DevicesList (audio_output_t *aout, char ***ids, char
>>> ***names)
>>>>> +{
>>> 
>>> +    aout_OutputLock(aout);
>>> 
>>> +    vlc_mutex_t *lock = &aout_owner(aout)->dev.lock;
>>> 
>>> +    vlc_mutex_lock(lock);
>>> 
>>> +    int ret = aout_DevicesListLocked(aout, ids, names);
>>> 
>>> +    vlc_mutex_unlock(lock);
>>> 
>>> +    aout_OutputUnlock(aout);
>>> 
>>> +    return ret;
>>> 
>>> +}
>>> 
>>> +
>>> 
>>> +/**
>>> 
>>> + * Cycle through audio output devices.
>>> 
>>> + * \param p_name pointer to the selected device human-readable
>>>   description [OUT]
>>>>> + * \return zero on success, non-zero on error.
>>> 
>>> + */
>>> 
>>> +int aout_DeviceNext (audio_output_t *aout, char **p_name)
>>> 
>>> +{
>>> 
>>> +    int ret = -1;
>>> 
>>> +    aout_owner_t *owner = aout_owner(aout);
>>> 
>>> +    aout_OutputLock(aout);
>>> 
>>> +    vlc_mutex_lock(&owner->dev.lock);
>>> 
>>> +    bool locked = true;
>>> 
>>> +
>>> 
>>> +    char **ids, **names;
>>> 
>>> +    int n = aout_DevicesListLocked(aout, &ids, &names);
>>> 
>>> +    if (n == -1)
>>> 
>>> +        goto end;
>>> 
>>> +    char *device = aout_DeviceGet(aout);
>>> 
>>> +    if (!device)
>>> 
>>> +        goto no_dev;
>>> 
>>> +
>>> 
>>> +    int index;
>>> 
>>> +    for (index = 0; index < n; ++index)
>>> 
>>> +        if (!strcmp(ids[index], device))
>>> 
>>> +        {
>>> 
>>> +            index = (index + 1) % n;
>>> 
>>> +            break;
>>> 
>>> +        }
>>> 
>>> +    vlc_mutex_unlock(&owner->dev.lock);
>>> 
>>> +    aout_OutputUnlock(aout);
>>> 
>>> +    locked = false;
>>> 
>>> +    ret = aout_DeviceSet(aout, ids[index]);
>>> 
>>> +    if (p_name != NULL)
>>> 
>>> +        *p_name = strdup(names[index]);
>>> 
>>> +
>>> 
>>> +    free(device);
>>> 
>>> +no_dev:
>>> 
>>> +    for (int i = 0; i < n; ++i)
>>> 
>>> +    {
>>> 
>>> +        free(ids[i]);
>>> 
>>> +        free(names[i]);
>>> 
>>> +    }
>>> 
>>> +    free(ids);
>>> 
>>> +    free(names);
>>> 
>>> +end:
>>> 
>>> +    if (locked)
>>> 
>>> +    {
>>> 
>>> +        vlc_mutex_unlock(&owner->dev.lock);
>>> 
>>> +        aout_OutputUnlock(aout);
>>> 
>>> +    }
>>> 
>>> +    return ret;
>>> 
>>> +}
>>> 
>>> +
>>> 
>>>  static void aout_ChangeViewpoint(audio_output_t *aout,
>>> 
>>>                                   const vlc_viewpoint_t
>*p_viewpoint)
>>>>>  {
>>> 
>>> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
>>> 
>>> index d4db4ccc51..41af4b48db 100644
>>> 
>>> --- a/src/libvlccore.sym
>>> 
>>> +++ b/src/libvlccore.sym
>>> 
>>> @@ -20,6 +20,7 @@ aout_MuteSet
>>> 
>>>  aout_MuteGet
>>> 
>>>  aout_DeviceGet
>>> 
>>>  aout_DeviceSet
>>> 
>>> +aout_DeviceNext
>>> 
>>>  aout_DevicesList
>>> 
>>>  aout_FiltersNew
>>> 
>>>  aout_FiltersChangeViewpoint
>>> 
>> 
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>> excuser ma brièveté.>
>_________________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20181130/1b8263fa/attachment.html>


More information about the vlc-devel mailing list