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
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
> I'm sorry but I've wasted enough time on this. I believe I'm still leaving more than enough time to give some feedback, either commenting on the code or even rejecting the changes for whatever reason you might find appropriate.
I'm sorry for not reviewing I find it very difficult to want to review promptly when my review is invariably met with argument, insistence that there is only one correct way to do things, and inability or unwillingness to view things from my perspective. Despite this, I am sorry, and I will try to renew my efforts to review better.
If you wish to have me removed as maintainer of winegstreamer, please propose that directly, rather than abusing GitLab to get your patches committed.
As far as comment on this patch goes, I'm sorry if this is not quite as baked as I'd like it to be, but:
* Renaming "preferred" to "current" is certainly true as far as the functionality of the code is, but it goes against its intent. The point of the "preferred" format is that it should be the one that does not require videoconvert, and therefore offers the best latency. (I know you come at things with a Media Foundation perspective, where the format is pretty much always specified by the application, but for DirectShow it's quite common to plug in a file and autoplug system components up to and including the renderer, so using GStreamer's preferred format is often perfectly feasible.) Hence I would rather see the code fixed so that the "preferred" caps remains what it is, and is not updated.
* From what I've understood, getting rid of wg_format by itself does not save that many LoC. I'm still not convinced that it's a desirable change to make. I've already told you my intentions for wg_format, and despite the fact that you dismissed them as false, claiming it's an ad-hoc struct, I still believe that it's a desirable IR. I'm not convinced that the tradeoffs that you're trying to make are good ones. I'm not convinced they're *not*, either, which is part of why I hadn't said anything yet.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5667#note_71430
Squad depends on that to properly fill audio capture device list for voice chat and handle capture device name change. The game uses mmdevapi objects for actual capturing so that fully works with this implementation (which is yet missing the actual media source implementation).
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5734#note_71429
On Mon May 27 23:57:31 2024 +0000, Jinoh Kang wrote:
> If you don't want to go through all the hassle, but still want to go "in
> the right direction," please read my first comment.
How's this? I don't think I need to translate the the SID itself
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5721#note_71426
On Mon May 27 23:57:31 2024 +0000, Jinoh Kang wrote:
> The `TOKEN_MANDATORY_LABEL` structure pointed by `ptr`, with length
> `len`. This is documented in https://learn.microsoft.com/en-us/windows/win32/api/winnt/ne-winnt-token_in….
> You need to define both `TOKEN_MANDATORY_LEVEL32` (with ULONG in place
> of pointers) and `TOKEB_MANDATORY_LABEL` (with actual pointers).
> Also, don't assume `ptr` is not "used." Maybe *right now,* but ntdll and
> wow64 are still separate components that can get out of sync without tests.
If you don't want to go through all the hassle, but still want to go "in the right direction," please read my first comment.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5721#note_71423