[PATCH v10 0/8] MR729: windows.media.speech: Initial recognition session work.

[Bernhard Kölbl]

--
  v10: windows.media.speech: Allow the recognition session worker to be paused.
       windows.media.speech: Add a worker thread to the recognition session.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 1/8] windows.media.speech: Move constraints vector to the recognition session.

[Bernhard Kölbl]

From: Bernhard Kölbl <besentv(a)gmail.com>

Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com>
---
 dlls/windows.media.speech/recognizer.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c
index bdcc57f883e..105ed2b0b05 100644
--- a/dlls/windows.media.speech/recognizer.c
+++ b/dlls/windows.media.speech/recognizer.c
@@ -156,6 +156,8 @@ struct session
     ISpeechContinuousRecognitionSession ISpeechContinuousRecognitionSession_iface;
     LONG ref;
 
+    IVector_ISpeechRecognitionConstraint *constraints;
+
     struct list completed_handlers;
     struct list result_handlers;
 };
@@ -208,6 +210,7 @@ static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface
     {
         typed_event_handlers_clear(&impl->completed_handlers);
         typed_event_handlers_clear(&impl->result_handlers);
+        IVector_ISpeechRecognitionConstraint_Release(impl->constraints);
         free(impl);
     }
 
@@ -360,7 +363,6 @@ struct recognizer
     LONG ref;
 
     ISpeechContinuousRecognitionSession *session;
-    IVector_ISpeechRecognitionConstraint *constraints;
 };
 
 /*
@@ -424,7 +426,6 @@ static ULONG WINAPI recognizer_Release( ISpeechRecognizer *iface )
     if (!ref)
     {
         ISpeechContinuousRecognitionSession_Release(impl->session);
-        IVector_ISpeechRecognitionConstraint_Release(impl->constraints);
         free(impl);
     }
 
@@ -452,8 +453,11 @@ static HRESULT WINAPI recognizer_GetTrustLevel( ISpeechRecognizer *iface, TrustL
 static HRESULT WINAPI recognizer_get_Constraints( ISpeechRecognizer *iface, IVector_ISpeechRecognitionConstraint **vector )
 {
     struct recognizer *impl = impl_from_ISpeechRecognizer(iface);
+    struct session *session = impl_from_ISpeechContinuousRecognitionSession(impl->session);
+
     TRACE("iface %p, operation %p.\n", iface, vector);
-    IVector_ISpeechRecognitionConstraint_AddRef((*vector = impl->constraints));
+
+    IVector_ISpeechRecognitionConstraint_AddRef((*vector = session->constraints));
     return S_OK;
 }
 
@@ -797,18 +801,22 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface
     if (language)
         FIXME("language parameter unused. Stub!\n");
 
+    /* Init ISpeechContinuousRecognitionSession */
     session->ISpeechContinuousRecognitionSession_iface.lpVtbl = &session_vtbl;
     session->ref = 1;
+
     list_init(&session->completed_handlers);
     list_init(&session->result_handlers);
 
+    if (FAILED(hr = vector_inspectable_create(&constraints_iids, (IVector_IInspectable**)&session->constraints)))
+        goto error;
+
+    /* Init ISpeechRecognizer */
     impl->ISpeechRecognizer_iface.lpVtbl = &speech_recognizer_vtbl;
     impl->IClosable_iface.lpVtbl = &closable_vtbl;
     impl->ISpeechRecognizer2_iface.lpVtbl = &speech_recognizer2_vtbl;
     impl->session = &session->ISpeechContinuousRecognitionSession_iface;
     impl->ref = 1;
-    if (FAILED(hr = vector_inspectable_create(&constraints_iids, (IVector_IInspectable**)&impl->constraints)))
-        goto error;
 
     TRACE("created SpeechRecognizer %p.\n", impl);
 
@@ -816,6 +824,7 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface
     return S_OK;
 
 error:
+    if (session->constraints) IVector_ISpeechRecognitionConstraint_Release(session->constraints);
     free(session);
     free(impl);
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 2/8] windows.media.speech: Do not force calling convention on internal callbacks.

[Bernhard Kölbl]

From: Bernhard Kölbl <besentv(a)gmail.com>

I also used the oppurtunity to more consistently name the callbacks.

Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com>
---
 dlls/windows.media.speech/private.h     | 4 ++--
 dlls/windows.media.speech/recognizer.c  | 8 ++++----
 dlls/windows.media.speech/synthesizer.c | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/dlls/windows.media.speech/private.h b/dlls/windows.media.speech/private.h
index 13964329697..41f7b02e3de 100644
--- a/dlls/windows.media.speech/private.h
+++ b/dlls/windows.media.speech/private.h
@@ -69,8 +69,8 @@ struct vector_iids
     const GUID *view;
 };
 
