May 2024 - Wine-GitLab - list.winehq.org

Re: [PATCH v2 0/2] MR5729: Fix SQLSetConnectOptionW prototype - approved
by Alexandre Julliard (＠julliard) May 28, 2024

May 28, 2024

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5729

1 0

Re: [PATCH v2 0/2] MR5734: mf: Implement audio capture device enumeration in MFEnumDeviceSources().
by Paul Gofman (＠gofman) May 28, 2024

May 28, 2024

On Mon May 27 20:51:50 2024 +0000, Nikolay Sivov wrote: > What does "sac" stand for? Source Audio Capture. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71442

1 0

Re: [PATCH v2 0/2] MR5734: mf: Implement audio capture device enumeration in MFEnumDeviceSources().
by Nikolay Sivov (＠nsivov) May 28, 2024

May 28, 2024

Nikolay Sivov (@nsivov) commented about dlls/mf/sac.c: > + > + hr = IMMDevice_OpenPropertyStore(device, STGM_READ, &ps); > + IMMDevice_Release(device); > + if (FAILED(hr)) > + goto done; > + hr = IPropertyStore_GetValue(ps, (const PROPERTYKEY*)&DEVPKEY_Device_FriendlyName, &pv); > + IPropertyStore_Release(ps); > + if (FAILED(hr)) > + goto done; > + if (pv.vt != VT_LPWSTR) > + { > + PropVariantClear(&pv); > + goto done; > + } > + IMFActivate_SetString(sources[i], &MF_DEVSOURCE_ATTRIBUTE_FRIENDLY_NAME, pv.pwszVal); > + PropVariantClear(&pv); I think cascading checks for hr are better here than multiple jumps. And if property store becomes useful for instance id, we could have a helper that checks value types. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71440

1 0

Re: [PATCH v2 0/2] MR5734: mf: Implement audio capture device enumeration in MFEnumDeviceSources().
by Nikolay Sivov (＠nsivov) May 28, 2024

May 28, 2024

Nikolay Sivov (@nsivov) commented about dlls/mf/main.c: > - FIXME("%p, %p, %p.\n", attributes, sources, count); > + GUID source_type; > + HRESULT hr; > + > + TRACE("%p, %p, %p.\n", attributes, sources, ret_count); > > - if (!attributes || !sources || !count) > + if (!attributes || !sources || !ret_count) > return E_INVALIDARG; > > - *count = 0; > + if (FAILED(hr = IMFAttributes_GetGUID(attributes, &MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE, &source_type))) > + return hr; > > - return S_OK; > + if (IsEqualIID(&source_type, &MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE_VIDCAP_GUID)) It makes no difference in practice, but IsEqualGUID() is a better fit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71439

1 0

Re: [PATCH v2 0/2] MR5734: mf: Implement audio capture device enumeration in MFEnumDeviceSources().
by Nikolay Sivov (＠nsivov) May 28, 2024

May 28, 2024

Nikolay Sivov (@nsivov) commented about dlls/mf/main.c: > + HRESULT hr; > + > + TRACE("%p, %p, %p.\n", attributes, sources, ret_count); > > - if (!attributes || !sources || !count) > + if (!attributes || !sources || !ret_count) > return E_INVALIDARG; > > - *count = 0; > + if (FAILED(hr = IMFAttributes_GetGUID(attributes, &MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE, &source_type))) > + return hr; > > - return S_OK; > + if (IsEqualIID(&source_type, &MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE_VIDCAP_GUID)) > + { > + FIXME("Not implementted for video capture devices.\n"); Typo here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71438

1 0

Re: [PATCH v2 0/2] MR5734: mf: Implement audio capture device enumeration in MFEnumDeviceSources().
by Nikolay Sivov (＠nsivov) May 28, 2024

May 28, 2024

Nikolay Sivov (@nsivov) commented about dlls/mf/Makefile.in: > mf.rc \ > quality.c \ > samplegrabber.c \ > + sac.c \ What does "sac" stand for? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71437

1 0

Re: [PATCH v2 0/2] MR5734: mf: Implement audio capture device enumeration in MFEnumDeviceSources().
by Nikolay Sivov (＠nsivov) May 28, 2024

May 28, 2024

Nikolay Sivov (@nsivov) commented about dlls/mf/sac.c: > + if (FAILED(hr = IMMDeviceCollection_GetCount(devices, &count))) > + goto done; > + sources = CoTaskMemAlloc(count * sizeof(*sources)); > + for (i = 0; i < count; ++i) > + { > + WCHAR symbolic_link[256]; > + IPropertyStore *ps; > + IMMDevice *device; > + PROPVARIANT pv; > + WCHAR *str; > + > + if (FAILED(hr = create_activation_object(NULL, &audio_capture_activate_funcs, &sources[i]))) > + goto done; > + ++created_count; > + > + IMFActivate_SetGUID(sources[i], &MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE, &MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE_AUDCAP_GUID); To handle failures in individual Set methods we have things like mediatype_set_guid() in mfplat, that take &hr argument to set error code appropriately without introducing more cleanup conditions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71436

1 0

Re: [PATCH v2 0/2] MR5734: mf: Implement audio capture device enumeration in MFEnumDeviceSources().
by Nikolay Sivov (＠nsivov) May 28, 2024

May 28, 2024

Nikolay Sivov (@nsivov) commented about dlls/mf/sac.c: > +static void audio_capture_free_private(void *user_context) > +{ > + TRACE("%p.\n", user_context); > +} > + > +static const struct activate_funcs audio_capture_activate_funcs = > +{ > + audio_capture_create_object, > + audio_capture_shutdown_object, > + audio_capture_free_private, > +}; > + > +HRESULT enum_audio_capture_sources(IMFAttributes *attributes, IMFActivate ***ret_sources, UINT32 *ret_count) > +{ > + static const WCHAR devinterface_audio_capture_wstr[] = L"{2eef81be-33fa-4800-9670-1cd474972c3f}"; > + static const WCHAR mmdev_path_prefix[] = L"\\\\?\\SWD#MMDEVAPI"; It's unlikely those device strings need to be constructed in client library here, I'd expect those id to come from setupapi (DEVPKEY_Device_InstanceId?), if they are not present in mmdevice property store already. If application really depends on those attributes we could add them here as a temporary solution, but if it doesn't, I'd rather prefer a fixme. Device symlink is required for video sources I think, but for mmdevapi you'd use an endpoint id. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71435

1 0

Re: [PATCH v2 0/6] MR5724: widl: Various fixes.
by Elizabeth Figura (＠zfigura) May 28, 2024

May 28, 2024

On Mon May 27 09:51:12 2024 +0000, Rémi Bernon wrote: > What about decoupling this from the definition status? Something like `is_imported`? I'm not sure I see how? If a type is just declared in an import, and defined in the main IDL, then we still want to define it. I don't see how to distinguish those two cases. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5724#note_71433

1 0

Re: [PATCH v4 0/1] MR5727: riched20: In EM_SETPARAFORMAT protect against out of bound cTabStop values
by Nikolay Sivov (＠nsivov) May 28, 2024

May 28, 2024

Looks fine to me. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5727#note_71432

1 0