Signed-off-by: Alistair Leslie-Hughes leslie_alistair@hotmail.com --- dlls/dmime/performance.c | 79 +++++++++++++++++++++++++++++++--- dlls/dmime/tests/performance.c | 50 +++++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-)
diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index 399b9b9919..bffc8ff872 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -22,6 +22,16 @@
WINE_DEFAULT_DEBUG_CHANNEL(dmime);
+enum { + notify_chord, + notify_command, + notify_measurement, + notify_perforance, + notify_recompose, + notify_segment, + notify_custom +}; + typedef struct IDirectMusicPerformance8Impl { IDirectMusicPerformance8 IDirectMusicPerformance8_iface; LONG ref; @@ -50,6 +60,9 @@ typedef struct IDirectMusicPerformance8Impl { CRITICAL_SECTION safe; struct DMUS_PMSGItem *head; struct DMUS_PMSGItem *imm_head; + + BOOL notifications[7]; + GUID custom_guid; } IDirectMusicPerformance8Impl;
typedef struct DMUS_PMSGItem DMUS_PMSGItem; @@ -550,19 +563,72 @@ static HRESULT WINAPI IDirectMusicPerformance8Impl_GetNotificationPMsg(IDirectMu static HRESULT WINAPI IDirectMusicPerformance8Impl_AddNotificationType(IDirectMusicPerformance8 *iface, REFGUID rguidNotificationType) { - IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface); + IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface); + int type; + + TRACE("(%p, %s)\n", This, debugstr_dmguid(rguidNotificationType)); + + if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_CHORD)) + type = notify_chord; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_COMMAND)) + type = notify_command; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_MEASUREANDBEAT)) + type = notify_measurement; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_PERFORMANCE)) + type = notify_perforance; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_RECOMPOSE)) + type = notify_recompose; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_SEGMENT)) + type = notify_segment; + else { + type = notify_custom; + if (This->notifications[notify_custom]) + WARN("Only one custom notification currently supported\n"); + This->custom_guid = *rguidNotificationType; + }
- FIXME("(%p, %s): stub\n", This, debugstr_dmguid(rguidNotificationType)); - return S_OK; + if (This->notifications[type]) + return S_FALSE; + + This->notifications[type] = TRUE; + + return S_OK; }
static HRESULT WINAPI IDirectMusicPerformance8Impl_RemoveNotificationType(IDirectMusicPerformance8 *iface, REFGUID rguidNotificationType) { - IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface); + IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface); + int type; + + TRACE("(%p, %s)\n", This, debugstr_dmguid(rguidNotificationType)); + + if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_CHORD)) + type = notify_chord; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_COMMAND)) + type = notify_command; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_MEASUREANDBEAT)) + type = notify_measurement; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_PERFORMANCE)) + type = notify_perforance; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_RECOMPOSE)) + type = notify_recompose; + else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_SEGMENT)) + type = notify_segment; + else { + type = notify_custom; + if (IsEqualGUID (&This->custom_guid, rguidNotificationType)) + This->custom_guid = GUID_NULL; + else + WARN("Trying to remove an unknown custom notification\n"); + }
- FIXME("(%p, %s): stub\n", This, debugstr_dmguid(rguidNotificationType)); - return S_OK; + if (!This->notifications[type]) + return S_FALSE; + + This->notifications[type] = FALSE; + + return S_OK; }
static HRESULT WINAPI IDirectMusicPerformance8Impl_AddPort(IDirectMusicPerformance8 *iface, @@ -1214,6 +1280,7 @@ HRESULT WINAPI create_dmperformance(REFIID lpcGUID, void **ppobj) obj->rtLatencyTime = 100; /* 100 ms TO FIX */ obj->dwBumperLength = 50; /* 50 ms default */ obj->dwPrepareTime = 1000; /* 1000 ms default */ + obj->custom_guid = GUID_NULL; return IDirectMusicPerformance8Impl_QueryInterface(&obj->IDirectMusicPerformance8_iface, lpcGUID, ppobj); } diff --git a/dlls/dmime/tests/performance.c b/dlls/dmime/tests/performance.c index 4433846f9b..e4e5dc1b58 100644 --- a/dlls/dmime/tests/performance.c +++ b/dlls/dmime/tests/performance.c @@ -482,6 +482,55 @@ static void test_notification_type(void) IDirectMusicPerformance8_Release(perf); }
+static void test_add_remove_notification_types(void) +{ + IDirectMusicPerformance8 *perf; + HRESULT hr; + + hr = CoCreateInstance(&CLSID_DirectMusicPerformance, NULL, + CLSCTX_INPROC_SERVER, &IID_IDirectMusicPerformance8, (void**)&perf); + ok(hr == S_OK, "CoCreateInstance failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_SEGMENT); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_SEGMENT); + ok(hr == S_FALSE, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_COMMAND); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_MEASUREANDBEAT); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_PERFORMANCE); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_RECOMPOSE); + ok(hr == S_OK, "Failed: %08x\n", hr); + + /* RemoveNotificationType */ + hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_SEGMENT); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_SEGMENT); + ok(hr == S_FALSE, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_COMMAND); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_MEASUREANDBEAT); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_PERFORMANCE); + ok(hr == S_OK, "Failed: %08x\n", hr); + + hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_RECOMPOSE); + ok(hr == S_OK, "Failed: %08x\n", hr); + + IDirectMusicPerformance8_Release(perf); +} + START_TEST( performance ) { HRESULT hr; @@ -501,6 +550,7 @@ START_TEST( performance ) test_COM(); test_createport(); test_notification_type(); + test_add_remove_notification_types();
CoUninitialize(); }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=57395
Your paranoid android.
=== w1064v1507 (32 bit report) ===
dmime: performance.c:456: Test failed: Failed: 88781161 0f8c:performance: unhandled exception c0000005 at 004069F7
This isn't caused by this patch. It's crash in the function introduced yesterday. I've send another patch to fix the crash.
________________________________ From: Marvin testbot@winehq.org Sent: Friday, 4 October 2019 12:17 PM To: leslie_alistair@hotmail.com leslie_alistair@hotmail.com Cc: wine-devel@winehq.org wine-devel@winehq.org Subject: Re: [PATCH] dmime: Implemnet IDirectMusicPerformance8 Add/Remove NotificationType
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftestbot.wi...
Your paranoid android.
=== w1064v1507 (32 bit report) ===
dmime: performance.c:456: Test failed: Failed: 88781161 0f8c:performance: unhandled exception c0000005 at 004069F7
Hello Alistair,
is there an application that this patch fixes? Well more like stopping from crashing...
According to the documentation the Add/Remove NotificationType should be passed down to the segment and tracks: http://doc.51windows.net/directx9_sdk/htm/idirectmusicperformance8addnotific... http://doc.51windows.net/directx9_sdk/htm/idirectmusicperformance8removenoti... And of course native returns E_NOTIMPL for those methods in the implementation of some tracks...
So I don't mind adding a band aid in performance, but that should be still a FIXME with semi-stub or so. There is also quite some code duplication between the two functions, it might make sense to factor out the guid to type code.
Please see some other comments inline.
On 10/4/19 3:07 AM, Alistair Leslie-Hughes wrote:
Signed-off-by: Alistair Leslie-Hughes leslie_alistair@hotmail.com
dlls/dmime/performance.c | 79 +++++++++++++++++++++++++++++++--- dlls/dmime/tests/performance.c | 50 +++++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-)
diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index 399b9b9919..bffc8ff872 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -22,6 +22,16 @@
WINE_DEFAULT_DEBUG_CHANNEL(dmime);
+enum {
If you use an enum please name it and used it as a type for "type".
- notify_chord,
- notify_command,
- notify_measurement,
- notify_perforance,
^^ typo
- notify_recompose,
- notify_segment,
- notify_custom
+};
typedef struct IDirectMusicPerformance8Impl { IDirectMusicPerformance8 IDirectMusicPerformance8_iface; LONG ref; @@ -50,6 +60,9 @@ typedef struct IDirectMusicPerformance8Impl { CRITICAL_SECTION safe; struct DMUS_PMSGItem *head; struct DMUS_PMSGItem *imm_head;
- BOOL notifications[7];
It seems excessive to use an array of BOOLs. I'd rather prefer a bitfield.
- GUID custom_guid;
} IDirectMusicPerformance8Impl;
typedef struct DMUS_PMSGItem DMUS_PMSGItem; @@ -550,19 +563,72 @@ static HRESULT WINAPI IDirectMusicPerformance8Impl_GetNotificationPMsg(IDirectMu static HRESULT WINAPI IDirectMusicPerformance8Impl_AddNotificationType(IDirectMusicPerformance8 *iface, REFGUID rguidNotificationType) {
IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface);
- IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface);
- int type;
Please use the enum as type.
- TRACE("(%p, %s)\n", This, debugstr_dmguid(rguidNotificationType));
- if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_CHORD))
type = notify_chord;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_COMMAND))
type = notify_command;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_MEASUREANDBEAT))
type = notify_measurement;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_PERFORMANCE))
type = notify_perforance;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_RECOMPOSE))
type = notify_recompose;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_SEGMENT))
type = notify_segment;- else {
type = notify_custom;if (This->notifications[notify_custom])WARN("Only one custom notification currently supported\n");This->custom_guid = *rguidNotificationType;- }
- FIXME("(%p, %s): stub\n", This, debugstr_dmguid(rguidNotificationType));
- return S_OK;
if (This->notifications[type])return S_FALSE;- This->notifications[type] = TRUE;
- return S_OK;
}
static HRESULT WINAPI IDirectMusicPerformance8Impl_RemoveNotificationType(IDirectMusicPerformance8 *iface, REFGUID rguidNotificationType) {
IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface);
- IDirectMusicPerformance8Impl *This = impl_from_IDirectMusicPerformance8(iface);
- int type;
- TRACE("(%p, %s)\n", This, debugstr_dmguid(rguidNotificationType));
- if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_CHORD))
type = notify_chord;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_COMMAND))
type = notify_command;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_MEASUREANDBEAT))
type = notify_measurement;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_PERFORMANCE))
type = notify_perforance;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_RECOMPOSE))
type = notify_recompose;- else if (IsEqualGUID (rguidNotificationType, &GUID_NOTIFICATION_SEGMENT))
type = notify_segment;- else {
type = notify_custom;if (IsEqualGUID (&This->custom_guid, rguidNotificationType))This->custom_guid = GUID_NULL;elseWARN("Trying to remove an unknown custom notification\n");- }
- FIXME("(%p, %s): stub\n", This, debugstr_dmguid(rguidNotificationType));
- return S_OK;
- if (!This->notifications[type])
return S_FALSE;- This->notifications[type] = FALSE;
- return S_OK;
}
static HRESULT WINAPI IDirectMusicPerformance8Impl_AddPort(IDirectMusicPerformance8 *iface, @@ -1214,6 +1280,7 @@ HRESULT WINAPI create_dmperformance(REFIID lpcGUID, void **ppobj) obj->rtLatencyTime = 100; /* 100 ms TO FIX */ obj->dwBumperLength = 50; /* 50 ms default */ obj->dwPrepareTime = 1000; /* 1000 ms default */
obj->custom_guid = GUID_NULL; return IDirectMusicPerformance8Impl_QueryInterface(&obj->IDirectMusicPerformance8_iface, lpcGUID, ppobj);} diff --git a/dlls/dmime/tests/performance.c b/dlls/dmime/tests/performance.c index 4433846f9b..e4e5dc1b58 100644 --- a/dlls/dmime/tests/performance.c +++ b/dlls/dmime/tests/performance.c @@ -482,6 +482,55 @@ static void test_notification_type(void) IDirectMusicPerformance8_Release(perf); }
+static void test_add_remove_notification_types(void) +{
- IDirectMusicPerformance8 *perf;
- HRESULT hr;
- hr = CoCreateInstance(&CLSID_DirectMusicPerformance, NULL,
CLSCTX_INPROC_SERVER, &IID_IDirectMusicPerformance8, (void**)&perf);- ok(hr == S_OK, "CoCreateInstance failed: %08x\n", hr);
Please use an array and loop for all the guid pointers. That way you can check the double add / double remove for all guids. Btw. any reason you don't check the chord change notification? Also what happens if you pass in GUID_NULL as "custom" notification?
- hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_SEGMENT);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_SEGMENT);
- ok(hr == S_FALSE, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_COMMAND);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_MEASUREANDBEAT);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_PERFORMANCE);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_AddNotificationType(perf, &GUID_NOTIFICATION_RECOMPOSE);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- /* RemoveNotificationType */
- hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_SEGMENT);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_SEGMENT);
- ok(hr == S_FALSE, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_COMMAND);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_MEASUREANDBEAT);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_PERFORMANCE);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- hr = IDirectMusicPerformance8_RemoveNotificationType(perf, &GUID_NOTIFICATION_RECOMPOSE);
- ok(hr == S_OK, "Failed: %08x\n", hr);
- IDirectMusicPerformance8_Release(perf);
+}
START_TEST( performance ) { HRESULT hr; @@ -501,6 +550,7 @@ START_TEST( performance ) test_COM(); test_createport(); test_notification_type();
test_add_remove_notification_types();
CoUninitialize();
}
thanks bye michael
Hi Michael.
On 5/10/19 7:35 am, Michael Stefaniuc wrote:
Hello Alistair,
is there an application that this patch fixes? Well more like stopping from crashing...
No, this was just me trying to fill in some of the blanks without fully
understanding how works.
I'll take on board your comments and just send tests in first then
work on the implementation when I understand it better.
Thanks
Alistair.
-
Alistair Leslie-Hughes -
Marvin -
Michael Stefaniuc