-typedef HRESULT (WINAPI *async_action_callback)( IInspectable *invoker );
-typedef HRESULT (WINAPI *async_operation_inspectable_callback)( IInspectable *invoker, IInspectable **result );
+typedef HRESULT (*async_action_callback)( IInspectable *invoker );
+typedef HRESULT (*async_operation_inspectable_callback)( IInspectable *invoker, IInspectable **result );
 
 HRESULT async_action_create( IInspectable *invoker, async_action_callback callback, IAsyncAction **out );
 HRESULT async_operation_inspectable_create( const GUID *iid, IInspectable *invoker, async_operation_inspectable_callback callback,
diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c
index 105ed2b0b05..aaf9c5f8908 100644
--- a/dlls/windows.media.speech/recognizer.c
+++ b/dlls/windows.media.speech/recognizer.c
@@ -247,7 +247,7 @@ static HRESULT WINAPI session_set_AutoStopSilenceTimeout( ISpeechContinuousRecog
     return E_NOTIMPL;
 }
 
-static HRESULT WINAPI start_callback( IInspectable *invoker )
+static HRESULT session_start_async( IInspectable *invoker )
 {
     return S_OK;
 }
@@ -255,7 +255,7 @@ static HRESULT WINAPI start_callback( IInspectable *invoker )
 static HRESULT WINAPI session_StartAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
 {
     FIXME("iface %p, action %p stub!\n", iface, action);
-    return async_action_create(NULL, start_callback, action);
+    return async_action_create(NULL, session_start_async, action);
 }
 
 static HRESULT WINAPI session_StartWithModeAsync( ISpeechContinuousRecognitionSession *iface,
@@ -479,7 +479,7 @@ static HRESULT WINAPI recognizer_get_UIOptions( ISpeechRecognizer *iface, ISpeec
     return E_NOTIMPL;
 }
 
-static HRESULT WINAPI compile_callback( IInspectable *invoker, IInspectable **result )
+static HRESULT recognizer_compile_constraints_async( IInspectable *invoker, IInspectable **result )
 {
     return compilation_result_create(SpeechRecognitionResultStatus_Success, (ISpeechRecognitionCompilationResult **) result);
 }
@@ -489,7 +489,7 @@ static HRESULT WINAPI recognizer_CompileConstraintsAsync( ISpeechRecognizer *ifa
 {
     IAsyncOperation_IInspectable **value = (IAsyncOperation_IInspectable **)operation;
     FIXME("iface %p, operation %p semi-stub!\n", iface, operation);
-    return async_operation_inspectable_create(&IID_IAsyncOperation_SpeechRecognitionCompilationResult, NULL, compile_callback, value);
+    return async_operation_inspectable_create(&IID_IAsyncOperation_SpeechRecognitionCompilationResult, NULL, recognizer_compile_constraints_async, value);
 }
 
 static HRESULT WINAPI recognizer_RecognizeAsync( ISpeechRecognizer *iface,
diff --git a/dlls/windows.media.speech/synthesizer.c b/dlls/windows.media.speech/synthesizer.c
index ce257c7c355..39d14b84ab7 100644
--- a/dlls/windows.media.speech/synthesizer.c
+++ b/dlls/windows.media.speech/synthesizer.c
@@ -375,7 +375,7 @@ static HRESULT WINAPI synthesizer_GetTrustLevel( ISpeechSynthesizer *iface, Trus
     return E_NOTIMPL;
 }
 
-static HRESULT CALLBACK text_to_stream_operation( IInspectable *invoker, IInspectable **result )
+static HRESULT synthesizer_synthesize_text_to_stream_async( IInspectable *invoker, IInspectable **result )
 {
     return synthesis_stream_create((ISpeechSynthesisStream **)result);
 }
@@ -385,10 +385,10 @@ static HRESULT WINAPI synthesizer_SynthesizeTextToStreamAsync( ISpeechSynthesize
 {
     TRACE("iface %p, text %p, operation %p.\n", iface, text, operation);
     return async_operation_inspectable_create(&IID_IAsyncOperation_SpeechSynthesisStream, NULL,
-                                              text_to_stream_operation, (IAsyncOperation_IInspectable **)operation);
+                                              synthesizer_synthesize_text_to_stream_async, (IAsyncOperation_IInspectable **)operation);
 }
 
-static HRESULT CALLBACK ssml_to_stream_operation( IInspectable *invoker, IInspectable **result )
+static HRESULT synthesizer_synthesize_ssml_to_stream_async( IInspectable *invoker, IInspectable **result )
 {
     return synthesis_stream_create((ISpeechSynthesisStream **)result);
 }
@@ -398,7 +398,7 @@ static HRESULT WINAPI synthesizer_SynthesizeSsmlToStreamAsync( ISpeechSynthesize
 {
     TRACE("iface %p, ssml %p, operation %p.\n", iface, ssml, operation);
     return async_operation_inspectable_create(&IID_IAsyncOperation_SpeechSynthesisStream, NULL,
-                                              ssml_to_stream_operation, (IAsyncOperation_IInspectable **)operation);
+                                              synthesizer_synthesize_ssml_to_stream_async, (IAsyncOperation_IInspectable **)operation);
 }
 
 static HRESULT WINAPI synthesizer_put_Voice( ISpeechSynthesizer *iface, IVoiceInformation *value )
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 3/8] windows.media.speech: Return IAsyncAction from session_StopAsync.

[Bernhard Kölbl]

From: Bernhard Kölbl <besentv(a)gmail.com>

Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com>
---
 dlls/windows.media.speech/recognizer.c   |  7 ++++++-
 dlls/windows.media.speech/tests/speech.c | 24 ++++++++++--------------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c
index aaf9c5f8908..b4f68489bd0 100644
--- a/dlls/windows.media.speech/recognizer.c
+++ b/dlls/windows.media.speech/recognizer.c
@@ -266,10 +266,15 @@ static HRESULT WINAPI session_StartWithModeAsync( ISpeechContinuousRecognitionSe
     return E_NOTIMPL;
 }
 
+static HRESULT session_stop_async( IInspectable *invoker )
+{
+    return S_OK;
+}
+
 static HRESULT WINAPI session_StopAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
 {
     FIXME("iface %p, action %p stub!\n", iface, action);
-    return E_NOTIMPL;
+    return async_action_create(NULL, session_stop_async, action);
 }
 
 static HRESULT WINAPI session_CancelAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c
index 445d10923ae..b6355743a83 100644
--- a/dlls/windows.media.speech/tests/speech.c
+++ b/dlls/windows.media.speech/tests/speech.c
@@ -1762,9 +1762,7 @@ static void test_Recognition(void)
      */
 
     hr = ISpeechContinuousRecognitionSession_StopAsync(session, &action2);
-    todo_wine ok(hr == S_OK, "ISpeechContinuousRecognitionSession_StopAsync failed, hr %#lx.\n", hr);
-
-    if (FAILED(hr)) goto skip_action;
+    ok(hr == S_OK, "ISpeechContinuousRecognitionSession_StopAsync failed, hr %#lx.\n", hr);
 
     async_void_handler_create_static(&action_handler);
     action_handler.event_block = CreateEventW(NULL, FALSE, FALSE, NULL);
@@ -1776,40 +1774,38 @@ static void test_Recognition(void)
     put_param.handler = &action_handler.IAsyncActionCompletedHandler_iface;
     put_param.action = action2;
     put_thread = CreateThread(NULL, 0, action_put_completed_thread, &put_param, 0, NULL);
-    todo_wine ok(!WaitForSingleObject(action_handler.event_finished , 5000), "Wait for event_finished failed.\n");
+    ok(!WaitForSingleObject(action_handler.event_finished , 5000), "Wait for event_finished failed.\n");
 
     handler = (void *)0xdeadbeef;
     old_ref = action_handler.ref;
     hr = IAsyncAction_get_Completed(action2, &handler);
-    todo_wine ok(hr == S_OK, "IAsyncAction_get_Completed failed, hr %#lx.\n", hr);
+    ok(hr == S_OK, "IAsyncAction_get_Completed failed, hr %#lx.\n", hr);
 
-    todo_wine ok(handler == &action_handler.IAsyncActionCompletedHandler_iface || /* Broken on 1507. */
-                            broken(handler != NULL && handler != (void *)0xdeadbeef), "Handler was %p.\n", handler);
+    todo_wine ok(handler == &action_handler.IAsyncActionCompletedHandler_iface, "Handler was %p.\n", handler);
 
     ref = action_handler.ref - old_ref;
     todo_wine ok(ref == 1, "The ref was increased by %lu.\n", ref);
-    IAsyncActionCompletedHandler_Release(handler);
+    if (handler) IAsyncActionCompletedHandler_Release(handler);
 
     hr = IAsyncAction_QueryInterface(action2, &IID_IAsyncInfo, (void **)&info);
-    todo_wine ok(hr == S_OK, "IAsyncAction_QueryInterface failed, hr %#lx.\n", hr);
+    ok(hr == S_OK, "IAsyncAction_QueryInterface failed, hr %#lx.\n", hr);
 
     hr = IAsyncInfo_Close(info); /* If IAsyncInfo_Close would wait for the handler to finish, the test would get stuck here. */
-    todo_wine ok(hr == S_OK, "IAsyncInfo_Close failed, hr %#lx.\n", hr);
+    ok(hr == S_OK, "IAsyncInfo_Close failed, hr %#lx.\n", hr);
     check_async_info((IInspectable *)action2, 3, AsyncStatus_Closed, S_OK);
 
     set = SetEvent(action_handler.event_block);
-    todo_wine ok(set == TRUE, "Event 'event_block' wasn't set.\n");
-    todo_wine ok(!WaitForSingleObject(put_thread , 1000), "Wait for put_thread failed.\n");
+    ok(set == TRUE, "Event 'event_block' wasn't set.\n");
+    ok(!WaitForSingleObject(put_thread , 1000), "Wait for put_thread failed.\n");
     IAsyncInfo_Release(info);
 
     CloseHandle(action_handler.event_finished);
     CloseHandle(action_handler.event_block);
     CloseHandle(put_thread);
 
-    todo_wine ok(action != action2, "actions were the same!\n");
+    ok(action != action2, "actions were the same!\n");
 
     IAsyncAction_Release(action2);
-skip_action:
     IAsyncAction_Release(action);
 
     hr = ISpeechContinuousRecognitionSession_remove_ResultGenerated(session, token);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 4/8] windows.media.speech: Return IAsyncAction from session_PauseAsync.

[Bernhard Kölbl]

From: Bernhard Kölbl <besentv(a)gmail.com>

Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com>
---
 dlls/windows.media.speech/recognizer.c   |  7 ++++++-
 dlls/windows.media.speech/tests/speech.c | 11 ++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c
index b4f68489bd0..54a0e165f5f 100644
--- a/dlls/windows.media.speech/recognizer.c
+++ b/dlls/windows.media.speech/recognizer.c
@@ -283,10 +283,15 @@ static HRESULT WINAPI session_CancelAsync( ISpeechContinuousRecognitionSession *
     return E_NOTIMPL;
 }
 
+static HRESULT session_pause_async( IInspectable *invoker )
+{
+    return S_OK;
+}
+
 static HRESULT WINAPI session_PauseAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
 {
     FIXME("iface %p, action %p stub!\n", iface, action);
-    return E_NOTIMPL;
+    return async_action_create(NULL, session_pause_async, action);
 }
 
 static HRESULT WINAPI session_Resume( ISpeechContinuousRecognitionSession *iface )
diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c
index b6355743a83..8b14221f487 100644
--- a/dlls/windows.media.speech/tests/speech.c
+++ b/dlls/windows.media.speech/tests/speech.c
@@ -1761,6 +1761,15 @@ static void test_Recognition(void)
      * TODO: Use a loopback device together with prerecorded audio files to test the recognizer's functionality.
      */
 
+    hr = ISpeechContinuousRecognitionSession_PauseAsync(session, &action2);
+    ok(hr == S_OK, "ISpeechContinuousRecognitionSession_PauseAsync failed, hr %#lx.\n", hr);
+    await_async_void(action2, &action_handler);
+    check_async_info((IInspectable *)action2, 3, Completed, S_OK);
+    IAsyncAction_Release(action2);
+
+    hr = ISpeechContinuousRecognitionSession_Resume(session);
+    todo_wine ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
+
     hr = ISpeechContinuousRecognitionSession_StopAsync(session, &action2);
     ok(hr == S_OK, "ISpeechContinuousRecognitionSession_StopAsync failed, hr %#lx.\n", hr);
 
@@ -1792,7 +1801,7 @@ static void test_Recognition(void)
 
     hr = IAsyncInfo_Close(info); /* If IAsyncInfo_Close would wait for the handler to finish, the test would get stuck here. */
     ok(hr == S_OK, "IAsyncInfo_Close failed, hr %#lx.\n", hr);
-    check_async_info((IInspectable *)action2, 3, AsyncStatus_Closed, S_OK);
+    check_async_info((IInspectable *)action2, 4, AsyncStatus_Closed, S_OK);
 
     set = SetEvent(action_handler.event_block);
     ok(set == TRUE, "Event 'event_block' wasn't set.\n");
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 5/8] windows.media.speech/tests: Test the recognizer state.

[Bernhard Kölbl]

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 | 27 ++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c
index 8b14221f487..155382fa76d 100644
--- a/dlls/windows.media.speech/tests/speech.c
+++ b/dlls/windows.media.speech/tests/speech.c
@@ -1619,6 +1619,7 @@ static void test_Recognition(void)
     struct iterator_hstring iterator_hstring;
     struct iterable_hstring iterable_hstring;
     EventRegistrationToken token = { .value = 0 };
+    SpeechRecognizerState recog_state;
     HSTRING commands[3], hstr, tag;
     HANDLE put_thread;
     LONG ref, old_ref;
@@ -1717,6 +1718,11 @@ static void test_Recognition(void)
     ok(hr == S_OK, "ISpeechContinuousRecognitionSession_add_ResultGenerated failed, hr %#lx.\n", hr);
     ok(token.value != 0xdeadbeef, "Got unexpexted token: %#I64x.\n", token.value);
 
+    recog_state = 0xdeadbeef;
+    hr = ISpeechRecognizer2_get_State(recognizer2, &recog_state);
+    todo_wine ok(hr == S_OK, "ISpeechRecognizer2_get_State failed, hr %#lx.\n", hr);
+    todo_wine ok(recog_state == SpeechRecognizerState_Idle, "recog_state was %u.\n", recog_state);
+
     hr = ISpeechRecognizer_CompileConstraintsAsync(recognizer, &operation);
     ok(hr == S_OK, "ISpeechRecognizer_CompileConstraintsAsync failed, hr %#lx.\n", hr);
     await_async_inspectable((IAsyncOperation_IInspectable *)operation,
@@ -1757,6 +1763,11 @@ static void test_Recognition(void)
 
     IAsyncInfo_Release(info);
 
+    recog_state = 0xdeadbeef;
+    hr = ISpeechRecognizer2_get_State(recognizer2, &recog_state);
+    todo_wine ok(hr == S_OK, "ISpeechRecognizer2_get_State failed, hr %#lx.\n", hr);
+    todo_wine ok(recog_state == SpeechRecognizerState_Capturing, "recog_state was %u.\n", recog_state);
+
     /*
      * TODO: Use a loopback device together with prerecorded audio files to test the recognizer's functionality.
      */
@@ -1767,9 +1778,20 @@ static void test_Recognition(void)
     check_async_info((IInspectable *)action2, 3, Completed, S_OK);
     IAsyncAction_Release(action2);
 
+    recog_state = 0xdeadbeef;
+    hr = ISpeechRecognizer2_get_State(recognizer2, &recog_state);
+    todo_wine ok(hr == S_OK, "ISpeechRecognizer2_get_State failed, hr %#lx.\n", hr);
+    todo_wine ok(recog_state == SpeechRecognizerState_Paused || /* Broken on Win10 1507 */
+                  broken(recog_state == SpeechRecognizerState_Capturing) , "recog_state was %u.\n", recog_state);
+
     hr = ISpeechContinuousRecognitionSession_Resume(session);
     todo_wine ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
 
+    recog_state = 0xdeadbeef;
+    hr = ISpeechRecognizer2_get_State(recognizer2, &recog_state);
+    todo_wine ok(hr == S_OK, "ISpeechRecognizer2_get_State failed, hr %#lx.\n", hr);
+    todo_wine ok(recog_state == SpeechRecognizerState_Capturing, "recog_state was %u.\n", recog_state);
+
     hr = ISpeechContinuousRecognitionSession_StopAsync(session, &action2);
     ok(hr == S_OK, "ISpeechContinuousRecognitionSession_StopAsync failed, hr %#lx.\n", hr);
 
@@ -1817,6 +1839,11 @@ static void test_Recognition(void)
     IAsyncAction_Release(action2);
     IAsyncAction_Release(action);
 
+    recog_state = 0xdeadbeef;
+    hr = ISpeechRecognizer2_get_State(recognizer2, &recog_state);
+    todo_wine ok(hr == S_OK, "ISpeechRecognizer2_get_State failed, hr %#lx.\n", hr);
+    todo_wine ok(recog_state == SpeechRecognizerState_Idle, "recog_state was %u.\n", recog_state);
+
     hr = ISpeechContinuousRecognitionSession_remove_ResultGenerated(session, token);
     ok(hr == S_OK, "ISpeechContinuousRecognitionSession_remove_ResultGenerated failed, hr %#lx.\n", hr);
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 6/8] windows.media.speech/tests: Test starting, stopping, pausing and resuming the recognition session.

[Bernhard Kölbl]

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 | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c
index 155382fa76d..a8ed8cff1e7 100644
--- a/dlls/windows.media.speech/tests/speech.c
+++ b/dlls/windows.media.speech/tests/speech.c
@@ -18,6 +18,7 @@
 #define COBJMACROS
 #include <stdarg.h>
 
+#include "corerror.h"
 #include "windef.h"
 #include "winbase.h"
 #include "winerror.h"
@@ -1741,6 +1742,11 @@ static void test_Recognition(void)
 
     await_async_void(action, &action_handler);
 
+    action2 = (void *)0xdeadbeef;
+    hr = ISpeechContinuousRecognitionSession_StartAsync(session, &action2);
+    todo_wine ok(hr == COR_E_INVALIDOPERATION, "ISpeechContinuousRecognitionSession_StartAsync failed, hr %#lx.\n", hr);
+    todo_wine ok(action2 == NULL, "action2 was %p.\n", action2);
+
     hr = IAsyncAction_QueryInterface(action, &IID_IAsyncInfo, (void **)&info);
     ok(hr == S_OK, "IAsyncAction_QueryInterface failed, hr %#lx.\n", hr);
     check_async_info((IInspectable *)action, 1, Completed, S_OK);
@@ -1784,6 +1790,17 @@ static void test_Recognition(void)
     todo_wine ok(recog_state == SpeechRecognizerState_Paused || /* Broken on Win10 1507 */
                   broken(recog_state == SpeechRecognizerState_Capturing) , "recog_state was %u.\n", recog_state);
 
+    /* Check what happens if we try to pause again, when the session is already paused. */
+    hr = ISpeechContinuousRecognitionSession_PauseAsync(session, &action2);
+    ok(hr == S_OK, "ISpeechContinuousRecognitionSession_PauseAsync failed, hr %#lx.\n", hr);
+    await_async_void(action2, &action_handler);
+    check_async_info((IInspectable *)action2, 4, Completed, S_OK);
+    IAsyncAction_Release(action2);
+
+    hr = ISpeechContinuousRecognitionSession_Resume(session);
+    todo_wine ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
+
+    /* Resume when already resumed. */
     hr = ISpeechContinuousRecognitionSession_Resume(session);
     todo_wine ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
 
@@ -1823,7 +1840,7 @@ static void test_Recognition(void)
 
     hr = IAsyncInfo_Close(info); /* If IAsyncInfo_Close would wait for the handler to finish, the test would get stuck here. */
     ok(hr == S_OK, "IAsyncInfo_Close failed, hr %#lx.\n", hr);
-    check_async_info((IInspectable *)action2, 4, AsyncStatus_Closed, S_OK);
+    check_async_info((IInspectable *)action2, 5, AsyncStatus_Closed, S_OK);
 
     set = SetEvent(action_handler.event_block);
     ok(set == TRUE, "Event 'event_block' wasn't set.\n");
@@ -1844,6 +1861,11 @@ static void test_Recognition(void)
     todo_wine ok(hr == S_OK, "ISpeechRecognizer2_get_State failed, hr %#lx.\n", hr);
     todo_wine ok(recog_state == SpeechRecognizerState_Idle, "recog_state was %u.\n", recog_state);
 
+    /* Try stopping, when already stopped. */
+    hr = ISpeechContinuousRecognitionSession_StopAsync(session, &action);
+    todo_wine ok(hr == COR_E_INVALIDOPERATION, "ISpeechContinuousRecognitionSession_StopAsync failed, hr %#lx.\n", hr);
+    todo_wine ok(action == NULL, "action was %p.\n", action);
+
     hr = ISpeechContinuousRecognitionSession_remove_ResultGenerated(session, token);
     ok(hr == S_OK, "ISpeechContinuousRecognitionSession_remove_ResultGenerated failed, hr %#lx.\n", hr);
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 7/8] windows.media.speech: Add a worker thread to the recognition session.

