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

[Thomas Guillem]

On Tue, Feb 27, 2018, at 15:39, Rémi Denis-Courmont wrote:
> 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.

Ah yes, I can do it more simply.

> -- 
> Remi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel