[PATCH v2 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 ``` -- v2: winepulse: Don't cap period(s). mmdevapi: Cap default period to 10 ms. https://gitlab.winehq.org/wine/wine/-/merge_requests/5810
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/mmdevapi/client.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index 0b849e47e3d..a760256540a 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -47,6 +47,8 @@ extern HRESULT get_audio_session(const GUID *sessionguid, IMMDevice *device, UIN struct audio_session **out); extern struct audio_session_wrapper *session_wrapper_create(struct audio_client *client); +static const REFERENCE_TIME min_def_period = 100000; + static HANDLE main_loop_thread; void main_loop_stop(void) @@ -122,7 +124,7 @@ static HRESULT adjust_timing(struct audio_client *This, TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)def_period, (ULONG)min_period); if (mode == AUDCLNT_SHAREMODE_SHARED) { - *period = def_period; + *period = max(def_period, min_def_period); if (*duration < 3 * *period) *duration = 3 * *period; } else { @@ -132,7 +134,7 @@ static HRESULT adjust_timing(struct audio_client *This, params.result = AUDCLNT_E_UNSUPPORTED_FORMAT; else { if (*period == 0) - *period = def_period; + *period = max(def_period, min_def_period); if (*period < min_period || *period > 5000000) params.result = AUDCLNT_E_INVALID_DEVICE_PERIOD; else if (*duration > 20000000) /* The smaller the period, the lower this limit. */ @@ -762,6 +764,9 @@ static HRESULT WINAPI client_GetDevicePeriod(IAudioClient3 *iface, REFERENCE_TIM wine_unix_call(get_device_period, ¶ms); + if (defperiod && *defperiod < min_def_period) + *defperiod = min_def_period; + return params.result; } -- 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 cap is not needed anymore because it now lives in mmdevapi. The minimum period cap 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
Do we not need to do this in the exclusive case?
Good catch, we do as there is no way to communicate to the application that we chose a period other than 10 ms (which is why `IAudioClient3` exists in the first place).
Also, 'cap' to me implies a maximum value. I'd try `min_def_period` and change the commit msg.
Agreed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73229
Do we not need to do this in the exclusive case?
Good catch, we do as there is no way to communicate to the application that we chose a period other than 10 ms [].
That being the case, I think it would make sense to add a helper function to do the unix call and fix-up the default period. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73230
Good catch, we do as there is no way to communicate to the application that we chose a period other than 10 ms (which is why `IAudioClient3` exists in the first place).
At least for older interfaces across sound APIs there were a few games that firmly assume 10ms (and overflow buffers with larger) periods even if there is a way to communicate a different period. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73231
On Fri Jun 14 18:55:45 2024 +0000, Paul Gofman wrote:
Good catch, we do as there is no way to communicate to the application that we chose a period other than 10 ms (which is why `IAudioClient3` exists in the first place). At least for older interfaces across sound APIs there were a few games that firmly assume 10ms (and overflow buffers with larger) periods even if there is a way to communicate a different period. On that note, I was thinking: should we perhaps force 10 ms even if the UNIX driver reports a higher period?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73235
On Fri Jun 14 19:03:55 2024 +0000, Davide Beatrici wrote:
On that note, I was thinking: should we perhaps force 10 ms even if the UNIX driver reports a higher period? We should, yeah, it should depend strictly on what Windows has as buffer length and not what backend hints. Back then it was fixed this way in xaudio / faudio. I am not sure if mmdevapi ever queried that from backends but back then it had voluntary buffer size and that was also causing problems (1d66a108b901ebfb11b134e9a6895b1d87333638).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5810#note_73238
participants (4)
-
Davide Beatrici -
Davide Beatrici (＠davidebeatrici)
-
Huw Davies (＠huw)
-
Paul Gofman (＠gofman)