[PATCH 3/4] winmm/tests: Add more MIDI System Exclusive message tests.

[Akihiro Sagawa]

Signed-off-by: Akihiro Sagawa 
---
 dlls/winmm/tests/midi.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

[Marvin]

Hi,

While running your changed tests on Windows, 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=37363

Your paranoid android.

=== w7u (32 bit midi) ===
midi.c:364: Test failed: bad message 1e/0 from midiOutLong unprepared, expect 0/feedf00d

=== w7pro64 (32 bit midi) ===
midi.c:364: Test failed: bad message 1e/0 from midiOutLong unprepared, expect 0/feedf00d

=== w7pro64 (64 bit midi) ===
midi.c:364: Test failed: bad message 1e/0 from midiOutLong unprepared, expect 0/feedf00d
The previous 2 run(s) terminated abnormally

[Andrew Eikum]

On Sun, Apr 01, 2018 at 11:43:38PM +0900, Akihiro Sagawa wrote:
> diff --git a/dlls/winmm/tests/midi.c b/dlls/winmm/tests/midi.c
> index bfe91dc..db5ae8f 100644
> --- a/dlls/winmm/tests/midi.c
> +++ b/dlls/winmm/tests/midi.c
> @@ -776,6 +777,73 @@ static void test_midiStream(UINT udev, HWND hwnd)
>          rc = midiStreamClose(hm);
>          ok(!rc, "midiStreamClose rc=%s\n", mmsys_error(rc));
>      }
> -
> - rc = midiOutGetDevCapsA((UINT_PTR)udev, &capsA, sizeof(capsA));
> - ok(!rc, "midiOutGetDevCaps(dev=%d) rc=%s\n", udev, mmsys_error(rc));
> - if (!strncmp(capsA.szPname, fluidsynth_prefix, strlen(fluidsynth_prefix)) ||
> -
>    (udev == MIDIMAPPER && found_fluidsynth)) {
> -
>    found_fluidsynth = TRUE;
> -
>    skip("FluidSynth (at least 1.1.6) doesn't support desired System Exclusive message.\n");
> -
>    return;
> - }
> -
>
> +#define ROUNDUP4(n) (((n) + 3) & ~3)
> - hm = NULL;
> - rc = midiStreamOpen(&hm, &udev, 1, (DWORD_PTR)0, (DWORD_PTR)0, CALLBACK_NULL);
> - ok(!rc, "midiOutOpen(dev=%d) rc=%s\n", udev, mmsys_error(rc));
> -
> - rc = midiStreamRestart(hm);
> - ok(!rc, "midiStreamRestart rc=%s\n", mmsys_error(rc));
> -
> - memset(&mhdr, 0, sizeof(mhdr));
> - mhdr.dwBufferLength = sizeof(MIDISHORTEVENT) * 5 + ROUNDUP4(sizeof(SysEx_reset)) +
> -
>    ROUNDUP4(sizeof(SysEx_volume_off)) + ROUNDUP4(sizeof(SysEx_volume_full));
> - mhdr.lpData = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, mhdr.dwBufferLength);
> - ok(mhdr.lpData!=NULL, "No %d bytes of memory!\n", mhdr.dwBufferLength);
> - if (mhdr.lpData) {
> -
>    MIDIEVENT *e;
> -
>    char *p = mhdr.lpData;
> -
>    /* GM System ON */
> -
>    e = (MIDIEVENT *)p;
> -
>    e->dwEvent = MEVT_F_LONG | sizeof(SysEx_reset);
> -
>    memcpy(&e->dwParms[0], SysEx_reset, sizeof(SysEx_reset));
> -
>    p += sizeof(MIDISHORTEVENT) + ROUNDUP4(sizeof(SysEx_reset));
> -
>    /* Master volume: off */
> -
>    e = (MIDIEVENT *)p;
> -
>    e->dwDeltaTime = 100;
> -
>    e->dwEvent = MEVT_F_LONG | sizeof(SysEx_volume_off);
> -
>    memcpy(&e->dwParms[0], SysEx_volume_off, sizeof(SysEx_volume_off));
> -
>    p += sizeof(MIDISHORTEVENT) + ROUNDUP4(sizeof(SysEx_volume_off));
> -
>    /* Note On (FIXME: we shouldn't hear this voice due to volume settings) */
> -
>    e = (MIDIEVENT *)p;
> -
>    e->dwEvent = MEVT_F_SHORT | 0x6F4593; /* note #69 */
> -
>    p += sizeof(MIDISHORTEVENT);
> -
>    /* Note Off */
> -
>    e = (MIDIEVENT *)p;
> -
>    e->dwDeltaTime = 400;

I noticed in patch 1, you added a Sleep(400) to let the note play. Is
that not needed here? Also, could the duration be shortened, so the
tests run more quickly?

Otherwise these patches look OK to me.

Thanks,
Andrew

[Akihiro Sagawa]

On Tue, 3 Apr 2018 13:21:49 -0500, Andrew Eikum wrote:
> On Sun, Apr 01, 2018 at 11:43:38PM +0900, Akihiro Sagawa wrote:

(snip)
> > -
> >    /* Note On (FIXME: we shouldn't hear this voice due to volume settings) */
> > -
> >    e = (MIDIEVENT *)p;
> > -
> >    e->dwEvent = MEVT_F_SHORT | 0x6F4593; /* note #69 */
> > -
> >    p += sizeof(MIDISHORTEVENT);
> > -
> >    /* Note Off */
> > -
> >    e = (MIDIEVENT *)p;
> > -
> >    e->dwDeltaTime = 400;
>
> I noticed in patch 1, you added a Sleep(400) to let the note play. Is
> that not needed here?

In MIDI Stream case, multimedia API maintains event timers.
dwDeltaTime member is used for that.
> Also, could the duration be shortened, so the tests run more quickly?

Sure. I reduced duration time from 400ms to 200ms and explicitly set time
division value. See v2 patch for details.

Thanks for reviewing,
Akihiro Sagawa