[PATCH v8 0/5] MR2091: windows.media.speech: Initial Unixlib work.
Second commit is to be removed. Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> -- v8: windows.media.speech: Implement Vosk create and release functions in the unixlib. windows.media.speech/tests: Allow the SpeechRecognizer creation to fail in Wine. windows.media.speech/tests: Get rid of duplicated hresult. windows.media.speech: Add unixlib stub. windows.media.speech: Add Vosk checks to autoconf. https://gitlab.winehq.org/wine/wine/-/merge_requests/2091
From: Bernhard Kölbl <besentv(a)gmail.com> Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> --- configure.ac | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 9ff7c5e8914..e6b5947a0eb 100644 --- a/configure.ac +++ b/configure.ac @@ -59,6 +59,7 @@ AC_ARG_WITH(udev, AS_HELP_STRING([--without-udev],[do not use udev (plug an AC_ARG_WITH(unwind, AS_HELP_STRING([--without-unwind],[do not use the libunwind library (exception handling)])) AC_ARG_WITH(usb, AS_HELP_STRING([--without-usb],[do not use the libusb library])) AC_ARG_WITH(v4l2, AS_HELP_STRING([--without-v4l2],[do not use v4l2 (video capture)])) +AC_ARG_WITH(vosk, AS_HELP_STRING([--without-vosk],[do not use Vosk])) AC_ARG_WITH(vulkan, AS_HELP_STRING([--without-vulkan],[do not use Vulkan])) AC_ARG_WITH(xcomposite,AS_HELP_STRING([--without-xcomposite],[do not use the Xcomposite extension]), [if test "x$withval" = "xno"; then ac_cv_header_X11_extensions_Xcomposite_h=no; fi]) @@ -483,7 +484,8 @@ AC_CHECK_HEADERS(\ syscall.h \ utime.h \ valgrind/memcheck.h \ - valgrind/valgrind.h + valgrind/valgrind.h \ + vosk_api.h ) WINE_HEADER_MAJOR() AC_HEADER_STAT() @@ -1787,6 +1789,15 @@ then WINE_WARNING([No sound system was found. Windows applications will be silent.]) fi +dnl **** Check for Vosk **** +if test x$with_vosk != xno +then + WINE_CHECK_SONAME(vosk,vosk_recognizer_new,[AC_SUBST(VOSK_LIBS,"-lvosk") ac_cv_lib_vosk=yes],,) +fi +WINE_NOTICE_WITH(vosk,[test x$with_vosk != xno -a x$ac_cv_lib_vosk != xyes], + [libvosk ${notice_platform}development files not found (or too old), Vosk aka speech recognition won't be supported.], + [enable_vosk]) + dnl *** Check for Vulkan *** if test "x$with_vulkan" != "xno" then -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2091
From: Bernhard Kölbl <besentv(a)gmail.com> windows.media.speech: Add unixlib stub. Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> --- dlls/windows.media.speech/Makefile.in | 2 ++ dlls/windows.media.speech/main.c | 19 +++++++++++++++ dlls/windows.media.speech/private.h | 4 ++++ dlls/windows.media.speech/unixlib.h | 33 +++++++++++++++++++++++++++ 4 files changed, 58 insertions(+) create mode 100644 dlls/windows.media.speech/unixlib.h diff --git a/dlls/windows.media.speech/Makefile.in b/dlls/windows.media.speech/Makefile.in index 10903cb1d7b..c06a142780b 100644 --- a/dlls/windows.media.speech/Makefile.in +++ b/dlls/windows.media.speech/Makefile.in @@ -1,5 +1,7 @@ MODULE = windows.media.speech.dll +UNIXLIB = windows.media.speech.so IMPORTS = combase uuid +UNIX_LIBS = $(VOSK_LIBS) C_SRCS = \ async.c \ diff --git a/dlls/windows.media.speech/main.c b/dlls/windows.media.speech/main.c index e772a791588..a3eb980438a 100644 --- a/dlls/windows.media.speech/main.c +++ b/dlls/windows.media.speech/main.c @@ -20,9 +20,28 @@ #include "initguid.h" #include "private.h" +#include "unixlib.h" + #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(speech); +WINE_DECLARE_DEBUG_CHANNEL(winediag); + +BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, void *reserved) +{ + if (reason == DLL_PROCESS_ATTACH) + { + DisableThreadLibraryCalls(instance); + if (__wine_init_unix_call()) + { + ERR_(winediag)("Wine is unable to load the Unix side dependencies for speech recognition. " + "Make sure Vosk is installed on your system and try again.\n"); + return FALSE; + } + } + + return TRUE; +} HRESULT WINAPI DllGetClassObject(REFCLSID clsid, REFIID riid, void **out) { diff --git a/dlls/windows.media.speech/private.h b/dlls/windows.media.speech/private.h index e80d73ec1fb..2f804fbf1a7 100644 --- a/dlls/windows.media.speech/private.h +++ b/dlls/windows.media.speech/private.h @@ -22,6 +22,10 @@ #include <stdarg.h> +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "winerror.h" +#include "winternl.h" #define COBJMACROS #include "corerror.h" #include "windef.h" diff --git a/dlls/windows.media.speech/unixlib.h b/dlls/windows.media.speech/unixlib.h new file mode 100644 index 00000000000..5516b51d235 --- /dev/null +++ b/dlls/windows.media.speech/unixlib.h @@ -0,0 +1,33 @@ +/* + * Unix library interface for Windows.Media.Speech + * + * Copyright 2023 Bernhard Kölbl for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_WINDOWS_MEDIA_SPEECH_UNIXLIB_H +#define __WINE_WINDOWS_MEDIA_SPEECH_UNIXLIB_H + +#include <stdbool.h> +#include <stdint.h> + +#include "windef.h" +#include "winternl.h" +#include "wtypes.h" + +#include "wine/unixlib.h" + +#endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2091
From: Bernhard Kölbl <besentv(a)gmail.com> Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> --- dlls/windows.media.speech/tests/speech.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c index 6bc5a8b1751..c7c7b6eb040 100644 --- a/dlls/windows.media.speech/tests/speech.c +++ b/dlls/windows.media.speech/tests/speech.c @@ -42,7 +42,6 @@ #define AsyncStatus_Closed 4 #define SPERR_WINRT_INTERNAL_ERROR 0x800455a0 -#define SPERR_WINRT_INCORRECT_FORMAT 0x80131537 #define IHandler_RecognitionResult ITypedEventHandler_SpeechContinuousRecognitionSession_SpeechContinuousRecognitionResultGeneratedEventArgs #define IHandler_RecognitionResultVtbl ITypedEventHandler_SpeechContinuousRecognitionSession_SpeechContinuousRecognitionResultGeneratedEventArgsVtbl @@ -1005,7 +1004,7 @@ static void test_SpeechSynthesizer(void) operation_ss_stream = (void *)0xdeadbeef; hr = ISpeechSynthesizer_SynthesizeSsmlToStreamAsync(synthesizer, str, &operation_ss_stream); /* Broken on Win 8 + 8.1 */ - ok(hr == S_OK || broken(hr == SPERR_WINRT_INCORRECT_FORMAT), "ISpeechSynthesizer_SynthesizeSsmlToStreamAsync failed, hr %#lx\n", hr); + ok(hr == S_OK || broken(hr == COR_E_FORMAT), "ISpeechSynthesizer_SynthesizeSsmlToStreamAsync failed, hr %#lx\n", hr); if (hr == S_OK) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2091
From: Bernhard Kölbl <besentv(a)gmail.com> To allow for error handling of missing Unix-side dependencies. Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> --- dlls/windows.media.speech/tests/speech.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c index c7c7b6eb040..57e216b78d5 100644 --- a/dlls/windows.media.speech/tests/speech.c +++ b/dlls/windows.media.speech/tests/speech.c @@ -1192,7 +1192,7 @@ static void test_SpeechRecognizer(void) ok(ref == 1, "Got unexpected ref %lu.\n", ref); hr = RoActivateInstance(hstr, &inspectable); - ok(hr == S_OK || broken(hr == SPERR_WINRT_INTERNAL_ERROR), "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK || hr == SPERR_WINRT_INTERNAL_ERROR, "Got unexpected hr %#lx.\n", hr); if (hr == S_OK) { @@ -1411,7 +1411,7 @@ skip_operation: } else if (hr == SPERR_WINRT_INTERNAL_ERROR) /* Not sure when this triggers. Probably if a language pack is not installed. */ { - win_skip("Could not init SpeechRecognizer with default language!\n"); + skip("Could not init SpeechRecognizer with default language!\n"); } done: @@ -1651,12 +1651,12 @@ static void test_Recognition(void) ok(hr == S_OK, "WindowsCreateString failed, hr %#lx.\n", hr); hr = RoActivateInstance(hstr, &inspectable); - ok(hr == S_OK || broken(hr == SPERR_WINRT_INTERNAL_ERROR || hr == REGDB_E_CLASSNOTREG), "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK || hr == SPERR_WINRT_INTERNAL_ERROR || broken(hr == REGDB_E_CLASSNOTREG), "Got unexpected hr %#lx.\n", hr); WindowsDeleteString(hstr); - if (FAILED(hr)) /* Win 8 and 8.1 and Win10 without enabled SR. */ + if (FAILED(hr)) /* Win 8 and 8.1 and Win10 without enabled SR. Wine with missing Unix side dependencies. */ { - win_skip("SpeechRecognizer cannot be activated!\n"); + skip("SpeechRecognizer cannot be activated!\n"); goto done; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2091
From: Bernhard Kölbl <besentv(a)gmail.com> Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> --- dlls/windows.media.speech/Makefile.in | 5 +- dlls/windows.media.speech/private.h | 3 + dlls/windows.media.speech/recognizer.c | 42 ++++ dlls/windows.media.speech/unixlib.h | 20 ++ dlls/windows.media.speech/vosk.c | 268 +++++++++++++++++++++++++ 5 files changed, 336 insertions(+), 2 deletions(-) create mode 100644 dlls/windows.media.speech/vosk.c diff --git a/dlls/windows.media.speech/Makefile.in b/dlls/windows.media.speech/Makefile.in index c06a142780b..7a7f9711799 100644 --- a/dlls/windows.media.speech/Makefile.in +++ b/dlls/windows.media.speech/Makefile.in @@ -1,6 +1,6 @@ MODULE = windows.media.speech.dll UNIXLIB = windows.media.speech.so -IMPORTS = combase uuid +IMPORTS = combase uuid user32 UNIX_LIBS = $(VOSK_LIBS) C_SRCS = \ @@ -10,6 +10,7 @@ C_SRCS = \ main.c \ recognizer.c \ synthesizer.c \ - vector.c + vector.c \ + vosk.c IDL_SRCS = classes.idl diff --git a/dlls/windows.media.speech/private.h b/dlls/windows.media.speech/private.h index 2f804fbf1a7..62952478bdf 100644 --- a/dlls/windows.media.speech/private.h +++ b/dlls/windows.media.speech/private.h @@ -31,6 +31,7 @@ #include "windef.h" #include "winbase.h" #include "winstring.h" +#include "winuser.h" #include "objbase.h" #include "activation.h" @@ -47,6 +48,8 @@ #include "wine/list.h" +#define SPERR_WINRT_INTERNAL_ERROR 0x800455a0 + /* * * Windows.Media.SpeechRecognition diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c index c2f386206b8..ff23acc2720 100644 --- a/dlls/windows.media.speech/recognizer.c +++ b/dlls/windows.media.speech/recognizer.c @@ -25,6 +25,9 @@ #include "wine/debug.h" +#include "unixlib.h" +#include "wine/unixlib.h" + WINE_DEFAULT_DEBUG_CHANNEL(speech); /* @@ -171,6 +174,8 @@ struct session IAudioCaptureClient *capture_client; WAVEFORMATEX capture_wfx; + vosk_handle vosk_handle; + HANDLE worker_thread, worker_control_event, audio_buf_event; BOOLEAN worker_running, worker_paused; CRITICAL_SECTION cs; @@ -318,7 +323,9 @@ static ULONG WINAPI session_AddRef( ISpeechContinuousRecognitionSession *iface ) static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface ) { struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface); + struct vosk_release_params vosk_release_params; ULONG ref = InterlockedDecrement(&impl->ref); + TRACE("iface %p, ref %lu.\n", iface, ref); if (!ref) @@ -344,6 +351,9 @@ static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface impl->cs.DebugInfo->Spare[0] = 0; DeleteCriticalSection(&impl->cs); + vosk_release_params.vosk_handle = impl->vosk_handle; + WINE_UNIX_CALL(unix_vosk_release, &vosk_release_params); + IVector_ISpeechRecognitionConstraint_Release(impl->constraints); free(impl); } @@ -1079,6 +1089,35 @@ cleanup: return hr; } +static HRESULT recognizer_factory_create_vosk_instance(struct session *session) +{ + struct vosk_create_params vosk_create_params = { 0 }; + WCHAR locale[LOCALE_NAME_MAX_LENGTH]; + NTSTATUS status; + INT len; + + if (!(len = GetUserDefaultLocaleName(locale, LOCALE_NAME_MAX_LENGTH))) + return E_FAIL; + + if (CharLowerBuffW(locale, len) != len) + return E_FAIL; + + if (!WideCharToMultiByte(CP_ACP, 0, locale, -1, (LPSTR)vosk_create_params.locale, len, NULL, NULL)) + return HRESULT_FROM_WIN32(GetLastError()); + + vosk_create_params.sample_rate = (FLOAT)session->capture_wfx.nSamplesPerSec; + + if ((status = WINE_UNIX_CALL(unix_vosk_create, &vosk_create_params))) + { + ERR("Unable to create Vosk instance for locale %s, status %#lx. Speech recognition won't work.\n", debugstr_a(vosk_create_params.locale), status); + return SPERR_WINRT_INTERNAL_ERROR; + } + + session->vosk_handle = vosk_create_params.vosk_handle; + + return S_OK; +} + static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface, ILanguage *language, ISpeechRecognizer **speechrecognizer ) { struct recognizer *impl; @@ -1125,6 +1164,9 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface if (FAILED(hr = recognizer_factory_create_audio_capture(session))) goto error; + if (FAILED(hr = recognizer_factory_create_vosk_instance(session))) + goto error; + InitializeCriticalSection(&session->cs); session->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": recognition_session.cs"); diff --git a/dlls/windows.media.speech/unixlib.h b/dlls/windows.media.speech/unixlib.h index 5516b51d235..5f45dcc0dc9 100644 --- a/dlls/windows.media.speech/unixlib.h +++ b/dlls/windows.media.speech/unixlib.h @@ -30,4 +30,24 @@ #include "wine/unixlib.h" +typedef UINT64 vosk_handle; + +struct vosk_create_params +{ + vosk_handle vosk_handle; + CHAR locale[LOCALE_NAME_MAX_LENGTH]; + FLOAT sample_rate; +}; + +struct vosk_release_params +{ + vosk_handle vosk_handle; +}; + +enum unix_funcs +{ + unix_vosk_create, + unix_vosk_release, +}; + #endif diff --git a/dlls/windows.media.speech/vosk.c b/dlls/windows.media.speech/vosk.c new file mode 100644 index 00000000000..3782ec2f46c --- /dev/null +++ b/dlls/windows.media.speech/vosk.c @@ -0,0 +1,268 @@ +/* + * Vosk interface for Windows.Media.Speech + * + * Copyright 2023 Bernhard Kölbl for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <wchar.h> + +#include <stdarg.h> +#include <dirent.h> +#include <dlfcn.h> +#include <errno.h> + +#ifdef HAVE_VOSK_API_H +#include <vosk_api.h> +#endif /* HAVE_VOSK_API_H */ + +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "winerror.h" +#include "winternl.h" + +#include "wine/debug.h" + +#include "unixlib.h" + +WINE_DEFAULT_DEBUG_CHANNEL(speech); + +#ifdef SONAME_LIBVOSK + +static inline vosk_handle vosk_recognizer_to_handle( VoskRecognizer *recognizer ) +{ + return (vosk_handle)(UINT_PTR)recognizer; +} + +static inline VoskRecognizer *vosk_recognizer_from_handle( vosk_handle handle ) +{ + return (VoskRecognizer *)(UINT_PTR)handle; +} + +static NTSTATUS errno_to_status( int err ) +{ + TRACE("errno %d.\n", err); + + switch (err) + { + case EAGAIN: return STATUS_SHARING_VIOLATION; + case EBADF: return STATUS_INVALID_HANDLE; + case EBUSY: return STATUS_DEVICE_BUSY; + case ENOSPC: return STATUS_DISK_FULL; + case EPERM: + case EROFS: + case EACCES: return STATUS_ACCESS_DENIED; + case ENOTDIR: return STATUS_OBJECT_PATH_NOT_FOUND; + case ENOENT: return STATUS_OBJECT_NAME_NOT_FOUND; + case EISDIR: return STATUS_INVALID_DEVICE_REQUEST; + case EMFILE: + case ENFILE: return STATUS_TOO_MANY_OPENED_FILES; + case EINVAL: return STATUS_INVALID_PARAMETER; + case ENOTEMPTY: return STATUS_DIRECTORY_NOT_EMPTY; + case EPIPE: return STATUS_PIPE_DISCONNECTED; + case EIO: return STATUS_DEVICE_NOT_READY; +#ifdef ENOMEDIUM + case ENOMEDIUM: return STATUS_NO_MEDIA_IN_DEVICE; +#endif + case ENXIO: return STATUS_NO_SUCH_DEVICE; + case ENOTTY: + case EOPNOTSUPP:return STATUS_NOT_SUPPORTED; + case ECONNRESET:return STATUS_PIPE_DISCONNECTED; + case EFAULT: return STATUS_ACCESS_VIOLATION; + case ESPIPE: return STATUS_ILLEGAL_FUNCTION; + case ELOOP: return STATUS_REPARSE_POINT_NOT_RESOLVED; +#ifdef ETIME /* Missing on FreeBSD */ + case ETIME: return STATUS_IO_TIMEOUT; +#endif + case ENOEXEC: /* ?? */ + case EEXIST: /* ?? */ + default: + FIXME("Converting errno %d to STATUS_UNSUCCESSFUL.\n", err); + return STATUS_UNSUCCESSFUL; + } +} + +static NTSTATUS find_model_by_locale_and_path( const char *path, const char *locale, VoskModel **model ) +{ + static const char *vosk_model_identifier_small = "vosk-model-small-"; + static const char *vosk_model_identifier = "vosk-model-"; + size_t ident_small_len = strlen(vosk_model_identifier_small); + size_t ident_len = strlen(vosk_model_identifier); + NTSTATUS status = STATUS_UNSUCCESSFUL; + char *dir_name, *model_path; + struct dirent *dirent; + size_t len, path_len; + DIR *dir; + + TRACE("path %s, locale %s, model %p.\n", path, debugstr_a(locale), model); + + if (!path || !locale || strlen(locale) < 4) + return STATUS_UNSUCCESSFUL; + + if ((dir = opendir(path)) == NULL) + return errno_to_status(errno); + + path_len = strlen(path); + *model = NULL; + + while ((dirent = readdir(dir))) + { + if (dirent->d_type != DT_DIR) + continue; + + if (!strcmp(dir_name = dirent->d_name, "..")) + continue; + + if (!strncmp(dir_name, vosk_model_identifier_small, ident_small_len)) + dir_name += ident_small_len; + else if (!strncmp(dir_name, vosk_model_identifier, ident_len)) + dir_name += ident_len; + else + continue; + + /* First match for lang and region (en-us), then only lang (en). */ + if (strncmp(dir_name, locale, 5) && strncmp(dir_name, locale, 2)) + continue; + + if(!(model_path = malloc(path_len + 1 /* '/' */ + strlen(dirent->d_name) + 1))) + { + status = STATUS_MEMORY_NOT_ALLOCATED; + break; + } + + sprintf(model_path, "%s/%s", path, dirent->d_name); + + TRACE("Trying to load Vosk model %s.\n", debugstr_a(model_path)); + + *model = vosk_model_new(model_path); + free(model_path); + + if (*model) + { + status = STATUS_SUCCESS; + break; + } + } + + closedir(dir); + + return status; +} + +static NTSTATUS find_model_by_locale( const char *locale, VoskModel **model ) +{ + NTSTATUS status = STATUS_UNSUCCESSFUL; + const char *suffix = NULL; + char *env, *path; + + TRACE("locale %s, model %p.\n", debugstr_a(locale), model); + + if (!model) + return STATUS_UNSUCCESSFUL; + + if (!find_model_by_locale_and_path(getenv("VOSK_MODEL_PATH"), locale, model)) + return STATUS_SUCCESS; + if (!find_model_by_locale_and_path("/usr/share/vosk", locale, model)) + return STATUS_SUCCESS; + + if ((env = getenv("XDG_CACHE_HOME"))) + suffix = "/vosk"; + else if ((env = getenv("HOME"))) + suffix = "/.cache/vosk"; + + if (suffix && (path = malloc(strlen(env) + strlen(suffix) + 1))) + { + sprintf(path, "%s%s", env, suffix); + status = find_model_by_locale_and_path(path, locale, model); + free(path); + } + + return status; +} + +static NTSTATUS vosk_create( void *args ) +{ + struct vosk_create_params *params = args; + VoskRecognizer *recognizer = NULL; + VoskModel *model = NULL; + NTSTATUS status; + + TRACE("args %p.\n", args); + + if ((status = find_model_by_locale(params->locale, &model))) + return status; + + if (!(recognizer = vosk_recognizer_new(model, params->sample_rate))) + goto error; + + /* The model is kept alive inside the recognizer, so we can safely free our ref here. */ + vosk_model_free(model); + + params->vosk_handle = vosk_recognizer_to_handle(recognizer); + return STATUS_SUCCESS; + +error: + if (model) vosk_model_free(model); + return STATUS_UNSUCCESSFUL; +} + +static NTSTATUS vosk_release( void *args ) +{ + struct vosk_release_params *params = args; + + TRACE("args %p.\n", args); + + vosk_recognizer_free(vosk_recognizer_from_handle(params->vosk_handle)); + + return STATUS_SUCCESS; +} + +#else /* SONAME_LIBVOSK */ + +#define MAKE_UNSUPPORTED_FUNC( f ) \ + static NTSTATUS f( void *args ) \ + { \ + WARN("wine was compiled without Vosk support. Speech recognition won't work.\n"); \ + return STATUS_NOT_SUPPORTED; \ + } + +MAKE_UNSUPPORTED_FUNC(vosk_create) +MAKE_UNSUPPORTED_FUNC(vosk_release) +#undef MAKE_UNSUPPORTED_FUNC + +#endif /* SONAME_LIBVOSK */ + +unixlib_entry_t __wine_unix_call_funcs[] = +{ + vosk_create, + vosk_release, +}; + +unixlib_entry_t __wine_unix_call_wow64_funcs[] = +{ + vosk_create, + vosk_release, +}; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2091
If you're linking directly you don't need checks for the soname. You also need to check for the header presence. libpcap or CL is probably a good model here. (Although it would be nice if vosk supported pkg-config...) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23941
On Mon Feb 13 20:03:35 2023 +0000, Zebediah Figura wrote:
If you're linking directly you don't need checks for the soname. You also need to check for the header presence. libpcap or CL is probably a good model here. (Although it would be nice if vosk supported pkg-config...) So just test with `AC_CHECK_LIB`? Also, is the issue from your other comment properly addressed?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23942
So just test with `AC_CHECK_LIB`?
Yes, if you don't have pkg-config then you'll need to use AC_CHECK_LIB.
Also, is the issue from your other comment properly addressed?
I don't know which comment you're referring to exactly, but you don't need the header and soname checks in the C code either. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23945
I don't know which comment you're referring to exactly, https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23188
but you don't need the header and soname checks in the C code either. Uhm, the soname makes sense, but how do I not need the header checks?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23964
Some more comments from patch 5/5:
+static inline vosk_handle vosk_recognizer_to_handle( VoskRecognizer *recognizer ) +{ + return (vosk_handle)(UINT_PTR)recognizer; +} + +static inline VoskRecognizer *vosk_recognizer_from_handle( vosk_handle handle ) +{ + return (VoskRecognizer *)(UINT_PTR)handle; +}
These don't need to be inline, do they?
+static NTSTATUS vosk_create( void *args )
vosk uses vosk_ as a namespace prefix, so it seems that for readability it would be better to avoid anything beginning with that name in our code. That includes other cases in this file, e.g. "vosk_handle", "vosk_model_identifier".
+ if (!path || !locale || strlen(locale) < 4) + return STATUS_UNSUCCESSFUL;
Do you need these checks?
+ if ((dir = opendir(path)) == NULL) + return errno_to_status(errno);
Is there some legitimate scenario where this can fail? (And if not, shouldn't we print an ERR?) Is it worth pulling in errno_to_status() for this?
+ if (!strcmp(dir_name = dirent->d_name, "..")) + continue; + + if (!strncmp(dir_name, vosk_model_identifier_small, ident_small_len)) + dir_name += ident_small_len; + else if (!strncmp(dir_name, vosk_model_identifier, ident_len)) + dir_name += ident_len; + else + continue;
Isn't the ".." check redundant here? Also, looking at this function, this looks like a very odd way to search for models. What's expected to follow the language and region, and why don't we care about it? Or, if the answer is "nothing", why are we doing an entire readdir() loop instead of just trying the 4 different possibilities? Does Vosk really expect every API consumer to do this all manually? This looks extremely suspicious.
+ if(!(model_path = malloc(path_len + 1 /* '/' */ + strlen(dirent->d_name) + 1))) + { + status = STATUS_MEMORY_NOT_ALLOCATED; + break; + }
You want STATUS_NO_MEMORY. STATUS_MEMORY_NOT_ALLOCATED is, from a brief search, supposed to be returned when trying to free memory that was not allocated.
+static NTSTATUS find_model_by_locale( const char *locale, VoskModel **model ) +{ + NTSTATUS status = STATUS_UNSUCCESSFUL; + const char *suffix = NULL; + char *env, *path; + + TRACE("locale %s, model %p.\n", debugstr_a(locale), model); + + if (!model) + return STATUS_UNSUCCESSFUL;
Why do you need this check?
+ if ((env = getenv("XDG_CACHE_HOME"))) + suffix = "/vosk"; + else if ((env = getenv("HOME"))) + suffix = "/.cache/vosk";
It seems mildly clearer to add an "else" case here rather than checking for a previous initialized variable below.
+ if (suffix && (path = malloc(strlen(env) + strlen(suffix) + 1)))
And probably better to return STATUS_NO_MEMORY if malloc() fails.
+ if (!(recognizer = vosk_recognizer_new(model, params->sample_rate))) + goto error; + + /* The model is kept alive inside the recognizer, so we can safely free our ref here. */ + vosk_model_free(model); + + params->vosk_handle = vosk_recognizer_to_handle(recognizer); + return STATUS_SUCCESS; + +error: + if (model) vosk_model_free(model); + return STATUS_UNSUCCESSFUL; +}
This "error" label is only used in one place and only has one line of cleanup, making it look a little redundant. I also believe that "model" cannot be NULL in this case, can it? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23983
I don't know which comment you're referring to exactly,
https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23188
Sorry, now that I investigate more closely that comment was just wrong; WINE_NOTICE_WITH already checks that variable.
but you don't need the header and soname checks in the C code either.
Uhm, the soname makes sense, but how do I not need the header checks?
If the header isn't found, then you won't (can't) look for the library, and if the library isn't found, then the DLL won't be compiled. Well, at least, that's the intent, but you've written "enable_vosk" as the fourth argument to WINE_NOTICE_WITH. You want enable_windows_media_speech. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23984
These don't need to be inline, do they?
Why not.
vosk uses vosk\_ as a namespace prefix, so it seems that for readability it would be better to avoid anything beginning with that name in our code. That includes other cases in this file, e.g. "vosk_handle", "vosk_model_identifier".
ok
Do you need these checks?
Yes, yes, maybe.
Is there some legitimate scenario where this can fail?
Yes, if the model path isn't created.
(And if not, shouldn't we print an ERR?) Is it worth pulling in errno_to_status() for this?
Sure, I can print an ERR. I pulled in that function for possible future uses.
Isn't the ".." check redundant here?
Not sure what you mean? It's a folder that's always listed so we skip it?
Does Vosk really expect every API consumer to do this all manually? This looks extremely suspicious.
Sort of, yes. They have a Python wrapper for this, but obviously I don't wanna interface with Python scripts.
Also, looking at this function, this looks like a very odd way to search for models. What's expected to follow the language and region, and why don't we care about it? Or, if the answer is "nothing", why are we doing an entire readdir() loop instead of just trying the 4 different possibilities?
There is something after the lang and region, but we indeed don't care about it because that's exactly how the Python script does it as well.
It seems mildly clearer to add an "else" case here rather than checking for a previous initialized variable below.
Sorry, I don't understand this one.
This "error" label is only used in one place and only has one line of cleanup, making it look a little redundant. I also believe that "model" cannot be NULL in this case, can it?
Yes, I can remove it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23988
If the header isn't found, then you won't (can't) look for the library, and if the library isn't found, then the DLL won't be compiled.
What if the header is present but not an arch specific lib?
You want enable_windows_media_speech.
I want to disable the whole DLL? that sounds odd. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23989
On Mon Feb 13 22:02:11 2023 +0000, Bernhard Kölbl wrote:
These don't need to be inline, do they? Why not. vosk uses vosk\_ as a namespace prefix, so it seems that for readability it would be better to avoid anything beginning with that name in our code. That includes other cases in this file, e.g. "vosk_handle", "vosk_model_identifier". ok Do you need these checks? Yes, yes, maybe. Is there some legitimate scenario where this can fail? Yes, if the model path isn't created. (And if not, shouldn't we print an ERR?) Is it worth pulling in errno_to_status() for this? Sure, I can print an ERR. I pulled in that function for possible future uses. Isn't the ".." check redundant here? Not sure what you mean? It's a folder that's always listed so we skip it? Does Vosk really expect every API consumer to do this all manually? This looks extremely suspicious. Sort of, yes. They have a Python wrapper for this, but obviously I don't wanna interface with Python scripts. Also, looking at this function, this looks like a very odd way to search for models. What's expected to follow the language and region, and why don't we care about it? Or, if the answer is "nothing", why are we doing an entire readdir() loop instead of just trying the 4 different possibilities? There is something after the lang and region, but we indeed don't care about it because that's exactly how the Python script does it as well. It seems mildly clearer to add an "else" case here rather than checking for a previous initialized variable below. Sorry, I don't understand this one. This "error" label is only used in one place and only has one line of cleanup, making it look a little redundant. I also believe that "model" cannot be NULL in this case, can it? Yes, I can remove it. Please don't strip out the code I'm commenting on, that makes this reply hard to read.
These don't need to be inline, do they?
Why not.
Why do they need to be inline?
Do you need these checks?
Yes, yes, maybe.
Why do you need those checks?
Is there some legitimate scenario where this can fail?
Yes, if the model path isn't created.
What do you mean by this?
Isn't the ".." check redundant here?
Not sure what you mean? It's a folder that's always listed so we skip it?
It's already covered by the following prefix checks, isn't it?
Does Vosk really expect every API consumer to do this all manually? This looks extremely suspicious.
Sort of, yes. They have a Python wrapper for this, but obviously I don't wanna interface with Python scripts.
So they export multiple language bindings, except they require Python in order to do certain basic library tasks like actually finding the models?? And this is all part of stable API?? Can we maybe get a function into their C API to do this for us? This just feels very poorly designed, and not the kind of thing that Wine should have to do.
It seems mildly clearer to add an "else" case here rather than checking for a previous initialized variable below.
Sorry, I don't understand this one.
I mean something like: if ((env = getenv("XDG_CACHE_HOME"))) suffix = "/vosk"; else if ((env = getenv("HOME"))) suffix = "/.cache/vosk"; else return STATUS_UNSUCCESSFUL; (possibly with some ERR in there, because undefined $HOME seems like a catastrophic failure that the user should know about, but anyway) That way you don't need to initialize the "suffix" variable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23991
If the header isn't found, then you won't (can't) look for the library, and if the library isn't found, then the DLL won't be compiled.
What if the header is present but not an arch specific lib?
Then AC_CHECK_LIB fails.
You want enable_windows_media_speech.
I want to disable the whole DLL? that sounds odd.
Well, we usually do that on the grounds that the library isn't useful otherwise. From a completely uneducated standpoint, I can only assume that's the case here. If not, I suppose you'd want to model it after qcap and v4l2. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23992
On Mon Feb 13 22:27:23 2023 +0000, Zebediah Figura wrote:
If the header isn't found, then you won't (can't) look for the library, and if the library isn't found, then the DLL won't be compiled.
What if the header is present but not an arch specific lib? Then AC_CHECK_LIB fails.
You want enable_windows_media_speech.
I want to disable the whole DLL? that sounds odd. Well, we usually do that on the grounds that the library isn't useful otherwise. From a completely uneducated standpoint, I can only assume that's the case here. If not, I suppose you'd want to model it after qcap and v4l2. Yeah, we definitely don't want to disable the whole dll in that case, because it's still needed for speech synthesis.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23994
On Mon Feb 13 22:21:48 2023 +0000, Zebediah Figura wrote:
Please don't strip out the code I'm commenting on, that makes this reply hard to read.
These don't need to be inline, do they?
Why not. Why do they need to be inline?
Do you need these checks?
Yes, yes, maybe. Why do you need those checks?
Is there some legitimate scenario where this can fail?
Yes, if the model path isn't created. What do you mean by this?
Isn't the ".." check redundant here?
Not sure what you mean? It's a folder that's always listed so we skip it? It's already covered by the following prefix checks, isn't it?
Does Vosk really expect every API consumer to do this all manually? This looks extremely suspicious.
Sort of, yes. They have a Python wrapper for this, but obviously I don't wanna interface with Python scripts. So they export multiple language bindings, except they require Python in order to do certain basic library tasks like actually finding the models?? And this is all part of stable API?? Can we maybe get a function into their C API to do this for us? This just feels very poorly designed, and not the kind of thing that Wine should have to do.
It seems mildly clearer to add an "else" case here rather than checking for a previous initialized variable below.
Sorry, I don't understand this one. I mean something like: if ((env = getenv("XDG_CACHE_HOME"))) suffix = "/vosk"; else if ((env = getenv("HOME"))) suffix = "/.cache/vosk"; else return STATUS_UNSUCCESSFUL; (possibly with some ERR in there, because undefined $HOME seems like a catastrophic failure that the user should know about, but anyway) That way you don't need to initialize the "suffix" variable. It'd be useful if you used the comment function on the Gitlab website, it's really inconvenient to answer like this.
We need a bunch of other workarounds for Vosk later as well, it's a small tradeoff and the unixlib won't be really big anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_23995
It'd be useful if you used the comment function on the Gitlab website, it's really inconvenient to answer like this.
I am using the Gitlab comment function.
We need a bunch of other workarounds for Vosk later as well, it's a small tradeoff and the unixlib won't be really big anyway.
I don't see how "we will need more workarounds" justifies a workaround? Ideally we wouldn't have any workarounds, and given that this is a library so new it's not packaged anywhere yet, it seems like there's no reason not to fix the library rather than Wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_24000
On Mon Feb 13 23:07:42 2023 +0000, Zebediah Figura wrote:
It'd be useful if you used the comment function on the Gitlab website, it's really inconvenient to answer like this. I am using the Gitlab comment function. We need a bunch of other workarounds for Vosk later as well, it's a small tradeoff and the unixlib won't be really big anyway. I don't see how "we will need more workarounds" justifies a workaround? Ideally we wouldn't have any workarounds, and given that this is a library so new it's not packaged anywhere yet, it seems like there's no reason not to fix the library rather than Wine. We need to work around the whole lib using Json as their ABI, meaning it is inevitable to have any.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_24146
On Tue Feb 14 22:03:56 2023 +0000, Bernhard Kölbl wrote:
We need to work around the whole lib using Json as their ABI, meaning it is inevitable to have any. Sorry, what I was trying to assert is that less workarounds is still better than more.
And if using JSON is painful, the same thing applies: surely we can add more friendly C APIs to the library? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2091#note_24147
participants (2)
-
Bernhard Kölbl -
Zebediah Figura (＠zfigura)