[vlc-devel] [PATCH 3/7] mmdevice: split DeviceHotplugReport()

[Rémi Denis-Courmont]

Le 27 février 2018 15:13:20 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>
>On Tue, Feb 27, 2018, at 13:55, Rémi Denis-Courmont wrote:
>> Le 27 février 2018 12:41:07 GMT+02:00, Thomas Guillem
><thomas at gllm.fr> a écrit :
>> >Add DeviceGetFriendlyName() that can be used to fetch the friendly
>> >name.
>> >---
>> >modules/audio_output/mmdevice.c | 45
>> >+++++++++++++++++++++++++++--------------
>> > 1 file changed, 30 insertions(+), 15 deletions(-)
>> >
>> >diff --git a/modules/audio_output/mmdevice.c
>> >b/modules/audio_output/mmdevice.c
>> >index 525f16ff6a..cc5bbf0eec 100644
>> >--- a/modules/audio_output/mmdevice.c
>> >+++ b/modules/audio_output/mmdevice.c
>> >@@ -470,42 +470,57 @@ static const struct
>> >IAudioVolumeDuckNotificationVtbl vlc_AudioVolumeDuckNotifica
>> > /*** Audio devices ***/
>> > 
>> > /** Gets the user-readable device name */
>> >-static int DeviceHotplugReport(audio_output_t *aout, LPCWSTR wid,
>> >-                               IMMDevice *dev)
>> >+static int DeviceGetFriendlyName(LPCWSTR wid, IMMDevice *dev, char
>> >**id, char **name)
>> > {
>> >     IPropertyStore *props;
>> >-    char *name;
>> >     PROPVARIANT v;
>> >     HRESULT hr;
>> > 
>> >-    char *id = FromWide(wid);
>> >+    *id = FromWide(wid);
>> >     if (unlikely(id == NULL))
>> >-        return VLC_ENOMEM;
>> >+        return VLC_EGENERIC;
>> > 
>> >     hr = IMMDevice_OpenPropertyStore(dev, STGM_READ, &props);
>> >     if (FAILED(hr))
>> >-    {
>> >-        free(id);
>> >-        return VLC_EGENERIC;
>> >-    }
>> >+        goto fallback;
>> > 
>> >     PropVariantInit(&v);
>> >    hr = IPropertyStore_GetValue(props, &PKEY_Device_FriendlyName,
>&v);
>> >     if (SUCCEEDED(hr))
>> >     {
>> >-        name = FromWide(v.pwszVal);
>> >+        *name = FromWide(v.pwszVal);
>> >         PropVariantClear(&v);
>> >     }
>> >     else
>> >-        name = id;
>> >+    {
>> >+        IPropertyStore_Release(props);
>> >+        goto fallback;
>> >+    }
>> > 
>> >     IPropertyStore_Release(props);
>> >-    aout_HotplugReport(aout, id, name);
>> > 
>> >-    free(id);
>> >-    if (id != name)
>> >-        free(name);
>> >     return VLC_SUCCESS;
>> >+fallback:
>> >+
>> >+    *name = strdup(*id);
>> >+    if (!*name)
>> >+    {
>> >+        free(*id);
>> >+        return VLC_EGENERIC;
>> >+    }
>> >+    return VLC_SUCCESS;
>> >+}
>> >+
>> >+static int DeviceHotplugReport(audio_output_t *aout, LPCWSTR wid,
>> >+                               IMMDevice *dev)
>> >+{
>> >+    char *id, *name;
>> >+    int ret = DeviceGetFriendlyName(wid, dev, &id, &name);
>> >+    if (ret == VLC_SUCCESS)
>> >+        aout_HotplugReport(aout, id, name);
>> >+    free(id);
>> >+    free(name);
>> >+    return ret;
>> > }
>> > 
>> > /** Checks that a device is an output device */
>> >-- 
>> >2.11.0
>> >
>> >_______________________________________________
>> >vlc-devel mailing list
>> >To unsubscribe or modify your subscription options:
>> >https://mailman.videolan.org/listinfo/vlc-devel
>> 
>> Splitting the Friendly name lookup is probably OK, but why mix ID 
>> conversion in it??
>
>Because fetching the friendly name can fail. In that case: name = id;
>
>> -- 
>> Remi Denis-Courmont
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

I don't really see how that justifies the functionality aggregation. And you need to handle failure in any case, since ID conversion can fail too.
-- 
Remi Denis-Courmont