[Bernhard Kölbl]

From: Bernhard Kölbl <besentv(a)gmail.com>

Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com>
---
 dlls/windows.media.speech/private.h      |   1 +
 dlls/windows.media.speech/recognizer.c   | 116 +++++++++++++++++++++--
 dlls/windows.media.speech/tests/speech.c |   8 +-
 3 files changed, 115 insertions(+), 10 deletions(-)

diff --git a/dlls/windows.media.speech/private.h b/dlls/windows.media.speech/private.h
index 41f7b02e3de..e80d73ec1fb 100644
--- a/dlls/windows.media.speech/private.h
+++ b/dlls/windows.media.speech/private.h
@@ -23,6 +23,7 @@
 #include <stdarg.h>
 
 #define COBJMACROS
+#include "corerror.h"
 #include "windef.h"
 #include "winbase.h"
 #include "winstring.h"
diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c
index 54a0e165f5f..3e3a0db3082 100644
--- a/dlls/windows.media.speech/recognizer.c
+++ b/dlls/windows.media.speech/recognizer.c
@@ -160,6 +160,11 @@ struct session
 
     struct list completed_handlers;
     struct list result_handlers;
+
+    HANDLE worker_thread;
+    BOOLEAN worker_running;
+    CONDITION_VARIABLE cv;
+    CRITICAL_SECTION cs;
 };
 
 /*
@@ -173,6 +178,23 @@ static inline struct session *impl_from_ISpeechContinuousRecognitionSession( ISp
     return CONTAINING_RECORD(iface, struct session, ISpeechContinuousRecognitionSession_iface);
 }
 
+static DWORD CALLBACK session_worker_thread_cb( void *args )
+{
+    ISpeechContinuousRecognitionSession *iface = args;
+    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
+
+    EnterCriticalSection(&impl->cs);
+    while (impl->worker_running)
+    {
+        /* TODO: Send mic data to recognizer and handle results. */
+
+        SleepConditionVariableCS(&impl->cv, &impl->cs, 500); /* Wait 500ms to slow down spinning for now. */
+    }
+    LeaveCriticalSection(&impl->cs);
+
+    return 0;
+}
+
 static HRESULT WINAPI session_QueryInterface( ISpeechContinuousRecognitionSession *iface, REFIID iid, void **out )
 {
     struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
@@ -200,6 +222,24 @@ static ULONG WINAPI session_AddRef( ISpeechContinuousRecognitionSession *iface )
     return ref;
 }
 
