[PATCH v2 0/3] MR2888: wine{alsa,coreaudio,oss}: Move stream mode and period/duration initialization logic into unixlib.
-- v2: wineoss: Move stream mode and period/duration initialization logic into unixlib. winecoreaudio: Move stream mode and period/duration initialization logic into unixlib. winealsa: Move stream mode and period/duration initialization logic into unixlib. https://gitlab.winehq.org/wine/wine/-/merge_requests/2888
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/winealsa.drv/alsa.c | 36 ++++++++++++++++++++++++++++- dlls/winealsa.drv/mmdevdrv.c | 44 ++++-------------------------------- 2 files changed, 39 insertions(+), 41 deletions(-) diff --git a/dlls/winealsa.drv/alsa.c b/dlls/winealsa.drv/alsa.c index 2ac126f411d..6daeead46ec 100644 --- a/dlls/winealsa.drv/alsa.c +++ b/dlls/winealsa.drv/alsa.c @@ -85,6 +85,7 @@ struct alsa_stream #define EXTRA_SAFE_RT 40000 static const REFERENCE_TIME def_period = 100000; +static const REFERENCE_TIME min_period = 50000; static const WCHAR drv_keyW[] = {'S','o','f','t','w','a','r','e','\\', 'W','i','n','e','\\','D','r','i','v','e','r','s','\\', @@ -789,10 +790,43 @@ static NTSTATUS alsa_create_stream(void *args) snd_pcm_sw_params_t *sw_params = NULL; snd_pcm_format_t format; unsigned int rate, alsa_period_us, i; - WAVEFORMATEXTENSIBLE *fmtex; + WAVEFORMATEXTENSIBLE *fmtex = (WAVEFORMATEXTENSIBLE *)params->fmt; int err; SIZE_T size; + /* We don't return early as priority is given to the other errors. */ + params->result = *params->stream ? AUDCLNT_E_ALREADY_INITIALIZED : S_OK; + + if (params->share == AUDCLNT_SHAREMODE_SHARED) { + params->period = def_period; + if (params->duration < 3 * params->period) + params->duration = 3 * params->period; + } else { + if (fmtex->Format.wFormatTag == WAVE_FORMAT_EXTENSIBLE && + (fmtex->dwChannelMask == 0 || fmtex->dwChannelMask & SPEAKER_RESERVED)) + params->result = AUDCLNT_E_UNSUPPORTED_FORMAT; + else { + if (!params->period) + params->period = def_period; + if (params->period < min_period || params->period > 5000000) + params->result = AUDCLNT_E_INVALID_DEVICE_PERIOD; + else if (params->duration > 20000000) /* The smaller the period, the lower this limit. */ + params->result = AUDCLNT_E_BUFFER_SIZE_ERROR; + else if (params->flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK) { + if (params->duration != params->period) + params->result = AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; + + FIXME("EXCLUSIVE mode with EVENTCALLBACK\n"); + + params->result = AUDCLNT_E_DEVICE_IN_USE; + } else if (params->duration < 8 * params->period) + params->duration = 8 * params->period; /* May grow above 2s. */ + } + } + + if (FAILED(params->result)) + return STATUS_SUCCESS; + stream = calloc(1, sizeof(*stream)); if(!stream){ params->result = E_OUTOFMEMORY; diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index 0126b1357a2..0baf954150e 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -53,9 +53,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(alsa); #define NULL_PTR_ERR MAKE_HRESULT(SEVERITY_ERROR, FACILITY_WIN32, RPC_X_NULL_REF_POINTER) -static const REFERENCE_TIME DefaultPeriod = 100000; -static const REFERENCE_TIME MinimumPeriod = 50000; - static CRITICAL_SECTION g_sessions_lock; static CRITICAL_SECTION_DEBUG g_sessions_lock_debug = { @@ -581,7 +578,6 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, { ACImpl *This = impl_from_IAudioClient3(iface); struct create_stream_params params; - stream_handle stream; unsigned int i; TRACE("(%p)->(%x, %lx, %s, %s, %p, %s)\n", This, mode, flags, @@ -607,41 +603,8 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return E_INVALIDARG; } - if(mode == AUDCLNT_SHAREMODE_SHARED){ - period = DefaultPeriod; - if( duration < 3 * period) - duration = 3 * period; - }else{ - if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE){ - if(((WAVEFORMATEXTENSIBLE*)fmt)->dwChannelMask == 0 || - ((WAVEFORMATEXTENSIBLE*)fmt)->dwChannelMask & SPEAKER_RESERVED) - return AUDCLNT_E_UNSUPPORTED_FORMAT; - } - - if(!period) - period = DefaultPeriod; /* not minimum */ - if(period < MinimumPeriod || period > 5000000) - return AUDCLNT_E_INVALID_DEVICE_PERIOD; - if(duration > 20000000) /* the smaller the period, the lower this limit */ - return AUDCLNT_E_BUFFER_SIZE_ERROR; - if(flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK){ - if(duration != period) - return AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; - FIXME("EXCLUSIVE mode with EVENTCALLBACK\n"); - return AUDCLNT_E_DEVICE_IN_USE; - }else{ - if( duration < 8 * period) - duration = 8 * period; /* may grow above 2s */ - } - } - sessions_lock(); - if(This->stream){ - sessions_unlock(); - return AUDCLNT_E_ALREADY_INITIALIZED; - } - dump_fmt(fmt); params.name = NULL; @@ -653,7 +616,7 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, params.period = period; params.fmt = fmt; params.channel_count = NULL; - params.stream = &stream; + params.stream = &This->stream; ALSA_CALL(create_stream, ¶ms); if(FAILED(params.result)){ @@ -679,11 +642,12 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, exit: if(FAILED(params.result)){ - alsa_stream_release(stream, NULL); + alsa_stream_release(This->stream, NULL); + This->stream = 0; + HeapFree(GetProcessHeap(), 0, This->vols); This->vols = NULL; }else{ - This->stream = stream; set_stream_volumes(This); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2888
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/winecoreaudio.drv/coreaudio.c | 38 ++++++++++++++++++++++++-- dlls/winecoreaudio.drv/mmdevdrv.c | 44 +++--------------------------- 2 files changed, 40 insertions(+), 42 deletions(-) diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c index 89265558a1d..27ed03cb560 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -649,12 +649,46 @@ static AudioDeviceID dev_id_from_device(const char *device) static NTSTATUS unix_create_stream(void *args) { struct create_stream_params *params = args; - struct coreaudio_stream *stream = calloc(1, sizeof(*stream)); + struct coreaudio_stream *stream; AURenderCallbackStruct input; OSStatus sc; SIZE_T size; - if(!stream){ + /* We don't return early as priority is given to the other errors. */ + params->result = *params->stream ? AUDCLNT_E_ALREADY_INITIALIZED : S_OK; + + if (params->share == AUDCLNT_SHAREMODE_SHARED) { + params->period = def_period; + if (params->duration < 3 * params->period) + params->duration = 3 * params->period; + } else { + const WAVEFORMATEXTENSIBLE *fmtex = (WAVEFORMATEXTENSIBLE *)params->fmt; + if (fmtex->Format.wFormatTag == WAVE_FORMAT_EXTENSIBLE && + (fmtex->dwChannelMask == 0 || fmtex->dwChannelMask & SPEAKER_RESERVED)) + params->result = AUDCLNT_E_UNSUPPORTED_FORMAT; + else { + if (!params->period) + params->period = def_period; + if (params->period < min_period || params->period > 5000000) + params->result = AUDCLNT_E_INVALID_DEVICE_PERIOD; + else if (params->duration > 20000000) /* The smaller the period, the lower this limit. */ + params->result = AUDCLNT_E_BUFFER_SIZE_ERROR; + else if (params->flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK) { + if (params->duration != params->period) + params->result = AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; + + FIXME("EXCLUSIVE mode with EVENTCALLBACK\n"); + + params->result = AUDCLNT_E_DEVICE_IN_USE; + } else if (params->duration < 8 * params->period) + params->duration = 8 * params->period; /* May grow above 2s. */ + } + } + + if (FAILED(params->result)) + return STATUS_SUCCESS; + + if (!(stream = calloc(1, sizeof(*stream)))) { params->result = E_OUTOFMEMORY; return STATUS_SUCCESS; } diff --git a/dlls/winecoreaudio.drv/mmdevdrv.c b/dlls/winecoreaudio.drv/mmdevdrv.c index d8a2e9e5d21..e15569dd2ef 100644 --- a/dlls/winecoreaudio.drv/mmdevdrv.c +++ b/dlls/winecoreaudio.drv/mmdevdrv.c @@ -48,9 +48,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(coreaudio); #define NULL_PTR_ERR MAKE_HRESULT(SEVERITY_ERROR, FACILITY_WIN32, RPC_X_NULL_REF_POINTER) -static const REFERENCE_TIME DefaultPeriod = 100000; -static const REFERENCE_TIME MinimumPeriod = 50000; - static const IAudioClient3Vtbl AudioClient3_Vtbl; extern const IAudioRenderClientVtbl AudioRenderClient_Vtbl; extern const IAudioCaptureClientVtbl AudioCaptureClient_Vtbl; @@ -554,7 +551,6 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, ACImpl *This = impl_from_IAudioClient3(iface); struct release_stream_params release_params; struct create_stream_params params; - stream_handle stream; UINT32 i; TRACE("(%p)->(%x, %lx, %s, %s, %p, %s)\n", This, mode, flags, @@ -582,41 +578,8 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return E_INVALIDARG; } - if(mode == AUDCLNT_SHAREMODE_SHARED){ - period = DefaultPeriod; - if( duration < 3 * period) - duration = 3 * period; - }else{ - if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE){ - if(((WAVEFORMATEXTENSIBLE*)fmt)->dwChannelMask == 0 || - ((WAVEFORMATEXTENSIBLE*)fmt)->dwChannelMask & SPEAKER_RESERVED) - return AUDCLNT_E_UNSUPPORTED_FORMAT; - } - - if(!period) - period = DefaultPeriod; /* not minimum */ - if(period < MinimumPeriod || period > 5000000) - return AUDCLNT_E_INVALID_DEVICE_PERIOD; - if(duration > 20000000) /* the smaller the period, the lower this limit */ - return AUDCLNT_E_BUFFER_SIZE_ERROR; - if(flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK){ - if(duration != period) - return AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; - FIXME("EXCLUSIVE mode with EVENTCALLBACK\n"); - return AUDCLNT_E_DEVICE_IN_USE; - }else{ - if( duration < 8 * period) - duration = 8 * period; /* may grow above 2s */ - } - } - sessions_lock(); - if(This->stream){ - sessions_unlock(); - return AUDCLNT_E_ALREADY_INITIALIZED; - } - params.name = NULL; params.device = This->device_name; params.flow = This->dataflow; @@ -626,7 +589,7 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, params.period = period; params.fmt = fmt; params.channel_count = NULL; - params.stream = &stream; + params.stream = &This->stream; UNIX_CALL(create_stream, ¶ms); if(FAILED(params.result)){ @@ -652,12 +615,13 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, end: if(FAILED(params.result)){ - release_params.stream = stream; + release_params.stream = This->stream; UNIX_CALL(release_stream, &release_params); + This->stream = 0; + HeapFree(GetProcessHeap(), 0, This->vols); This->vols = NULL; }else{ - This->stream = stream; set_stream_volumes(This); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2888
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/wineoss.drv/mmdevdrv.c | 38 ++++--------------------------------- dlls/wineoss.drv/oss.c | 35 +++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/dlls/wineoss.drv/mmdevdrv.c b/dlls/wineoss.drv/mmdevdrv.c index c55176307f0..18e63756616 100644 --- a/dlls/wineoss.drv/mmdevdrv.c +++ b/dlls/wineoss.drv/mmdevdrv.c @@ -50,9 +50,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(oss); #define NULL_PTR_ERR MAKE_HRESULT(SEVERITY_ERROR, FACILITY_WIN32, RPC_X_NULL_REF_POINTER) -static const REFERENCE_TIME DefaultPeriod = 100000; -static const REFERENCE_TIME MinimumPeriod = 50000; - typedef struct _OSSDevice { struct list entry; EDataFlow flow; @@ -553,7 +550,6 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, { ACImpl *This = impl_from_IAudioClient3(iface); struct create_stream_params params; - stream_handle stream; unsigned int i; TRACE("(%p)->(%x, %lx, %s, %s, %p, %s)\n", This, mode, flags, @@ -581,35 +577,8 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return E_INVALIDARG; } - if(mode == AUDCLNT_SHAREMODE_SHARED){ - period = DefaultPeriod; - if( duration < 3 * period) - duration = 3 * period; - }else{ - if(!period) - period = DefaultPeriod; /* not minimum */ - if(period < MinimumPeriod || period > 5000000) - return AUDCLNT_E_INVALID_DEVICE_PERIOD; - if(duration > 20000000) /* the smaller the period, the lower this limit */ - return AUDCLNT_E_BUFFER_SIZE_ERROR; - if(flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK){ - if(duration != period) - return AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; - FIXME("EXCLUSIVE mode with EVENTCALLBACK\n"); - return AUDCLNT_E_DEVICE_IN_USE; - }else{ - if( duration < 8 * period) - duration = 8 * period; /* may grow above 2s */ - } - } - sessions_lock(); - if(This->stream){ - sessions_unlock(); - return AUDCLNT_E_ALREADY_INITIALIZED; - } - params.name = NULL; params.device = This->device_name; params.flow = This->dataflow; @@ -619,7 +588,7 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, params.period = period; params.fmt = fmt; params.channel_count = NULL; - params.stream = &stream; + params.stream = &This->stream; OSS_CALL(create_stream, ¶ms); if(FAILED(params.result)){ @@ -641,12 +610,13 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, exit: if(FAILED(params.result)){ - stream_release(stream, NULL); + stream_release(This->stream, NULL); + This->stream = 0; + HeapFree(GetProcessHeap(), 0, This->vols); This->vols = NULL; } else { list_add_tail(&This->session->clients, &This->entry); - This->stream = stream; set_stream_volumes(This); } diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c index 585df97bc43..e837bc1f0c1 100644 --- a/dlls/wineoss.drv/oss.c +++ b/dlls/wineoss.drv/oss.c @@ -555,11 +555,44 @@ static ULONG_PTR zero_bits(void) static NTSTATUS oss_create_stream(void *args) { struct create_stream_params *params = args; - WAVEFORMATEXTENSIBLE *fmtex; + WAVEFORMATEXTENSIBLE *fmtex = (WAVEFORMATEXTENSIBLE *)params->fmt; struct oss_stream *stream; oss_audioinfo ai; SIZE_T size; + /* We don't return early as priority is given to the other errors. */ + params->result = *params->stream ? AUDCLNT_E_ALREADY_INITIALIZED : S_OK; + + if (params->share == AUDCLNT_SHAREMODE_SHARED) { + params->period = def_period; + if (params->duration < 3 * params->period) + params->duration = 3 * params->period; + } else { + if (fmtex->Format.wFormatTag == WAVE_FORMAT_EXTENSIBLE && + (fmtex->dwChannelMask == 0 || fmtex->dwChannelMask & SPEAKER_RESERVED)) + params->result = AUDCLNT_E_UNSUPPORTED_FORMAT; + else { + if (!params->period) + params->period = def_period; + if (params->period < min_period || params->period > 5000000) + params->result = AUDCLNT_E_INVALID_DEVICE_PERIOD; + else if (params->duration > 20000000) /* The smaller the period, the lower this limit. */ + params->result = AUDCLNT_E_BUFFER_SIZE_ERROR; + else if (params->flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK) { + if (params->duration != params->period) + params->result = AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; + + FIXME("EXCLUSIVE mode with EVENTCALLBACK\n"); + + params->result = AUDCLNT_E_DEVICE_IN_USE; + } else if (params->duration < 8 * params->period) + params->duration = 8 * params->period; /* May grow above 2s. */ + } + } + + if (FAILED(params->result)) + return STATUS_SUCCESS; + stream = calloc(1, sizeof(*stream)); if(!stream){ params->result = E_OUTOFMEMORY; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2888
Davide Beatrici (@davidebeatrici) commented about dlls/wineoss.drv/oss.c:
+ WAVEFORMATEXTENSIBLE *fmtex = (WAVEFORMATEXTENSIBLE *)params->fmt; struct oss_stream *stream; oss_audioinfo ai; SIZE_T size;
+ /* We don't return early as priority is given to the other errors. */ + params->result = *params->stream ? AUDCLNT_E_ALREADY_INITIALIZED : S_OK; + + if (params->share == AUDCLNT_SHAREMODE_SHARED) { + params->period = def_period; + if (params->duration < 3 * params->period) + params->duration = 3 * params->period; + } else { + if (fmtex->Format.wFormatTag == WAVE_FORMAT_EXTENSIBLE && + (fmtex->dwChannelMask == 0 || fmtex->dwChannelMask & SPEAKER_RESERVED)) + params->result = AUDCLNT_E_UNSUPPORTED_FORMAT; Please note that this check was missing in the driver.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2888#note_33629
Huw Davies (@huw) commented about dlls/winealsa.drv/alsa.c:
snd_pcm_sw_params_t *sw_params = NULL; snd_pcm_format_t format; unsigned int rate, alsa_period_us, i; - WAVEFORMATEXTENSIBLE *fmtex; + WAVEFORMATEXTENSIBLE *fmtex = (WAVEFORMATEXTENSIBLE *)params->fmt; int err; SIZE_T size;
+ /* We don't return early as priority is given to the other errors. */ + params->result = *params->stream ? AUDCLNT_E_ALREADY_INITIALIZED : S_OK;
I don't think we want this check in the drivers. The idea is that the assignment of `stream` to `This->stream` on the PE-side happens only when the stream has been created so that other threads can freely use it. If the other errors really need to take precedence then we'll have to create the stream unconditionally on `This->stream` and then destroy it later if `This->stream` is set. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2888#note_33635
On Wed May 24 06:43:41 2023 +0000, Huw Davies wrote:
I don't think we want this check in the drivers. The idea is that the assignment of `stream` to `This->stream` on the PE-side happens only when the stream has been created so that other threads can freely use it. If the other errors really need to take precedence then we'll have to create the stream unconditionally on `This->stream` and then destroy it later if `This->stream` is set. I'm honestly not sure whether the other errors really need to take precedence, we should probably check the native behavior.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2888#note_33655
On Wed May 24 09:11:28 2023 +0000, Davide Beatrici wrote:
I'm honestly not sure whether the other errors really need to take precedence, we should probably check the native behavior. If we have tests that would fail if we change the order then we should honour them, otherwise just change the order: i.e. don't bother adding new tests for this sort of thing - if some app really relies on this, we'll fix that later.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2888#note_33656
On Wed May 24 09:15:18 2023 +0000, Huw Davies wrote:
If we have tests that would fail if we change the order then we should honour them, otherwise just change the order: i.e. don't bother adding new tests for this sort of thing - if some app really relies on this, we'll fix that later. Excellent, sounds good.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2888#note_33657
participants (3)
-
Davide Beatrici -
Davide Beatrici (＠davidebeatrici)
-
Huw Davies (＠huw)