[PATCH v4 0/2] MR5810: Cap default period in mmdevapi instead of the audio driver.
Trace output with PulseAudio: ``` 0144:trace:mmdevapi:adjust_timing Requested duration 1000000 and period 0 0144:trace:mmdevapi:adjust_timing Device periods: 26666 default and 26666 minimum 0144:trace:mmdevapi:adjust_timing Adjusted duration 1000000 and period 100000 ``` -- v4: winepulse: Don't set a floor for the period(s). mmdevapi: Set the default period to a minimum of 10 ms. https://gitlab.winehq.org/wine/wine/-/merge_requests/5810
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/mmdevapi/client.c | 55 ++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index 0b849e47e3d..ed635bcf98a 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -99,25 +99,36 @@ static inline struct audio_client *impl_from_IAudioStreamVolume(IAudioStreamVolu return CONTAINING_RECORD(iface, struct audio_client, IAudioStreamVolume_iface); } -static HRESULT adjust_timing(struct audio_client *This, +static HRESULT get_periods(struct audio_client *client, + REFERENCE_TIME *def_period, REFERENCE_TIME *min_period) +{ + static const REFERENCE_TIME min_def_period = 100000; /* 10 ms */ + struct get_device_period_params params; + + params.device = client->device_name; + params.flow = client->dataflow; + params.def_period = def_period; + params.min_period = min_period; + + wine_unix_call(get_device_period, ¶ms); + + if (def_period) *def_period = max(*def_period, min_def_period); + + return params.result; +} + +static HRESULT adjust_timing(struct audio_client *client, REFERENCE_TIME *duration, REFERENCE_TIME *period, const AUDCLNT_SHAREMODE mode, const DWORD flags, const WAVEFORMATEX *fmt) { - struct get_device_period_params params; REFERENCE_TIME def_period, min_period; + HRESULT hr; TRACE("Requested duration %lu and period %lu\n", (ULONG)*duration, (ULONG)*period); - params.device = This->device_name; - params.flow = This->dataflow; - params.def_period = &def_period; - params.min_period = &min_period; - - wine_unix_call(get_device_period, ¶ms); - - if (FAILED(params.result)) - return params.result; + if (FAILED(hr = get_periods(client, &def_period, &min_period))) + return hr; TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)def_period, (ULONG)min_period); @@ -129,21 +140,21 @@ static HRESULT adjust_timing(struct audio_client *This, const WAVEFORMATEXTENSIBLE *fmtex = (WAVEFORMATEXTENSIBLE *)fmt; if (fmtex->Format.wFormatTag == WAVE_FORMAT_EXTENSIBLE && (fmtex->dwChannelMask == 0 || fmtex->dwChannelMask & SPEAKER_RESERVED)) - params.result = AUDCLNT_E_UNSUPPORTED_FORMAT; + hr = AUDCLNT_E_UNSUPPORTED_FORMAT; else { if (*period == 0) *period = def_period; if (*period < min_period || *period > 5000000) - params.result = AUDCLNT_E_INVALID_DEVICE_PERIOD; + hr = AUDCLNT_E_INVALID_DEVICE_PERIOD; else if (*duration > 20000000) /* The smaller the period, the lower this limit. */ - params.result = AUDCLNT_E_BUFFER_SIZE_ERROR; + hr = AUDCLNT_E_BUFFER_SIZE_ERROR; else if (flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK) { if (*duration != *period) - params.result = AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; + hr = AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL; FIXME("EXCLUSIVE mode with EVENTCALLBACK\n"); - params.result = AUDCLNT_E_DEVICE_IN_USE; + hr = AUDCLNT_E_DEVICE_IN_USE; } else if (*duration < 8 * *period) *duration = 8 * *period; /* May grow above 2s. */ } @@ -151,7 +162,7 @@ static HRESULT adjust_timing(struct audio_client *This, TRACE("Adjusted duration %lu and period %lu\n", (ULONG)*duration, (ULONG)*period); - return params.result; + return hr; } static void dump_fmt(const WAVEFORMATEX *fmt) @@ -748,21 +759,13 @@ static HRESULT WINAPI client_GetDevicePeriod(IAudioClient3 *iface, REFERENCE_TIM REFERENCE_TIME *minperiod) { struct audio_client *This = impl_from_IAudioClient3(iface); - struct get_device_period_params params; TRACE("(%p)->(%p, %p)\n", This, defperiod, minperiod); if (!defperiod && !minperiod) return E_POINTER; - params.device = This->device_name; - params.flow = This->dataflow; - params.def_period = defperiod; - params.min_period = minperiod; - - wine_unix_call(get_device_period, ¶ms); - - return params.result; + return get_periods(This, defperiod, minperiod); } static HRESULT WINAPI client_Start(IAudioClient3 *iface) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5810
From: Davide Beatrici <git(a)davidebeatrici.dev> This driver, unlike the others, queries the engine for the actual device period. The default period floor is not needed anymore because it now lives in mmdevapi. The minimum period floor never mattered because exclusive mode is not supported. --- dlls/winepulse.drv/pulse.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index dd8d0b4441d..068245f03b2 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -113,9 +113,6 @@ static pa_mainloop *pulse_ml; static struct list g_phys_speakers = LIST_INIT(g_phys_speakers); static struct list g_phys_sources = LIST_INIT(g_phys_sources); -static const REFERENCE_TIME MinimumPeriod = 30000; -static const REFERENCE_TIME DefaultPeriod = 100000; - static pthread_mutex_t pulse_mutex; static pthread_cond_t pulse_cond = PTHREAD_COND_INITIALIZER; @@ -750,12 +747,6 @@ static void pulse_probe_settings(int render, const char *pulse_name, WAVEFORMATE if (length) *def_period = *min_period = pa_bytes_to_usec(10 * length, &ss); - if (*min_period < MinimumPeriod) - *min_period = MinimumPeriod; - - if (*def_period < DefaultPeriod) - *def_period = DefaultPeriod; - wfx->wFormatTag = WAVE_FORMAT_EXTENSIBLE; wfx->cbSize = sizeof(WAVEFORMATEXTENSIBLE) - sizeof(WAVEFORMATEX); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5810
I've pushed a few fixups. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73352
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810
On Sat Jun 15 16:24:53 2024 +0000, Huw Davies wrote:
I've pushed a few fixups. Thank you!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73356
Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/client.c:
+static HRESULT get_periods(struct audio_client *client, REFERENCE_TIME *def_period, REFERENCE_TIME *min_period) { static const REFERENCE_TIME min_def_period = 100000; /* 10 ms */ struct get_device_period_params params;
- params.device = This->device_name; - params.flow = This->dataflow; + params.device = client->device_name; + params.flow = client->dataflow; params.def_period = def_period; params.min_period = min_period;
wine_unix_call(get_device_period, ¶ms);
- TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)*def_period, (ULONG)*min_period); For reference, the reason why I moved this debug message here is to know the default period reported by the backend in case of problems (e.g. glitches, xruns).
However, I believe it's better to write one in the driver's code instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73357
On Sat Jun 15 16:30:30 2024 +0000, Davide Beatrici wrote:
For reference, the reason why I moved this debug message here is to know the default period reported by the backend in case of problems (e.g. glitches, xruns). However, I believe it's better to write one in the driver's code instead. `def_period` or `min_period` can be `NULL` when this is called from `IAudioClient_GetDefaultPeriod()` and I didn't want to add the conditionals to deal with this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73361
participants (3)
-
Davide Beatrici -
Davide Beatrici (＠davidebeatrici)
-
Huw Davies (＠huw)