+static HRESULT session_join_worker_thread( ISpeechContinuousRecognitionSession *iface )
+{
+    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
+
+    WakeConditionVariable(&impl->cv);
+
+    if (impl->worker_thread)
+    {
+        if (WaitForSingleObject(impl->worker_thread, 1000))
+            ERR("Wait for joining the session worker timed out!\n");
+
+        CloseHandle(impl->worker_thread);
+        impl->worker_thread = NULL;
+    }
+
+    return S_OK;
+}
+
 static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface )
 {
     struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
@@ -208,8 +248,18 @@ static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface
 
     if (!ref)
     {
+        EnterCriticalSection(&impl->cs);
+        impl->worker_running = FALSE;
+        LeaveCriticalSection(&impl->cs);
+
+        session_join_worker_thread(iface);
+
         typed_event_handlers_clear(&impl->completed_handlers);
         typed_event_handlers_clear(&impl->result_handlers);
+
+        impl->cs.DebugInfo->Spare[0] = 0;
+        DeleteCriticalSection(&impl->cs);
+
         IVector_ISpeechRecognitionConstraint_Release(impl->constraints);
         free(impl);
     }
@@ -249,13 +299,43 @@ static HRESULT WINAPI session_set_AutoStopSilenceTimeout( ISpeechContinuousRecog
 
 static HRESULT session_start_async( IInspectable *invoker )
 {
-    return S_OK;
+    struct session *impl = impl_from_ISpeechContinuousRecognitionSession((ISpeechContinuousRecognitionSession *)invoker);
+    HRESULT hr = S_OK;
+
+    EnterCriticalSection(&impl->cs);
+    if (!(impl->worker_thread = CreateThread(NULL, 0, session_worker_thread_cb, impl, 0, NULL)))
+    {
+        hr = HRESULT_FROM_WIN32(GetLastError());
+        impl->worker_running = FALSE;
+    }
+    LeaveCriticalSection(&impl->cs);
+
+    return hr;
 }
 
 static HRESULT WINAPI session_StartAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
 {
-    FIXME("iface %p, action %p stub!\n", iface, action);
-    return async_action_create(NULL, session_start_async, action);
+    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
+    BOOLEAN running;
+    HRESULT hr;
+
+    TRACE("iface %p, action %p.\n", iface, action);
+
+    *action = NULL;
+
+    EnterCriticalSection(&impl->cs);
+    if (!(running = impl->worker_running))
+    {
+        impl->worker_running = TRUE;
+    }
+    LeaveCriticalSection(&impl->cs);
+
+    if (running)
+        hr = COR_E_INVALIDOPERATION;
+    else
+        hr = async_action_create((IInspectable *)iface, session_start_async, action);
+
+    return hr;
 }
 
 static HRESULT WINAPI session_StartWithModeAsync( ISpeechContinuousRecognitionSession *iface,
@@ -268,13 +348,32 @@ static HRESULT WINAPI session_StartWithModeAsync( ISpeechContinuousRecognitionSe
 
 static HRESULT session_stop_async( IInspectable *invoker )
 {
-    return S_OK;
+    return session_join_worker_thread((ISpeechContinuousRecognitionSession *)invoker);
 }
 
 static HRESULT WINAPI session_StopAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
 {
-    FIXME("iface %p, action %p stub!\n", iface, action);
-    return async_action_create(NULL, session_stop_async, action);
+    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
+    BOOLEAN running;
+    HRESULT hr;
+
+    TRACE("iface %p, action %p.\n", iface, action);
+
+    *action = NULL;
+
+    EnterCriticalSection(&impl->cs);
+    if ((running = impl->worker_running))
+    {
+        impl->worker_running = FALSE;
+    }
+    LeaveCriticalSection(&impl->cs);
+
+    if (!running)
+        hr = COR_E_INVALIDOPERATION;
+    else
+        hr = async_action_create((IInspectable *)iface, session_stop_async, action);
+
+    return hr;
 }
 
 static HRESULT WINAPI session_CancelAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
@@ -818,6 +917,10 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface
     list_init(&session->completed_handlers);
     list_init(&session->result_handlers);
 
+    InitializeConditionVariable(&session->cv);
+    InitializeCriticalSection(&session->cs);
+    session->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": recognition_session.cs");
+
     if (FAILED(hr = vector_inspectable_create(&constraints_iids, (IVector_IInspectable**)&session->constraints)))
         goto error;
 
@@ -835,6 +938,7 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface
 
 error:
     if (session->constraints) IVector_ISpeechRecognitionConstraint_Release(session->constraints);
+    DeleteCriticalSection(&session->cs);
     free(session);
     free(impl);
 
diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c
index a8ed8cff1e7..8b31031d3c5 100644
--- a/dlls/windows.media.speech/tests/speech.c
+++ b/dlls/windows.media.speech/tests/speech.c
@@ -1744,8 +1744,8 @@ static void test_Recognition(void)
 
     action2 = (void *)0xdeadbeef;
     hr = ISpeechContinuousRecognitionSession_StartAsync(session, &action2);
-    todo_wine ok(hr == COR_E_INVALIDOPERATION, "ISpeechContinuousRecognitionSession_StartAsync failed, hr %#lx.\n", hr);
-    todo_wine ok(action2 == NULL, "action2 was %p.\n", action2);
+    ok(hr == COR_E_INVALIDOPERATION, "ISpeechContinuousRecognitionSession_StartAsync failed, hr %#lx.\n", hr);
+    ok(action2 == NULL, "action2 was %p.\n", action2);
 
     hr = IAsyncAction_QueryInterface(action, &IID_IAsyncInfo, (void **)&info);
     ok(hr == S_OK, "IAsyncAction_QueryInterface failed, hr %#lx.\n", hr);
@@ -1863,8 +1863,8 @@ static void test_Recognition(void)
 
     /* Try stopping, when already stopped. */
     hr = ISpeechContinuousRecognitionSession_StopAsync(session, &action);
-    todo_wine ok(hr == COR_E_INVALIDOPERATION, "ISpeechContinuousRecognitionSession_StopAsync failed, hr %#lx.\n", hr);
-    todo_wine ok(action == NULL, "action was %p.\n", action);
+    ok(hr == COR_E_INVALIDOPERATION, "ISpeechContinuousRecognitionSession_StopAsync failed, hr %#lx.\n", hr);
+    ok(action == NULL, "action was %p.\n", action);
 
     hr = ISpeechContinuousRecognitionSession_remove_ResultGenerated(session, token);
     ok(hr == S_OK, "ISpeechContinuousRecognitionSession_remove_ResultGenerated failed, hr %#lx.\n", hr);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 8/8] windows.media.speech: Allow the recognition session worker to be paused.

[Bernhard Kölbl]

From: Bernhard Kölbl <besentv(a)gmail.com>

Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com>
---
 dlls/windows.media.speech/recognizer.c   | 35 ++++++++++++++++++++----
 dlls/windows.media.speech/tests/speech.c |  4 +--
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c
index 3e3a0db3082..5fabdfe3f06 100644
--- a/dlls/windows.media.speech/recognizer.c
+++ b/dlls/windows.media.speech/recognizer.c
@@ -162,7 +162,7 @@ struct session
     struct list result_handlers;
 
     HANDLE worker_thread;
-    BOOLEAN worker_running;
+    BOOLEAN worker_running, worker_paused;
     CONDITION_VARIABLE cv;
     CRITICAL_SECTION cs;
 };
@@ -188,6 +188,16 @@ static DWORD CALLBACK session_worker_thread_cb( void *args )
     {
         /* TODO: Send mic data to recognizer and handle results. */
 
+        if (impl->worker_paused)
+        {
+            TRACE("Session worker paused.\n");
+
+            SleepConditionVariableCS(&impl->cv, &impl->cs, INFINITE);
+
+            TRACE("Session worker resumed.\n");
+            continue;
+        }
+
         SleepConditionVariableCS(&impl->cv, &impl->cs, 500); /* Wait 500ms to slow down spinning for now. */
     }
     LeaveCriticalSection(&impl->cs);
@@ -384,19 +394,34 @@ static HRESULT WINAPI session_CancelAsync( ISpeechContinuousRecognitionSession *
 
 static HRESULT session_pause_async( IInspectable *invoker )
 {
+    struct session *impl = impl_from_ISpeechContinuousRecognitionSession((ISpeechContinuousRecognitionSession *)invoker);
+
+    EnterCriticalSection(&impl->cs);
+    if (impl->worker_running) impl->worker_paused = TRUE;
+    LeaveCriticalSection(&impl->cs);
+
     return S_OK;
 }
 
 static HRESULT WINAPI session_PauseAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action )
 {
-    FIXME("iface %p, action %p stub!\n", iface, action);
-    return async_action_create(NULL, session_pause_async, action);
+    TRACE("iface %p, action %p.\n", iface, action);
+    return async_action_create((IInspectable *)iface, session_pause_async, action);
 }
 
 static HRESULT WINAPI session_Resume( ISpeechContinuousRecognitionSession *iface )
 {
-    FIXME("iface %p stub!\n", iface);
-    return E_NOTIMPL;
+    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
+
+    TRACE("iface %p.\n", iface);
+
+    EnterCriticalSection(&impl->cs);
+    if (impl->worker_running) impl->worker_paused = FALSE;
+    LeaveCriticalSection(&impl->cs);
+
+    WakeConditionVariable(&impl->cv);
+
+    return S_OK;
 }
 
 static HRESULT WINAPI session_add_Completed( ISpeechContinuousRecognitionSession *iface,
diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c
index 8b31031d3c5..196d0a98302 100644
--- a/dlls/windows.media.speech/tests/speech.c
+++ b/dlls/windows.media.speech/tests/speech.c
@@ -1798,11 +1798,11 @@ static void test_Recognition(void)
     IAsyncAction_Release(action2);
 
     hr = ISpeechContinuousRecognitionSession_Resume(session);
-    todo_wine ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
+    ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
 
     /* Resume when already resumed. */
     hr = ISpeechContinuousRecognitionSession_Resume(session);
-    todo_wine ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
+    ok(hr == S_OK, "ISpeechContinuousRecognitionSession_Resume failed, hr %#lx.\n", hr);
 
     recog_state = 0xdeadbeef;
     hr = ISpeechRecognizer2_get_State(recognizer2, &recog_state);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/729

[PATCH v10 8/8] windows.media.speech: Allow the recognition session worker to be paused.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=128161

Your paranoid android.


=== w1064_adm (64 bit report) ===

windows.media.speech:
speech.c:1775: Test failed: recog_state was 0.

[Bernhard Kölbl]

Should be fine now, there was still a bad bug in the joining code.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20307

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
> static HRESULT session_pause_async( IInspectable *invoker )
>  {
> +    struct session *impl = impl_from_ISpeechContinuousRecognitionSession((ISpeechContinuousRecognitionSession *)invoker);
> +
> +    EnterCriticalSection(&impl->cs);
> +    if (impl->worker_running) impl->worker_paused = TRUE;
> +    LeaveCriticalSection(&impl->cs);

You don't *have* to, here, because the thread will eventually check it, but I think it's maybe better to notify the condition variable every time a flag that can be waited on is modified.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20411

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
> -    return async_action_create(NULL, session_stop_async, action);
> +    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
> +    BOOLEAN running;
> +    HRESULT hr;
> +
> +    TRACE("iface %p, action %p.\n", iface, action);
> +
> +    *action = NULL;
> +
> +    EnterCriticalSection(&impl->cs);
> +    if ((running = impl->worker_running))
> +    {
> +        impl->worker_running = FALSE;
> +    }
> +    LeaveCriticalSection(&impl->cs);
> +
> You'll probably need to signal the condition variable here, for instance to wake up the if the thread if it was paused.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20412

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
> +
> +    TRACE("iface %p, action %p.\n", iface, action);
> +
> +    *action = NULL;
> +
> +    EnterCriticalSection(&impl->cs);
> +    if (!(running = impl->worker_running))
> +    {
> +        impl->worker_running = TRUE;
> +    }
> +    LeaveCriticalSection(&impl->cs);
> +
> +    if (running)
> +        hr = COR_E_INVALIDOPERATION;
> +    else
> +        hr = async_action_create((IInspectable *)iface, session_start_async, action);
> I'm not completely sure how this should be done, but you have a race condition here. For instance, if:

1) StartAsync is called, setting worker_running to TRUE,

2) StopAsync is called, setting worker_running to FALSE,

3) session_stop_async is executed, not doing anything as the thread isn't started,

