-- v12: windows.media.speech: Implement the audio capturing system. windows.media.speech: Allow the recognition session worker to be paused. windows.media.speech: Add a worker thread to the recognition session.
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@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);
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@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 )
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@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);
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@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");
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@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);
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@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);
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@gmail.com --- dlls/windows.media.speech/private.h | 1 + dlls/windows.media.speech/recognizer.c | 133 ++++++++++++++++++++++- dlls/windows.media.speech/tests/speech.c | 8 +- 3 files changed, 134 insertions(+), 8 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..7e5e8e47302 100644 --- a/dlls/windows.media.speech/recognizer.c +++ b/dlls/windows.media.speech/recognizer.c @@ -160,6 +160,10 @@ struct session
struct list completed_handlers; struct list result_handlers; + + HANDLE worker_thread, worker_control_event; + BOOLEAN worker_running; + CRITICAL_SECTION cs; };
/* @@ -173,6 +177,33 @@ 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); + BOOLEAN running; + DWORD status; + + EnterCriticalSection(&impl->cs); + running = impl->worker_running; + LeaveCriticalSection(&impl->cs); + + while (running) + { + status = WaitForMultipleObjects(1, &impl->worker_control_event, FALSE, INFINITE); + if (status == 0) /* worker_control_event signaled */ + { + EnterCriticalSection(&impl->cs); + running = impl->worker_running; + LeaveCriticalSection(&impl->cs); + } + + /* TODO: Send mic data to recognizer and handle results. */ + } + + return 0; +} + static HRESULT WINAPI session_QueryInterface( ISpeechContinuousRecognitionSession *iface, REFIID iid, void **out ) { struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface); @@ -200,6 +231,28 @@ static ULONG WINAPI session_AddRef( ISpeechContinuousRecognitionSession *iface ) return ref; }
+static HRESULT session_join_worker_thread( struct session *session ) +{ + HANDLE thread; + + EnterCriticalSection(&session->cs); + thread = session->worker_thread; + LeaveCriticalSection(&session->cs); + + if (thread) + { + WaitForSingleObject(thread, INFINITE); + + CloseHandle(thread); + + EnterCriticalSection(&session->cs); + session->worker_thread = NULL; + LeaveCriticalSection(&session->cs); + } + + return S_OK; +} + static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface ) { struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface); @@ -208,8 +261,19 @@ static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface
if (!ref) { + EnterCriticalSection(&impl->cs); + impl->worker_running = FALSE; + LeaveCriticalSection(&impl->cs); + SetEvent(impl->worker_control_event); + + session_join_worker_thread(impl); + 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); } @@ -254,8 +318,34 @@ static HRESULT session_start_async( 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, session_start_async, action); + struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface); + BOOLEAN invalid_state = FALSE; + HRESULT hr = S_OK; + + TRACE("iface %p, action %p.\n", iface, action); + + *action = NULL; + + EnterCriticalSection(&impl->cs); + if (!impl->worker_running && !impl->worker_thread) + { + impl->worker_running = TRUE; + if (!(impl->worker_thread = CreateThread(NULL, 0, session_worker_thread_cb, impl, 0, NULL))) + { + hr = HRESULT_FROM_WIN32(GetLastError()); + impl->worker_running = FALSE; + } + } + else + invalid_state = TRUE; + LeaveCriticalSection(&impl->cs); + + if (invalid_state) + hr = COR_E_INVALIDOPERATION; + else + async_action_create(NULL, session_start_async, action); + + return hr; }
static HRESULT WINAPI session_StartWithModeAsync( ISpeechContinuousRecognitionSession *iface, @@ -273,8 +363,31 @@ static HRESULT session_stop_async( IInspectable *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 invalid_state = FALSE; + HRESULT hr; + + TRACE("iface %p, action %p.\n", iface, action); + + *action = NULL; + + EnterCriticalSection(&impl->cs); + if (impl->worker_running && impl->worker_thread) + impl->worker_running = FALSE; + else + invalid_state = TRUE; + LeaveCriticalSection(&impl->cs); + + if (invalid_state) + hr = COR_E_INVALIDOPERATION; + else + { + SetEvent(impl->worker_control_event); + session_join_worker_thread(impl); + hr = async_action_create((IInspectable *)iface, session_stop_async, action); + } + + return hr; }
static HRESULT WINAPI session_CancelAsync( ISpeechContinuousRecognitionSession *iface, IAsyncAction **action ) @@ -818,6 +931,15 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface list_init(&session->completed_handlers); list_init(&session->result_handlers);
+ if (!(session->worker_control_event = CreateEventW(NULL, FALSE, FALSE, NULL))) + { + hr = HRESULT_FROM_WIN32(GetLastError()); + goto error; + } + + 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 +957,9 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface
error: if (session->constraints) IVector_ISpeechRecognitionConstraint_Release(session->constraints); + if (session->cs.DebugInfo) session->cs.DebugInfo->Spare[0] = 0; + DeleteCriticalSection(&session->cs); + CloseHandle(session->worker_control_event); 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);
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@gmail.com --- dlls/windows.media.speech/recognizer.c | 27 +++++++++++++++++++----- dlls/windows.media.speech/tests/speech.c | 4 ++-- 2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c index 7e5e8e47302..c4ba419ebe6 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, worker_control_event; - BOOLEAN worker_running; + BOOLEAN worker_running, worker_paused; CRITICAL_SECTION cs; };
@@ -398,19 +398,36 @@ 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); + + SetEvent(impl->worker_control_event); + 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); + + SetEvent(impl->worker_control_event); + + 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);
From: Bernhard Kölbl besentv@gmail.com
Signed-off-by: Bernhard Kölbl besentv@gmail.com --- dlls/windows.media.speech/recognizer.c | 126 ++++++++++++++++++++++++- 1 file changed, 122 insertions(+), 4 deletions(-)
diff --git a/dlls/windows.media.speech/recognizer.c b/dlls/windows.media.speech/recognizer.c index c4ba419ebe6..5781c29487f 100644 --- a/dlls/windows.media.speech/recognizer.c +++ b/dlls/windows.media.speech/recognizer.c @@ -17,9 +17,15 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
+#include "initguid.h" #include "private.h"
+#include "audioclient.h" +#include "mmdeviceapi.h" + +#include "winbase.h" #include "wine/debug.h" +#include "winnt.h"
WINE_DEFAULT_DEBUG_CHANNEL(speech);
@@ -161,7 +167,10 @@ struct session struct list completed_handlers; struct list result_handlers;
- HANDLE worker_thread, worker_control_event; + IAudioClient *audio_client; + IAudioCaptureClient *capture_client; + + HANDLE worker_thread, worker_control_event, audio_buf_event; BOOLEAN worker_running, worker_paused; CRITICAL_SECTION cs; }; @@ -181,8 +190,16 @@ static DWORD CALLBACK session_worker_thread_cb( void *args ) { ISpeechContinuousRecognitionSession *iface = args; struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface); - BOOLEAN running; + BOOLEAN running, paused = FALSE; + HANDLE events[2]; DWORD status; + UINT32 frame_count, frames_available; + BYTE *audio_buf; + DWORD flags; + + IAudioClient_Start(impl->audio_client); + IAudioClient_GetBufferSize(impl->audio_client, &frame_count); + FIXME("frame_count %u.\n", frame_count);
EnterCriticalSection(&impl->cs); running = impl->worker_running; @@ -190,17 +207,52 @@ static DWORD CALLBACK session_worker_thread_cb( void *args )
while (running) { - status = WaitForMultipleObjects(1, &impl->worker_control_event, FALSE, INFINITE); + BOOLEAN old_paused = paused; + UINT32 count = 0; + + events[count++] = impl->worker_control_event; + if (!paused) events[count++] = impl->audio_buf_event; + + status = WaitForMultipleObjects(ARRAY_SIZE(events), events, FALSE, INFINITE); if (status == 0) /* worker_control_event signaled */ { EnterCriticalSection(&impl->cs); + paused = impl->worker_paused; running = impl->worker_running; LeaveCriticalSection(&impl->cs); + + if (old_paused < paused) + { + IAudioClient_Stop(impl->audio_client); + TRACE("session worker paused.\n"); + } + if (old_paused > paused) + { + IAudioClient_Start(impl->audio_client); + TRACE("session worker resumed.\n"); + } + + continue; } + if (status == 1) /* audio_buf_event signaled */ + { + while (IAudioCaptureClient_GetBuffer(impl->capture_client, &audio_buf, &frames_available, &flags, NULL, NULL) == S_OK) + { + /* TODO: Send mic data to recognizer and handle results. */ + IAudioCaptureClient_ReleaseBuffer(impl->capture_client, frames_available); + }
- /* TODO: Send mic data to recognizer and handle results. */ + continue; + } + else + { + ERR("Unexpected state entered. Aborting worker!\n"); + break; + } }
+ IAudioClient_Stop(impl->audio_client); + return 0; }
@@ -271,6 +323,9 @@ static ULONG WINAPI session_Release( ISpeechContinuousRecognitionSession *iface typed_event_handlers_clear(&impl->completed_handlers); typed_event_handlers_clear(&impl->result_handlers);
+ IAudioCaptureClient_Release(impl->capture_client); + IAudioClient_Release(impl->audio_client); + impl->cs.DebugInfo->Spare[0] = 0; DeleteCriticalSection(&impl->cs);
@@ -914,6 +969,64 @@ static const struct IActivationFactoryVtbl activation_factory_vtbl =
DEFINE_IINSPECTABLE(recognizer_factory, ISpeechRecognizerFactory, struct recognizer_statics, IActivationFactory_iface)
+static HRESULT recognizer_factory_create_audio_capture(struct session *session) +{ + const REFERENCE_TIME buffer_duration = 5000000; /* 0.5 second */ + IMMDeviceEnumerator *mm_enum = NULL; + IMMDevice *mm_device = NULL; + WAVEFORMATEX *wfx = NULL; + WCHAR *str = NULL; + HRESULT hr = S_OK; + + session->audio_buf_event = CreateEventW(NULL, FALSE, FALSE, NULL); + if (FAILED(hr = HRESULT_FROM_WIN32(GetLastError()))) + return hr; + + if (FAILED(hr = CoCreateInstance(&CLSID_MMDeviceEnumerator, NULL, CLSCTX_INPROC_SERVER, &IID_IMMDeviceEnumerator, (void**)&mm_enum))) + goto cleanup; + + if (FAILED(hr = IMMDeviceEnumerator_GetDefaultAudioEndpoint(mm_enum, eCapture, eMultimedia, &mm_device))) + goto cleanup; + + if (FAILED(hr = IMMDevice_Activate(mm_device, &IID_IAudioClient, CLSCTX_INPROC_SERVER, NULL, (void**)&session->audio_client))) + goto cleanup; + + if (SUCCEEDED(hr = IMMDevice_GetId(mm_device, &str))) + { + TRACE("selected capture device ID: %s\n", debugstr_w(str)); + CoTaskMemFree(str); + } + + if (FAILED(hr = IAudioClient_GetMixFormat(session->audio_client, (WAVEFORMATEX **)&wfx))) + goto cleanup; + + wfx->wFormatTag = WAVE_FORMAT_PCM; + wfx->nChannels = 1; + wfx->nSamplesPerSec = 16000; + wfx->nBlockAlign = 2; + wfx->wBitsPerSample = 16; + wfx->nAvgBytesPerSec = wfx->nSamplesPerSec * wfx->nBlockAlign; + TRACE("wfx tag %u, channels %u, samples %lu, bits %u, align %u.\n", wfx->wFormatTag, wfx->nChannels, wfx->nSamplesPerSec, wfx->wBitsPerSample, wfx->nBlockAlign); + + if (FAILED(hr = IAudioClient_Initialize(session->audio_client, AUDCLNT_SHAREMODE_SHARED, AUDCLNT_STREAMFLAGS_EVENTCALLBACK, buffer_duration, 0, wfx, NULL))) + goto cleanup; + + if (FAILED(hr = IAudioClient_SetEventHandle(session->audio_client, session->audio_buf_event))) + goto cleanup; + + hr = IAudioClient_GetService(session->audio_client, &IID_IAudioCaptureClient, (void**)&session->capture_client); + +cleanup: + if (FAILED(hr)) + { + if (session->audio_client) IAudioClient_Release(session->audio_client); + if (session->audio_buf_event) CloseHandle(session->audio_buf_event); + } + if (mm_device) IMMDevice_Release(mm_device); + if (mm_enum) IMMDeviceEnumerator_Release(mm_enum); + return hr; +} + static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface, ILanguage *language, ISpeechRecognizer **speechrecognizer ) { struct recognizer *impl; @@ -960,6 +1073,9 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface if (FAILED(hr = vector_inspectable_create(&constraints_iids, (IVector_IInspectable**)&session->constraints))) goto error;
+ if (FAILED(hr = recognizer_factory_create_audio_capture(session))) + goto error; + /* Init ISpeechRecognizer */ impl->ISpeechRecognizer_iface.lpVtbl = &speech_recognizer_vtbl; impl->IClosable_iface.lpVtbl = &closable_vtbl; @@ -973,6 +1089,8 @@ static HRESULT WINAPI recognizer_factory_Create( ISpeechRecognizerFactory *iface return S_OK;
error: + if (session->capture_client) IAudioCaptureClient_Release(session->capture_client); + if (session->audio_client) IAudioClient_Release(session->audio_client); if (session->constraints) IVector_ISpeechRecognitionConstraint_Release(session->constraints); if (session->cs.DebugInfo) session->cs.DebugInfo->Spare[0] = 0; DeleteCriticalSection(&session->cs);
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=128285
Your paranoid android.
=== debian11 (build log) ===
collect2: error: ld returned 1 exit status Task: The win32 Wine build failed
=== debian11b (build log) ===
collect2: error: ld returned 1 exit status Task: The wow64 Wine build failed
Connor McAdams (@cmcadams) commented about dlls/windows.media.speech/recognizer.c:
{EnterCriticalSection(&impl->cs);paused = impl->worker_paused;running = impl->worker_running;LeaveCriticalSection(&impl->cs);if (old_paused < paused){IAudioClient_Stop(impl->audio_client);TRACE("session worker paused.\n");}if (old_paused > paused){IAudioClient_Start(impl->audio_client);TRACE("session worker resumed.\n");}
It makes no difference functionally, but it'd probably be prettier if this was: ``` if (old_paused < paused) { IAudioClient_Stop(impl->audio_client); TRACE("session worker paused.\n"); } else if (old_paused > paused) { IAudioClient_Start(impl->audio_client); TRACE("session worker resumed.\n"); } ```
Connor McAdams (@cmcadams) commented about dlls/windows.media.speech/recognizer.c:
{ ISpeechContinuousRecognitionSession *iface = args; struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
- BOOLEAN running, paused = FALSE;
- HANDLE events[2];
- DWORD status;
- UINT32 frame_count, frames_available;
- BYTE *audio_buf;
- DWORD flags;
- IAudioClient_Start(impl->audio_client);
- IAudioClient_GetBufferSize(impl->audio_client, &frame_count);
- FIXME("frame_count %u.\n", frame_count);
Leftover from debugging, I assume.
Connor McAdams (@cmcadams) commented about dlls/windows.media.speech/recognizer.c:
if (old_paused < paused){IAudioClient_Stop(impl->audio_client);TRACE("session worker paused.\n");}if (old_paused > paused){IAudioClient_Start(impl->audio_client);TRACE("session worker resumed.\n");}
TRACE("Session worker resumed.\n"); continue; }
if (status == 1) /* audio_buf_event signaled */
instead of a magic value of 1 in `status == 1` could you maybe have some enum/macro?
On Tue Jan 10 23:41:14 2023 +0000, Connor McAdams wrote:
instead of a magic value of 1 in `status == 1` could you maybe have some enum/macro?
Wouldn't make much sense imho as the indexes are computed and aren't static. As long as there's two events and as this loop stays small I think it's fine to use literals.
On Wed Jan 11 16:35:14 2023 +0000, Rémi Bernon wrote:
Wouldn't make much sense imho as the indexes are computed and aren't static. As long as there's two events and as this loop stays small I think it's fine to use literals.
Ah, right. I guess there's `WAIT_OBJECT_0` and so on, but I guess they don't really add too much extra clarity.
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
error: if (session->constraints) IVector_ISpeechRecognitionConstraint_Release(session->constraints);
- if (session->cs.DebugInfo) session->cs.DebugInfo->Spare[0] = 0;
- DeleteCriticalSection(&session->cs);
You could avoid having to do this (and the question whether calling `DeleteCriticalSection` on a non-yet initialized CS is fine) by moving `InitializeCriticalSection` down where it cannot fail anymore.
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);
- BOOLEAN running;
- DWORD status;
- EnterCriticalSection(&impl->cs);
- running = impl->worker_running;
- LeaveCriticalSection(&impl->cs);
- while (running)
I think you can assume running is true on first iteration, then you should get an event notification if it's been stopped already, and read it from within the CS in a single place. You can also maybe change this to a `do {} while(running)` loop too.
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
if (status == 1) /* audio_buf_event signaled */{while (IAudioCaptureClient_GetBuffer(impl->capture_client, &audio_buf, &frames_available, &flags, NULL, NULL) == S_OK){/* TODO: Send mic data to recognizer and handle results. */IAudioCaptureClient_ReleaseBuffer(impl->capture_client, frames_available);}
/* TODO: Send mic data to recognizer and handle results. */
continue;}else{ERR("Unexpected state entered. Aborting worker!\n");break;}
The `if { continue; } / if { continue; } / else ` pattern feels a bit weird. Perhaps `if / else if / else`, or just keep the continues and keep the break without a else.
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
if (old_paused < paused){IAudioClient_Stop(impl->audio_client);TRACE("session worker paused.\n");}if (old_paused > paused){IAudioClient_Start(impl->audio_client);TRACE("session worker resumed.\n");}continue; }if (status == 1) /* audio_buf_event signaled */{while (IAudioCaptureClient_GetBuffer(impl->capture_client, &audio_buf, &frames_available, &flags, NULL, NULL) == S_OK)
Do you really want a nested loop here, what if it never ends? It's what the audio event is supposed to signal right? So maybe just rely on the event being signaled each time you can call `IAudioCaptureClient_GetBuffer`?
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
while (running) {
status = WaitForMultipleObjects(1, &impl->worker_control_event, FALSE, INFINITE);
BOOLEAN old_paused = paused;UINT32 count = 0;events[count++] = impl->worker_control_event;if (!paused) events[count++] = impl->audio_buf_event;status = WaitForMultipleObjects(ARRAY_SIZE(events), events, FALSE, INFINITE);
You need to indicate the actual number of events, not always `ARRAY_SIZE(events)` here, or don't make the second event conditional.
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
DEFINE_IINSPECTABLE(recognizer_factory, ISpeechRecognizerFactory, struct recognizer_statics, IActivationFactory_iface)
+static HRESULT recognizer_factory_create_audio_capture(struct session *session) +{
- const REFERENCE_TIME buffer_duration = 5000000; /* 0.5 second */
- IMMDeviceEnumerator *mm_enum = NULL;
- IMMDevice *mm_device = NULL;
- WAVEFORMATEX *wfx = NULL;
- WCHAR *str = NULL;
- HRESULT hr = S_OK;
- session->audio_buf_event = CreateEventW(NULL, FALSE, FALSE, NULL);
- if (FAILED(hr = HRESULT_FROM_WIN32(GetLastError())))
return hr;
I don't think last error is reset on success, you probably need to check the event value.
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
- if (FAILED(hr = CoCreateInstance(&CLSID_MMDeviceEnumerator, NULL, CLSCTX_INPROC_SERVER, &IID_IMMDeviceEnumerator, (void**)&mm_enum)))
goto cleanup;- if (FAILED(hr = IMMDeviceEnumerator_GetDefaultAudioEndpoint(mm_enum, eCapture, eMultimedia, &mm_device)))
goto cleanup;- if (FAILED(hr = IMMDevice_Activate(mm_device, &IID_IAudioClient, CLSCTX_INPROC_SERVER, NULL, (void**)&session->audio_client)))
goto cleanup;- if (SUCCEEDED(hr = IMMDevice_GetId(mm_device, &str)))
- {
TRACE("selected capture device ID: %s\n", debugstr_w(str));CoTaskMemFree(str);- }
- if (FAILED(hr = IAudioClient_GetMixFormat(session->audio_client, (WAVEFORMATEX **)&wfx)))
Don't you need to `CoTaskMemFree(wfx)` somewhere maybe?
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
return S_OK;error:
- if (session->capture_client) IAudioCaptureClient_Release(session->capture_client);
- if (session->audio_client) IAudioClient_Release(session->audio_client);
I don't think this can happen right now, though maybe it's fine to keep.
Rémi Bernon (@rbernon) commented about dlls/windows.media.speech/recognizer.c:
- HANDLE thread;
- EnterCriticalSection(&session->cs);
- thread = session->worker_thread;
- LeaveCriticalSection(&session->cs);
- if (thread)
- {
WaitForSingleObject(thread, INFINITE);CloseHandle(thread);EnterCriticalSection(&session->cs);session->worker_thread = NULL;LeaveCriticalSection(&session->cs);- }
Although I think it's okay given the rest of the code, I feel a bit unsafe about this CS re-entry.
I think that maybe it would be better to read and reset `session->worker_thread` to `NULL` at once, although I haven't completely thought about its effect on consistency.
You could also arguably drop the condition and wait and close on the resulting thread even if it is `NULL`, as a `NULL` handle will just do nothing.
On Wed Jan 11 17:00:57 2023 +0000, Rémi Bernon wrote:
I think you can assume running is true on first iteration, then you should get an event notification if it's been stopped already, and read it from within the CS in a single place. You can also maybe change this to a `do {} while(running)` loop too.
Assuming `worker_control_event` is used purely for notifcation or wake-up and the thread never has to reset or wait on the event, I think it makes more sense to change it into a `for (;;)` loop, and move the running/paused check to the start of the loop.
Jinoh Kang (@iamahuman) commented about dlls/windows.media.speech/recognizer.c:
- IAudioClient_GetBufferSize(impl->audio_client, &frame_count);
- FIXME("frame_count %u.\n", frame_count);
- EnterCriticalSection(&impl->cs);
- running = impl->worker_running;
- LeaveCriticalSection(&impl->cs);
- while (running)
- {
BOOLEAN old_paused = paused;UINT32 count = 0;events[count++] = impl->worker_control_event;if (!paused) events[count++] = impl->audio_buf_event;status = WaitForMultipleObjects(ARRAY_SIZE(events), events, FALSE, INFINITE);
```suggestion:-0+0 status = WaitForMultipleObjects(count, events, FALSE, INFINITE); ```
Jinoh Kang (@iamahuman) commented about dlls/windows.media.speech/recognizer.c:
return ref;}
+static HRESULT session_join_worker_thread( struct session *session ) +{
- HANDLE thread;
- EnterCriticalSection(&session->cs);
- thread = session->worker_thread;
- LeaveCriticalSection(&session->cs);
If `session_join-worker_thread` is free from data race, no other threads will modify `worker_thread` until `session_join-worker_thread` sets it to `NULL`. The converse is also true: if any other thread could modify `worker_thread`, then `session_join_worker_thread` has a data race regardless of the critical section. Therefore, the critical section acquisition here is unnecessary.
Although I think there are no more obvious race bugs in your code, it is still hard to verify its race-free status due to unclear ownership of resources. In this case, every caller of `session_join_worker_thread` conceptually _owns_ the `worker_thread` variable, but each by different means:
1. `session_StopAsync` owns `worker_thread` by the virtue of transitioning `worker_running` from `TRUE` to `FALSE`. No other threads will mutate `worker_thread` until it is set to `NULL` again. 2. `session_Release` owns `worker_thread` since the COM contract guarantees that no user reference to the object exists when the reference count drops to zero. The only existing reference is from the worker thread, which never accesses `worker_thread`.
I think the code will be much clearer if `session_join_worker_thread` accepted `worker_thread` obtained from the caller's crtical section. This way, only the ownership of the thread handle itself, and not the entire variable, needs to be verified. You can take it further and make sure to set `worker_thread` to an invalid handle for other threads so that the thread handle isn't accessible by others at all. This will guarantee that nothing will concurrently close the handle, greatly simplifying verification.
On Wed Jan 11 17:00:57 2023 +0000, Rémi Bernon wrote:
I think you can assume running is true on first iteration, then you should get an event notification if it's been stopped already, and read it from within the CS in a single place. You can also maybe change this to a `do {} while(running)` loop too.
Right. If you skip the loop and don't wait on the `worker_control_event` handle, the event won't be reset and thus wake up the next worker thread when the user calls `StartAsync` the second time.
On Fri Jan 6 11:22:02 2023 +0000, Rémi Bernon wrote:
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.
Maybe revisit this comment since we've switched from CVs to events? Well, it's fine if we are going to tolerate spurious wakeups from events, though. Note that CVs are generally always prone to spurious wakeups regardless of our usage.
Jinoh Kang (@iamahuman) commented about dlls/windows.media.speech/recognizer.c:
}
+static DWORD CALLBACK session_worker_thread_cb( void *args ) +{
- ISpeechContinuousRecognitionSession *iface = args;
- struct session *impl = impl_from_ISpeechContinuousRecognitionSession(iface);
- BOOLEAN running, paused = FALSE;
- HANDLE events[2];
- DWORD status;
- UINT32 frame_count, frames_available;
- BYTE *audio_buf;
- DWORD flags;
- IAudioClient_Start(impl->audio_client);
- IAudioClient_GetBufferSize(impl->audio_client, &frame_count);
- FIXME("frame_count %u.\n", frame_count);
```suggestion:-0+0 ```
-
Bernhard Kölbl -
Bernhard Kölbl (＠besentv)
-
Connor McAdams (＠cmcadams)
-
Jinoh Kang (＠iamahuman)
-
Marvin -
Rémi Bernon (＠rbernon)