-- v2: mmdevapi/tests: Add test for capturing render loopback. winepulse.drv: Implement pulse_get_loopback_capture_device().
From: Paul Gofman pgofman@codeweavers.com
--- dlls/mmdevapi/client.c | 9 ++++----- dlls/mmdevapi/mmdevdrv.h | 4 +--- 2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index ed635bcf98a..6ee010fd3a6 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -526,6 +526,7 @@ static ULONG WINAPI client_Release(IAudioClient3 *iface) if (This->stream) stream_release(This->stream, This->timer_thread);
+ free(This->device_name); free(This); }
@@ -1462,7 +1463,6 @@ HRESULT AudioClient_Create(GUID *guid, IMMDevice *device, IAudioClient **out) struct audio_client *This; char *name; EDataFlow dataflow; - size_t size; HRESULT hr;
TRACE("%s %p %p\n", debugstr_guid(guid), device, out); @@ -1477,15 +1477,13 @@ HRESULT AudioClient_Create(GUID *guid, IMMDevice *device, IAudioClient **out) return E_UNEXPECTED; }
- size = strlen(name) + 1; - This = calloc(1, FIELD_OFFSET(struct audio_client, device_name[size])); + This = calloc(1, sizeof(*This)); if (!This) { free(name); return E_OUTOFMEMORY; }
- memcpy(This->device_name, name, size); - free(name); + This->device_name = name;
This->IAudioCaptureClient_iface.lpVtbl = &AudioCaptureClient_Vtbl; This->IAudioClient3_iface.lpVtbl = &AudioClient3_Vtbl; @@ -1499,6 +1497,7 @@ HRESULT AudioClient_Create(GUID *guid, IMMDevice *device, IAudioClient **out)
hr = CoCreateFreeThreadedMarshaler((IUnknown *)&This->IAudioClient3_iface, &This->marshal); if (FAILED(hr)) { + free(This->device_name); free(This); return hr; } diff --git a/dlls/mmdevapi/mmdevdrv.h b/dlls/mmdevapi/mmdevdrv.h index 42b01443ff0..7683867a9c1 100644 --- a/dlls/mmdevapi/mmdevdrv.h +++ b/dlls/mmdevapi/mmdevdrv.h @@ -80,7 +80,5 @@ struct audio_client { struct audio_session_wrapper *session_wrapper;
struct list entry; - - /* Keep at end */ - char device_name[0]; + char *device_name; };
From: Paul Gofman pgofman@codeweavers.com
--- dlls/mmdevapi/client.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index 6ee010fd3a6..ad7cc9ff2ab 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -569,9 +569,6 @@ static HRESULT WINAPI client_Initialize(IAudioClient3 *iface, AUDCLNT_SHAREMODE return E_INVALIDARG; }
- if (FAILED(params.result = adjust_timing(This, &duration, &period, mode, flags, fmt))) - return params.result; - sessions_lock();
if (This->stream) { @@ -584,6 +581,12 @@ static HRESULT WINAPI client_Initialize(IAudioClient3 *iface, AUDCLNT_SHAREMODE return params.result; }
+ if (FAILED(params.result = adjust_timing(This, &duration, &period, mode, flags, fmt))) + { + sessions_unlock(); + return params.result; + } + params.name = name = get_application_name(); params.device = This->device_name; params.flow = This->dataflow;
From: Paul Gofman pgofman@codeweavers.com
--- dlls/mmdevapi/client.c | 39 ++++++++++++++++++++++++++++++ dlls/mmdevapi/unixlib.h | 10 ++++++++ dlls/winealsa.drv/alsa.c | 2 ++ dlls/winecoreaudio.drv/coreaudio.c | 2 ++ dlls/wineoss.drv/oss.c | 2 ++ dlls/winepulse.drv/pulse.c | 2 ++ 6 files changed, 57 insertions(+)
diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index ad7cc9ff2ab..5f6357a3bb9 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -21,6 +21,9 @@
#define COBJMACROS
+#include "ntstatus.h" +#define WIN32_NO_STATUS + #include <wchar.h>
#include <audiopolicy.h> @@ -581,6 +584,42 @@ static HRESULT WINAPI client_Initialize(IAudioClient3 *iface, AUDCLNT_SHAREMODE return params.result; }
+ if (flags & AUDCLNT_STREAMFLAGS_LOOPBACK) + { + struct get_loopback_capture_device_params params; + + if (This->dataflow != eRender) + { + sessions_unlock(); + return AUDCLNT_E_WRONG_ENDPOINT_TYPE; + } + + params.device = This->device_name; + params.name = name = get_application_name(); + params.ret_device_len = 0; + params.ret_device = NULL; + params.result = E_NOTIMPL; + wine_unix_call(get_loopback_capture_device, ¶ms); + while (params.result == STATUS_BUFFER_TOO_SMALL) + { + free(params.ret_device); + params.ret_device = malloc(params.ret_device_len); + wine_unix_call(get_loopback_capture_device, ¶ms); + } + free(name); + if (FAILED(params.result)) + { + sessions_unlock(); + free(params.ret_device); + if (params.result == E_NOTIMPL) + FIXME("get_loopback_capture_device is not supported by backend.\n"); + return params.result; + } + free(This->device_name); + This->device_name = params.ret_device; + This->dataflow = eCapture; + } + if (FAILED(params.result = adjust_timing(This, &duration, &period, mode, flags, fmt))) { sessions_unlock(); diff --git a/dlls/mmdevapi/unixlib.h b/dlls/mmdevapi/unixlib.h index d83ed918a51..4cb4c881bcf 100644 --- a/dlls/mmdevapi/unixlib.h +++ b/dlls/mmdevapi/unixlib.h @@ -140,6 +140,15 @@ struct is_format_supported_params HRESULT result; };
+struct get_loopback_capture_device_params +{ + const WCHAR *name; + const char *device; + char *ret_device; + UINT32 ret_device_len; + HRESULT result; +}; + struct get_mix_format_params { const char *device; @@ -313,6 +322,7 @@ enum unix_funcs get_capture_buffer, release_capture_buffer, is_format_supported, + get_loopback_capture_device, get_mix_format, get_device_period, get_buffer_size, diff --git a/dlls/winealsa.drv/alsa.c b/dlls/winealsa.drv/alsa.c index a24cb56d1d8..970db59ad12 100644 --- a/dlls/winealsa.drv/alsa.c +++ b/dlls/winealsa.drv/alsa.c @@ -2491,6 +2491,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = alsa_get_capture_buffer, alsa_release_capture_buffer, alsa_is_format_supported, + alsa_not_implemented, alsa_get_mix_format, alsa_get_device_period, alsa_get_buffer_size, @@ -2947,6 +2948,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = alsa_wow64_get_capture_buffer, alsa_release_capture_buffer, alsa_wow64_is_format_supported, + alsa_not_implemented, alsa_wow64_get_mix_format, alsa_wow64_get_device_period, alsa_wow64_get_buffer_size, diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c index deb9df1b45a..db5e1116bf8 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -1837,6 +1837,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = unix_get_capture_buffer, unix_release_capture_buffer, unix_is_format_supported, + unix_not_implemented, unix_get_mix_format, unix_get_device_period, unix_get_buffer_size, @@ -2292,6 +2293,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = unix_wow64_get_capture_buffer, unix_release_capture_buffer, unix_wow64_is_format_supported, + unix_not_implemented, unix_wow64_get_mix_format, unix_wow64_get_device_period, unix_wow64_get_buffer_size, diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c index 819e876606c..a017d242a98 100644 --- a/dlls/wineoss.drv/oss.c +++ b/dlls/wineoss.drv/oss.c @@ -1691,6 +1691,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = oss_get_capture_buffer, oss_release_capture_buffer, oss_is_format_supported, + oss_not_implemented, oss_get_mix_format, oss_get_device_period, oss_get_buffer_size, @@ -2186,6 +2187,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = oss_wow64_get_capture_buffer, oss_release_capture_buffer, oss_wow64_is_format_supported, + oss_not_implemented, oss_wow64_get_mix_format, oss_wow64_get_device_period, oss_wow64_get_buffer_size, diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 068245f03b2..9d3363e2638 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2536,6 +2536,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = pulse_get_capture_buffer, pulse_release_capture_buffer, pulse_is_format_supported, + pulse_not_implemented, pulse_get_mix_format, pulse_get_device_period, pulse_get_buffer_size, @@ -3007,6 +3008,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = pulse_wow64_get_capture_buffer, pulse_release_capture_buffer, pulse_wow64_is_format_supported, + pulse_not_implemented, pulse_wow64_get_mix_format, pulse_wow64_get_device_period, pulse_wow64_get_buffer_size,
From: Paul Gofman pgofman@codeweavers.com
--- dlls/winepulse.drv/pulse.c | 42 ++++++++++++++------------------------ 1 file changed, 15 insertions(+), 27 deletions(-)
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 9d3363e2638..726a86e299d 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -203,6 +203,16 @@ static char *wstr_to_str(const WCHAR *wstr) return str; }
+static void wait_pa_operation_complete(pa_operation *o) +{ + if (!o) + return; + + while (pa_operation_get_state(o) == PA_OPERATION_RUNNING) + pulse_cond_wait(); + pa_operation_unref(o); +} + /* Following pulseaudio design here, mainloop has the lock taken whenever * it is handling something for pulse, and the lock is required whenever * doing any pa_* call that can affect the state in any way @@ -1548,7 +1558,6 @@ static NTSTATUS pulse_timer_loop(void *args) pa_usec_t last_time; UINT32 adv_bytes; int success; - pa_operation *o;
pulse_lock(); delay.QuadPart = -stream->mmdev_period_usec * 10; @@ -1566,13 +1575,7 @@ static NTSTATUS pulse_timer_loop(void *args)
delay.QuadPart = -stream->mmdev_period_usec * 10;
- o = pa_stream_update_timing_info(stream->stream, pulse_op_cb, &success); - if (o) - { - while (pa_operation_get_state(o) == PA_OPERATION_RUNNING) - pulse_cond_wait(); - pa_operation_unref(o); - } + wait_pa_operation_complete(pa_stream_update_timing_info(stream->stream, pulse_op_cb, &success)); err = pa_stream_get_time(stream->stream, &now); if (err == 0) { @@ -1684,11 +1687,7 @@ static NTSTATUS pulse_start(void *args) { o = pa_stream_cork(stream->stream, 0, pulse_op_cb, &success); if (o) - { - while(pa_operation_get_state(o) == PA_OPERATION_RUNNING) - pulse_cond_wait(); - pa_operation_unref(o); - } + wait_pa_operation_complete(o); else success = 0; if (!success) @@ -1731,11 +1730,7 @@ static NTSTATUS pulse_stop(void *args) { o = pa_stream_cork(stream->stream, 1, pulse_op_cb, &success); if (o) - { - while(pa_operation_get_state(o) == PA_OPERATION_RUNNING) - pulse_cond_wait(); - pa_operation_unref(o); - } + wait_pa_operation_complete(o); else success = 0; if (!success) @@ -1779,15 +1774,8 @@ static NTSTATUS pulse_reset(void *args) /* If there is still data in the render buffer it needs to be removed from the server */ int success = 0; if (stream->held_bytes) - { - pa_operation *o = pa_stream_flush(stream->stream, pulse_op_cb, &success); - if (o) - { - while (pa_operation_get_state(o) == PA_OPERATION_RUNNING) - pulse_cond_wait(); - pa_operation_unref(o); - } - } + wait_pa_operation_complete(pa_stream_flush(stream->stream, pulse_op_cb, &success)); + if (success || !stream->held_bytes) { stream->clock_lastpos = stream->clock_written = 0;
From: Paul Gofman pgofman@codeweavers.com
--- dlls/winepulse.drv/pulse.c | 112 ++++++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 2 deletions(-)
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 726a86e299d..2c2a7d1f05b 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2206,6 +2206,89 @@ static NTSTATUS pulse_is_format_supported(void *args) return STATUS_SUCCESS; }
+static void sink_name_info_cb(pa_context *c, const pa_sink_info *i, int eol, void *userdata) +{ + uint32_t *current_device_index = userdata; + pulse_broadcast(); + + if (!i || !i->name || !i->name[0]) + return; + *current_device_index = i->index; +} + +struct find_monitor_of_sink_cb_param +{ + struct get_loopback_capture_device_params *params; + uint32_t current_device_index; +}; + +static void find_monitor_of_sink_cb(pa_context *c, const pa_source_info *i, int eol, void *userdata) +{ + struct find_monitor_of_sink_cb_param *p = userdata; + unsigned int len; + + pulse_broadcast(); + + if (!i || !i->name || !i->name[0]) + return; + if (i->monitor_of_sink != p->current_device_index) + return; + + len = strlen(i->name) + 1; + if (len <= p->params->ret_device_len) + { + memcpy(p->params->ret_device, i->name, len); + p->params->result = STATUS_SUCCESS; + return; + } + p->params->ret_device_len = len; + p->params->result = STATUS_BUFFER_TOO_SMALL; +} + +static NTSTATUS pulse_get_loopback_capture_device(void *args) +{ + struct get_loopback_capture_device_params *params = args; + uint32_t current_device_index = PA_INVALID_INDEX; + struct find_monitor_of_sink_cb_param p; + const char *device_name; + char *name; + + pulse_lock(); + + if (!pulse_ml) + { + pulse_unlock(); + ERR("Called without main loop running.\n"); + params->result = E_INVALIDARG; + return STATUS_SUCCESS; + } + + name = wstr_to_str(params->name); + params->result = pulse_connect(name); + free(name); + + if (FAILED(params->result)) + { + pulse_unlock(); + return STATUS_SUCCESS; + } + + device_name = params->device; + if (device_name && !device_name[0]) device_name = NULL; + + params->result = E_FAIL; + wait_pa_operation_complete(pa_context_get_sink_info_by_name(pulse_ctx, device_name, &sink_name_info_cb, ¤t_device_index)); + if (current_device_index != PA_INVALID_INDEX) + { + p.current_device_index = current_device_index; + p.params = params; + wait_pa_operation_complete(pa_context_get_source_info_list(pulse_ctx, &find_monitor_of_sink_cb, &p)); + } + + pulse_unlock(); + return STATUS_SUCCESS; +} + static NTSTATUS pulse_get_mix_format(void *args) { struct get_mix_format_params *params = args; @@ -2524,7 +2607,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = pulse_get_capture_buffer, pulse_release_capture_buffer, pulse_is_format_supported, - pulse_not_implemented, + pulse_get_loopback_capture_device, pulse_get_mix_format, pulse_get_device_period, pulse_get_buffer_size, @@ -2716,6 +2799,31 @@ static NTSTATUS pulse_wow64_is_format_supported(void *args) return STATUS_SUCCESS; }
+static NTSTATUS pulse_wow64_get_loopback_capture_device(void *args) +{ + struct + { + PTR32 name; + PTR32 device; + PTR32 ret_device; + UINT32 ret_device_len; + HRESULT result; + } *params32 = args; + + struct get_loopback_capture_device_params params = + { + .name = ULongToPtr(params32->name), + .device = ULongToPtr(params32->device), + .ret_device = ULongToPtr(params32->ret_device), + .ret_device_len = params32->ret_device_len, + }; + + pulse_get_loopback_capture_device(¶ms); + params32->result = params.result; + params32->ret_device_len = params.ret_device_len; + return STATUS_SUCCESS; +} + static NTSTATUS pulse_wow64_get_mix_format(void *args) { struct @@ -2996,7 +3104,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = pulse_wow64_get_capture_buffer, pulse_release_capture_buffer, pulse_wow64_is_format_supported, - pulse_not_implemented, + pulse_wow64_get_loopback_capture_device, pulse_wow64_get_mix_format, pulse_wow64_get_device_period, pulse_wow64_get_buffer_size,
From: Paul Gofman pgofman@codeweavers.com
--- dlls/mmdevapi/tests/capture.c | 62 +++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/dlls/mmdevapi/tests/capture.c b/dlls/mmdevapi/tests/capture.c index 08ab730ef1a..e3ae87fd9bd 100644 --- a/dlls/mmdevapi/tests/capture.c +++ b/dlls/mmdevapi/tests/capture.c @@ -41,7 +41,8 @@ /* undocumented error code */ #define D3D11_ERROR_4E MAKE_HRESULT(SEVERITY_ERROR, FACILITY_DIRECT3D11, 0x4e)
-static IMMDevice *dev = NULL; +static IMMDeviceEnumerator *mme; +static IMMDevice *dev; static const LARGE_INTEGER ullZero;
static void test_uninitialized(IAudioClient *ac) @@ -1012,10 +1013,66 @@ static void test_marshal(void)
}
+static void test_render_loopback(void) +{ + IAudioCaptureClient *capture; + IAudioRenderClient *render; + WAVEFORMATEX *pwfx; + IMMDevice *device; + IAudioClient *ac; + HRESULT hr; + + hr = IMMDeviceEnumerator_GetDefaultAudioEndpoint(mme, eRender, eMultimedia, &device); + ok(hr == S_OK || hr == E_NOTFOUND, "GetDefaultAudioEndpoint failed: %#lx.\n", hr); + + if (hr != S_OK || !dev) + { + if (hr == E_NOTFOUND) + skip("No sound card available\n"); + else + skip("GetDefaultAudioEndpoint returns 0x%08lx\n", hr); + return; + } + + hr = IMMDevice_Activate(dev, &IID_IAudioClient, CLSCTX_INPROC_SERVER, NULL, (void**)&ac); + ok(hr == S_OK, "got %#lx.\n", hr); + + hr = IAudioClient_GetMixFormat(ac, &pwfx); + ok(hr == S_OK, "got %#lx.\n", hr); + + hr = IAudioClient_Initialize(ac, AUDCLNT_SHAREMODE_SHARED, + AUDCLNT_STREAMFLAGS_EVENTCALLBACK | AUDCLNT_STREAMFLAGS_LOOPBACK, 5000000, 0, pwfx, NULL); + ok(hr == AUDCLNT_E_WRONG_ENDPOINT_TYPE, "got %#lx.\n", hr); + IAudioClient_Release(ac); + CoTaskMemFree(pwfx); + + hr = IMMDevice_Activate(device, &IID_IAudioClient, CLSCTX_INPROC_SERVER, NULL, (void**)&ac); + ok(hr == S_OK, "got %#lx.\n", hr); + + hr = IAudioClient_GetMixFormat(ac, &pwfx); + ok(hr == S_OK, "got %#lx.\n", hr); + + hr = IAudioClient_Initialize(ac, AUDCLNT_SHAREMODE_SHARED, AUDCLNT_STREAMFLAGS_LOOPBACK, 5000000, 0, pwfx, NULL); + todo_wine_if(hr == E_NOTIMPL) ok(hr == S_OK, "got %#lx.\n", hr); + if (FAILED(hr)) + goto done; + + hr = IAudioClient_GetService(ac, &IID_IAudioRenderClient, (void **)&render); + ok(hr == AUDCLNT_E_WRONG_ENDPOINT_TYPE, "got %#lx.\n", hr); + + hr = IAudioClient_GetService(ac, &IID_IAudioCaptureClient, (void **)&capture); + ok(hr == S_OK, "got %#lx.\n", hr); + IAudioCaptureClient_Release(capture); + +done: + IAudioClient_Release(ac); + IMMDevice_Release(device); + CoTaskMemFree(pwfx); +} + START_TEST(capture) { HRESULT hr; - IMMDeviceEnumerator *mme = NULL;
CoInitializeEx(NULL, COINIT_MULTITHREADED); hr = CoCreateInstance(&CLSID_MMDeviceEnumerator, NULL, CLSCTX_INPROC_SERVER, &IID_IMMDeviceEnumerator, (void**)&mme); @@ -1042,6 +1099,7 @@ START_TEST(capture) test_simplevolume(); test_volume_dependence(); test_marshal(); + test_render_loopback();
IMMDevice_Release(dev);
v2: - fix wow64 thunk.
Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/client.c:
return params.result; }
- if (FAILED(params.result = adjust_timing(This, &duration, &period, mode, flags, fmt)))
- {
```suggestion:-1+0 if (FAILED(params.result = adjust_timing(This, &duration, &period, mode, flags, fmt))) { ```
Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/client.c:
return params.result; }
- if (flags & AUDCLNT_STREAMFLAGS_LOOPBACK)
- {
```suggestion:-1+0 if (flags & AUDCLNT_STREAMFLAGS_LOOPBACK) { ```
Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/client.c:
return params.result; }
- if (flags & AUDCLNT_STREAMFLAGS_LOOPBACK)
- {
struct get_loopback_capture_device_params params;if (This->dataflow != eRender){
```suggestion:-1+0 if (This->dataflow != eRender) { ```
Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/client.c:
struct get_loopback_capture_device_params params;if (This->dataflow != eRender){sessions_unlock();return AUDCLNT_E_WRONG_ENDPOINT_TYPE;}params.device = This->device_name;params.name = name = get_application_name();params.ret_device_len = 0;params.ret_device = NULL;params.result = E_NOTIMPL;wine_unix_call(get_loopback_capture_device, ¶ms);while (params.result == STATUS_BUFFER_TOO_SMALL){
```suggestion:-1+0 while (params.result == STATUS_BUFFER_TOO_SMALL) { ```
Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/client.c:
params.device = This->device_name;params.name = name = get_application_name();params.ret_device_len = 0;params.ret_device = NULL;params.result = E_NOTIMPL;wine_unix_call(get_loopback_capture_device, ¶ms);while (params.result == STATUS_BUFFER_TOO_SMALL){free(params.ret_device);params.ret_device = malloc(params.ret_device_len);wine_unix_call(get_loopback_capture_device, ¶ms);}free(name);if (FAILED(params.result)){
```suggestion:-1+0 if (FAILED(params.result)) { ```
Davide Beatrici (@davidebeatrici) commented about dlls/winepulse.drv/pulse.c:
return str;}
+static void wait_pa_operation_complete(pa_operation *o)
```suggestion:-0+0 static void wait_pa_operation_finish(pa_operation *o) ```
May look better.
Davide Beatrici (@davidebeatrici) commented about dlls/winepulse.drv/pulse.c:
struct start_params *params = args; struct pulse_stream *stream = handle_get_stream(params->stream); int success; pa_operation *o;
```suggestion:-0+0 ```
Davide Beatrici (@davidebeatrici) commented about dlls/winepulse.drv/pulse.c:
{ o = pa_stream_cork(stream->stream, 0, pulse_op_cb, &success); if (o)
{while(pa_operation_get_state(o) == PA_OPERATION_RUNNING)pulse_cond_wait();pa_operation_unref(o);}
wait_pa_operation_complete(o);
```suggestion:-2+0 wait_pa_operation_complete(pa_stream_cork(stream->stream, 0, pulse_op_cb, &success)); ```
Davide Beatrici (@davidebeatrici) commented about dlls/winepulse.drv/pulse.c:
{ struct stop_params *params = args; struct pulse_stream *stream = handle_get_stream(params->stream); pa_operation *o;
```suggestion:-0+0 ```
Davide Beatrici (@davidebeatrici) commented about dlls/winepulse.drv/pulse.c:
{ o = pa_stream_cork(stream->stream, 1, pulse_op_cb, &success); if (o)
{while(pa_operation_get_state(o) == PA_OPERATION_RUNNING)pulse_cond_wait();pa_operation_unref(o);}
wait_pa_operation_complete(o);
```suggestion:-2+0 wait_pa_operation_complete(pa_stream_cork(stream->stream, 1, pulse_op_cb, &success)); ```
On Sat Jun 22 01:01:13 2024 +0000, Davide Beatrici wrote:
wait_pa_operation_complete(pa_stream_cork(stream->stream, 1, pulse_op_cb, &success));
But there is else below? It is hard to look at diff in gitlab, but I grepped the code for pa_stream_cork() and there is always the else.
On Sat Jun 22 01:01:01 2024 +0000, Davide Beatrici wrote:
if (FAILED(params.result)) {
Eh, I know that winpulse.drv has this special style distinct from majority of other Wine code, but unless Huw advises otherwise I'd prefer fixing that on the occasion rather than contributing to that.
On Sat Jun 22 01:16:31 2024 +0000, Paul Gofman wrote:
v2:
- fix wow64 thunk.
Can we move the thunks to `mmdevapi` somehow? That way we don't duplicate them across backends...
If I recall correctly @huw suggested that in a merge request of mine.
On Sat Jun 22 01:16:31 2024 +0000, Davide Beatrici wrote:
Can we move the thunks to `mmdevapi` somehow? That way we don't duplicate them across backends... If I recall correctly @huw suggested that in a merge request of mine.
Do you have some reference so I could see the suggestion? Generally I don't think we can exactly move the thunks, logically that belongs to Unix backend. What could be done is using 32 / 64 bit compatible structures (at the cost of additional casts in mmdevapi on every parameter structure fill), but I am not sure if that is a clear win: it will avoid duplicating thunk but will be prone to errors in struct definition, it is easy to make those incompatible. Anyway, I guess it is a bit orthogonal to my MR and also we should probably stick to the same way of doing it throughout the modules?
On Sat Jun 22 01:13:55 2024 +0000, Paul Gofman wrote:
But there is else below? It is hard to look at diff in gitlab, but I grepped the code for pa_stream_cork() and there is always the else.
Sorry, you're right.
I propose to make `wait_pa_operation_complete()` return a `BOOLEAN` value: `TRUE` for success and `FALSE` for failure. Then we can use that to handle errors that may occur with such operations.
On Sat Jun 22 01:16:11 2024 +0000, Paul Gofman wrote:
Eh, I know that winpulse.drv has this special style distinct from majority of other Wine code, but unless Huw advises otherwise I'd prefer fixing that on the occasion rather than contributing to that.
I actually thought the one I suggested is the preferred style.
@huw
Do you have some reference so I could see the suggestion?
Unfortunately not, but the first way that comes to mind would be to move the thunks into a source file in `mmdevapi` that is then compiled by each backend.
Generally I don't think we can exactly move the thunks, logically that belongs to Unix backend.
Anyway, I guess it is a bit orthogonal to my MR and also we should probably stick to the same way of doing it throughout the modules?
The `mmdevapi` design is special compared to a common module: the unixlib definition already resides in `mmdevapi` because it's shared across backends (that are then loaded on demand).
I agree that the thunks should be moved in a dedicated MR.
-
Davide Beatrici (＠davidebeatrici) -
Paul Gofman
-
Paul Gofman (＠gofman)