4) session_start_async is executed, starting the thread, setting the thread handle.

The thread will exit quickly as running is false, but you'll end up with an inconsistent state.

I think it's perhaps better to do everything in the async callbacks, serialized with the critical section. Or otherwise in the method itself, and have the callbacks do nothing (not sure if asynchronicity really matters here).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20413

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
> if (!ref)
>      {
> +        EnterCriticalSection(&impl->cs);
> +        impl->worker_running = FALSE;
> +        LeaveCriticalSection(&impl->cs);

There too you'll probably need to notify the CV.

Notifying it every time one of the flags is changed is probably better to avoid problems like that, even if it's not always necessary.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20414

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
> return ref;
>  }
>
> +static HRESULT session_join_worker_thread( ISpeechContinuousRecognitionSession *iface )
> +{
> +    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
> +
> +    WakeConditionVariable(&impl->cv);

So I missed that in the previous comments and it means things are probably fine then, but I'd find it more obvious if the wake up is done right after the flags are changed, rather than doing it here without any apparent reason (it's not because you wake it up that the thread will exit).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20415

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
> return CONTAINING_RECORD(iface, struct session, ISpeechContinuousRecognitionSession_iface);
>  }
>
> +static DWORD CALLBACK session_worker_thread_cb( void *args )
> +{
> +    ISpeechContinuousRecognitionSession *iface = args;
> +    struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
> +
> +    EnterCriticalSection(&impl->cs);
> +    while (impl->worker_running)
> +    {
> +        /* TODO: Send mic data to recognizer and handle results. */
> +
> +        SleepConditionVariableCS(&impl->cv, &impl->cs, 500); /* Wait 500ms to slow down spinning for now. */

I think there's an even better way to write this, with a single SleepCV and infinite timeout. And later when the thread will have something to do, you would simply skip it unless it's also paused.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20416

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
> error:
>      if (session->constraints) IVector_ISpeechRecognitionConstraint_Release(session->constraints);
> +    DeleteCriticalSection(&session->cs);

You need to clear Spare[0] here too probably, or initialize the CS after any possible error.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20417

[Rémi Bernon]

In https://gitlab.winehq.org/wine/wine/-/merge_requests/729/diffs?commit_id=455... you could probably strip the commit comment instead.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20418

[Rémi Bernon]

On Fri Jan  6 11:22:05 2023 +0000, Rémi Bernon wrote:
> I think there's an even better way to write this, with a single SleepCV
> and infinite timeout. And later when the thread will have something to
> do, you would simply skip it unless it's also paused.
> (I don't mind too much, as I understand this sleep will eventually go away)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20419

[Bernhard Kölbl]

On Fri Jan  6 11:22:03 2023 +0000, Rémi Bernon wrote:
> You'll probably need to signal the condition variable here, for instance
> to wake up the if the thread if it was paused.
> Hm, I had the idea of just putting it in the join function, but this seems better.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20439

[Bernhard Kölbl]

On Fri Jan  6 11:23:22 2023 +0000, Rémi Bernon wrote:
> I'm not completely sure how this should be done, but you have a race
> condition here. For instance, if:
> 1) StartAsync is called, setting worker_running to TRUE,
> 2) StopAsync is called, setting worker_running to FALSE,
> 3) session_stop_async is executed, not doing anything as the thread
> isn't started,
> 4) session_start_async is executed, starting the thread, setting the
> thread handle.
> The thread will exit quickly as running is false, but you'll end up with
> an inconsistent state.
> I think it's perhaps better to do everything in the async callbacks,
> serialized with the critical section. Or otherwise in the method itself,
> and have the callbacks do nothing (not sure if asynchronicity really
> matters here).
> What if I set `impl->worker_running = TRUE;` before the worker loop? That would mean it's only set when the worker is truly running.
> It would mean it runs at least one cycle, but I think that can be ignored?

```suggestion:-0+0
    EnterCriticalSection(&impl->cs);
    impl->worker_running = TRUE;
    while (impl->worker_running)
    {
        /* TODO: Send mic data to recognizer and handle results. */

        SleepConditionVariableCS(&impl->cv, &impl->cs, 500); /* Wait 500ms to slow down spinning for now. */
    }
    LeaveCriticalSection(&impl->cs);
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20444

[Bernhard Kölbl]

On Fri Jan  6 11:30:16 2023 +0000, Rémi Bernon wrote:
> (I don't mind too much, as I understand this sleep will eventually go away)
> The current way is pretty good for the later parts from what I can tell, so I'd keep it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/729#note_20490