[PATCH v3 0/3] MR5141: dmime: bandtrack for MIDI files
Create a bandtrack for MIDI files by picking out the MIDI 0xCn events, and creating matching DMUS_PATCH_PMSG. * * * This also has a commit adding a stub chordtrack, which is small so I think a separate MR is unwarranted. Can be split out if necessary. -- v3: dmime: Add a stub chordtrack for MIDI segments. https://gitlab.winehq.org/wine/wine/-/merge_requests/5141
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/dmband/bandtrack.c | 1 + dlls/dmband/dmband_main.c | 1 + dlls/dmband/dmband_private.h | 10 ---------- dlls/dmusic/Makefile.in | 1 + dlls/{dmband => dmusic}/band.c | 11 ++++++++++- dlls/dmusic/band.h | 30 ++++++++++++++++++++++++++++++ 6 files changed, 43 insertions(+), 11 deletions(-) rename dlls/{dmband => dmusic}/band.c (99%) create mode 100644 dlls/dmusic/band.h diff --git a/dlls/dmband/bandtrack.c b/dlls/dmband/bandtrack.c index b410cf81d3a..2142d4969a0 100644 --- a/dlls/dmband/bandtrack.c +++ b/dlls/dmband/bandtrack.c @@ -17,6 +17,7 @@ */ #include "dmband_private.h" +#include "band.h" #include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmband); diff --git a/dlls/dmband/dmband_main.c b/dlls/dmband/dmband_main.c index 55f979e3fbc..2b470ba4d30 100644 --- a/dlls/dmband/dmband_main.c +++ b/dlls/dmband/dmband_main.c @@ -20,6 +20,7 @@ #include "initguid.h" #include "dmband_private.h" #include "rpcproxy.h" +#include "band.h" #include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmband); diff --git a/dlls/dmband/dmband_private.h b/dlls/dmband/dmband_private.h index 48c5ec02aeb..3b2617322fe 100644 --- a/dlls/dmband/dmband_private.h +++ b/dlls/dmband/dmband_private.h @@ -20,21 +20,16 @@ #ifndef __WINE_DMBAND_PRIVATE_H #define __WINE_DMBAND_PRIVATE_H -#include <stdio.h> #include <stdarg.h> -#include <string.h> #define COBJMACROS #include "windef.h" #include "winbase.h" #include "winnt.h" -#include "wingdi.h" -#include "winuser.h" #include "wine/debug.h" #include "wine/list.h" -#include "winreg.h" #include "objbase.h" #include "dmusici.h" @@ -44,11 +39,6 @@ /***************************************************************************** * ClassFactory */ -extern HRESULT create_dmband(REFIID riid, void **ret_iface); extern HRESULT create_dmbandtrack(REFIID riid, void **ret_iface); -extern HRESULT band_connect_to_collection(IDirectMusicBand *iface, IDirectMusicCollection *collection); -extern HRESULT band_send_messages(IDirectMusicBand *iface, IDirectMusicPerformance *performance, - IDirectMusicGraph *graph, MUSIC_TIME time, DWORD track_id); - #endif /* __WINE_DMBAND_PRIVATE_H */ diff --git a/dlls/dmusic/Makefile.in b/dlls/dmusic/Makefile.in index 8e1c07e90a7..446eca15543 100644 --- a/dlls/dmusic/Makefile.in +++ b/dlls/dmusic/Makefile.in @@ -2,6 +2,7 @@ MODULE = dmusic.dll IMPORTS = dxguid uuid ole32 advapi32 dsound user32 winmm SOURCES = \ + band.c \ buffer.c \ clock.c \ collection.c \ diff --git a/dlls/dmband/band.c b/dlls/dmusic/band.c similarity index 99% rename from dlls/dmband/band.c rename to dlls/dmusic/band.c index 78ec68ae035..2fdee5701b4 100644 --- a/dlls/dmband/band.c +++ b/dlls/dmusic/band.c @@ -17,7 +17,16 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ -#include "dmband_private.h" + +#define COBJMACROS + +#include "wine/debug.h" +#include "wine/list.h" +#include "objbase.h" + +#include "dmusici.h" +#include "dmusicf.h" + #include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmband); diff --git a/dlls/dmusic/band.h b/dlls/dmusic/band.h new file mode 100644 index 00000000000..660f208f34a --- /dev/null +++ b/dlls/dmusic/band.h @@ -0,0 +1,30 @@ +/* DirectMusicBand Private Include + * + * Copyright (C) 2024 Yuxuan Shui for CodeWeavers + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_BAND_H +#define __WINE_BAND_H + +#include "dmusici.h" + +extern HRESULT create_dmband(REFIID riid, void **ret_iface); +extern HRESULT band_connect_to_collection(IDirectMusicBand *iface, IDirectMusicCollection *collection); +extern HRESULT band_send_messages(IDirectMusicBand *iface, IDirectMusicPerformance *performance, + IDirectMusicGraph *graph, MUSIC_TIME time, DWORD track_id); + +#endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5141
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/dmime/Makefile.in | 1 + dlls/dmime/dmime_private.h | 2 +- dlls/dmime/midi.c | 118 +++++++++++++++++++++++++++------ dlls/dmime/segment.c | 20 ++---- dlls/dmime/tests/dmime.c | 130 +++++++++++++++++++++++++++++++++++-- dlls/dmusic/band.c | 14 ++++ dlls/dmusic/band.h | 2 + 7 files changed, 247 insertions(+), 40 deletions(-) diff --git a/dlls/dmime/Makefile.in b/dlls/dmime/Makefile.in index f8f622f8f3d..0f7b7db609e 100644 --- a/dlls/dmime/Makefile.in +++ b/dlls/dmime/Makefile.in @@ -4,6 +4,7 @@ PARENTSRC = ../dmusic SOURCES = \ audiopath.c \ + band.c \ dmime.idl \ dmime_main.c \ dmobject.c \ diff --git a/dlls/dmime/dmime_private.h b/dlls/dmime/dmime_private.h index cdf0810381d..11367a02d10 100644 --- a/dlls/dmime/dmime_private.h +++ b/dlls/dmime/dmime_private.h @@ -73,7 +73,7 @@ extern HRESULT create_dmwavetrack(REFIID riid, void **ret_iface); /* Create a new MIDI file parser. Note the stream might still be modified even * when this function fails. */ extern HRESULT midi_parser_new(IStream *stream, struct midi_parser **out_parser); -extern HRESULT midi_parser_next_track(struct midi_parser *parser, IDirectMusicTrack **out_track, MUSIC_TIME *out_length); +extern HRESULT midi_parser_parse(struct midi_parser *parser, IDirectMusicSegment8 *segment); extern void midi_parser_destroy(struct midi_parser *parser); extern void set_audiopath_perf_pointer(IDirectMusicAudioPath*,IDirectMusicPerformance8*); diff --git a/dlls/dmime/midi.c b/dlls/dmime/midi.c index a31146e636f..86d11afe228 100644 --- a/dlls/dmime/midi.c +++ b/dlls/dmime/midi.c @@ -18,6 +18,7 @@ #include "dmusic_midi.h" #include "dmime_private.h" +#include "band.h" #include "winternl.h" WINE_DEFAULT_DEBUG_CHANNEL(dmime); @@ -30,9 +31,23 @@ WINE_DEFAULT_DEBUG_CHANNEL(dmime); #define GET_BE_DWORD(x) RtlUlongByteSwap(x) #endif +struct midi_event +{ + MUSIC_TIME delta_time; + BYTE status; + BYTE data[2]; +}; + struct midi_parser { + IDirectMusicTrack *bandtrack; + /* The latest band in `bandtrack`. */ + IDirectMusicBand *band; + /* The music time of the latest band in `bandtrack`. */ + MUSIC_TIME band_time; + MUSIC_TIME time; IStream *stream; + DWORD division; }; static HRESULT stream_read_at_most(IStream *stream, void *buffer, ULONG size, ULONG *bytes_left) @@ -62,26 +77,27 @@ static HRESULT read_variable_length_number(IStream *stream, DWORD *out, ULONG *b return S_OK; } -static HRESULT read_midi_event(IStream *stream, BYTE *last_status, ULONG *bytes_left) +static HRESULT read_midi_event(IStream *stream, struct midi_event *event, BYTE *last_status, ULONG *bytes_left) { - BYTE byte, status, meta_type; + BYTE byte, status_type, meta_type; DWORD length; LARGE_INTEGER offset; HRESULT hr = S_OK; DWORD delta_time; if ((hr = read_variable_length_number(stream, &delta_time, bytes_left)) != S_OK) return hr; + event->delta_time = delta_time; if ((hr = stream_read_at_most(stream, &byte, 1, bytes_left)) != S_OK) return hr; if (byte & 0x80) { - status = *last_status = byte; + event->status = *last_status = byte; if ((hr = stream_read_at_most(stream, &byte, 1, bytes_left)) != S_OK) return hr; } - else status = *last_status; + else event->status = *last_status; - if (status == MIDI_META) + if (event->status == MIDI_META) { meta_type = byte; @@ -98,8 +114,9 @@ static HRESULT read_midi_event(IStream *stream, BYTE *last_status, ULONG *bytes_ *bytes_left -= length; } TRACE("MIDI meta event type %#02x, length %lu, time +%lu\n", meta_type, length, delta_time); + return S_OK; } - else if (status == MIDI_SYSEX1 || status == MIDI_SYSEX2) + else if (event->status == MIDI_SYSEX1 || event->status == MIDI_SYSEX2) { if (byte & 0x80) { @@ -112,33 +129,72 @@ static HRESULT read_midi_event(IStream *stream, BYTE *last_status, ULONG *bytes_ offset.QuadPart = length; if (FAILED(hr = IStream_Seek(stream, offset, STREAM_SEEK_CUR, NULL))) return hr; *bytes_left -= length; - FIXME("MIDI sysex event type %#02x, length %lu, time +%lu. not supported\n", status, length, delta_time); + FIXME("MIDI sysex event type %#02x, length %lu, time +%lu. not supported\n", event->status, + length, delta_time); + return S_OK; + } + + status_type = event->status & 0xf0; + if (status_type == MIDI_PROGRAM_CHANGE) + { + event->data[0] = byte; + TRACE("MIDI program change event status %#02x, data: %#02x, time +%lu\n", event->status, + event->data[0], delta_time); } else { - if ((status & 0xf0) != MIDI_PROGRAM_CHANGE && (status & 0xf0) != MIDI_CHANNEL_PRESSURE && - (hr = stream_read_at_most(stream, &byte, 1, bytes_left)) != S_OK) + if (status_type != MIDI_CHANNEL_PRESSURE && (hr = stream_read_at_most(stream, &byte, 1, bytes_left)) != S_OK) return hr; - FIXME("MIDI event status %#02x, time +%lu, not supported\n", status, delta_time); + FIXME("MIDI event status %#02x, time +%lu, not supported\n", event->status, delta_time); } return S_OK; } -HRESULT midi_parser_next_track(struct midi_parser *parser, IDirectMusicTrack **out_track, MUSIC_TIME *out_length) +static HRESULT midi_parser_handle_program_change(struct midi_parser *parser, struct midi_event *event) +{ + HRESULT hr; + DMUS_IO_INSTRUMENT instrument; + instrument.dwPChannel = event->status & 0xf; + instrument.dwFlags = DMUS_IO_INST_PATCH; + instrument.dwPatch = event->data[0]; + if (parser->band_time != parser->time) + { + DMUS_BAND_PARAM band_param; + MUSIC_TIME dmusic_time = (ULONGLONG)parser->time * DMUS_PPQ / parser->division; + if (parser->band) IDirectMusicBand_Release(parser->band); + parser->band = NULL; + if (FAILED(hr = CoCreateInstance(&CLSID_DirectMusicBand, NULL, CLSCTX_INPROC_SERVER, + &IID_IDirectMusicBand, (void **)&parser->band))) + return hr; + band_param.pBand = parser->band; + band_param.mtTimePhysical = dmusic_time; + TRACE("Adding band at time %lu\n", dmusic_time); + if (FAILED(hr = IDirectMusicTrack_SetParam(parser->bandtrack, &GUID_BandParam, dmusic_time, &band_param))) + return hr; + parser->band_time = parser->time; + } + return band_add_instrument(parser->band, &instrument); +} + +HRESULT midi_parser_parse(struct midi_parser *parser, IDirectMusicSegment8 *segment) { WORD i = 0; - TRACE("(%p, %p): stub\n", parser, out_length); + HRESULT hr; + MUSIC_TIME music_length = 0; + + TRACE("(%p): semi-stub\n", parser); + for (i = 0;; i++) { - HRESULT hr; BYTE magic[4] = {0}, last_status = 0; DWORD length_be; ULONG length; ULONG read = 0; + struct midi_event event = {0}; TRACE("Start parsing track %u\n", i); - if ((hr = IStream_Read(parser->stream, magic, sizeof(magic), &read)) != S_OK) return hr; + if ((hr = IStream_Read(parser->stream, magic, sizeof(magic), &read)) != S_OK) break; if (read < sizeof(magic)) break; if (memcmp(magic, "MTrk", 4) != 0) break; @@ -148,15 +204,26 @@ HRESULT midi_parser_next_track(struct midi_parser *parser, IDirectMusicTrack **o length = GET_BE_DWORD(length_be); TRACE("Track %u, length %lu bytes\n", i, length); - while ((hr = read_midi_event(parser->stream, &last_status, &length)) == S_OK) - ; + while ((hr = read_midi_event(parser->stream, &event, &last_status, &length)) == S_OK) + { + parser->time += event.delta_time; + if ((event.status & 0xf0) == MIDI_PROGRAM_CHANGE) + hr = midi_parser_handle_program_change(parser, &event); + if (FAILED(hr)) break; + } - if (FAILED(hr)) return hr; + if (FAILED(hr)) break; TRACE("End of track %u\n", i); + if (parser->time > music_length) music_length = parser->time; + parser->time = 0; } TRACE("End of file\n"); - return S_FALSE; + + music_length = (ULONGLONG)music_length * DMUS_PPQ / parser->division + 1; + if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_SetLength(segment, music_length); + if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_InsertTrack(segment, parser->bandtrack, 0xffff); + return hr; } HRESULT midi_parser_new(IStream *stream, struct midi_parser **out_parser) @@ -196,14 +263,25 @@ HRESULT midi_parser_new(IStream *stream, struct midi_parser **out_parser) parser = calloc(1, sizeof(struct midi_parser)); if (!parser) return E_OUTOFMEMORY; parser->stream = stream; - IStream_AddRef(stream); - *out_parser = parser; + parser->division = division; + parser->band_time = -1; + hr = CoCreateInstance(&CLSID_DirectMusicBandTrack, NULL, CLSCTX_INPROC_SERVER, + &IID_IDirectMusicTrack, (void **)&parser->bandtrack); + if (FAILED(hr)) + { + free(parser); + return hr; + } + *out_parser = parser; + IStream_AddRef(stream); return hr; } void midi_parser_destroy(struct midi_parser *parser) { IStream_Release(parser->stream); + IDirectMusicTrack_Release(parser->bandtrack); + if (parser->band) IDirectMusicBand_Release(parser->band); free(parser); } diff --git a/dlls/dmime/segment.c b/dlls/dmime/segment.c index b0485aaedd9..2de460f5601 100644 --- a/dlls/dmime/segment.c +++ b/dlls/dmime/segment.c @@ -791,10 +791,8 @@ static inline struct segment *impl_from_IPersistStream(IPersistStream *iface) static HRESULT WINAPI segment_persist_stream_Load(IPersistStream *iface, IStream *stream) { struct segment *This = impl_from_IPersistStream(iface); - IDirectMusicTrack *track; - MUSIC_TIME length; struct chunk_entry chunk = {0}; - struct midi_parser *midi_parser; + struct midi_parser *midi_parser = NULL; HRESULT hr; TRACE("(%p, %p): Loading\n", This, stream); @@ -810,18 +808,8 @@ static HRESULT WINAPI segment_persist_stream_Load(IPersistStream *iface, IStream break; case mmioFOURCC('M','T','h','d'): - hr = midi_parser_new(stream, &midi_parser); - if (FAILED(hr)) break; - This->header.mtLength = 0; - while ((hr = midi_parser_next_track(midi_parser, &track, &length)) == S_OK) - { - hr = segment_append_track(This, track, 1, 0); - IDirectMusicTrack_Release(track); - if (FAILED(hr)) break; - if (length > This->header.mtLength) - This->header.mtLength = length; - } - midi_parser_destroy(midi_parser); + if (FAILED(hr = midi_parser_new(stream, &midi_parser))) break; + if (FAILED(hr = midi_parser_parse(midi_parser, &This->IDirectMusicSegment8_iface))) break; break; case MAKE_IDTYPE(FOURCC_RIFF, mmioFOURCC('W','A','V','E')): @@ -844,6 +832,8 @@ static HRESULT WINAPI segment_persist_stream_Load(IPersistStream *iface, IStream } } + if (midi_parser) midi_parser_destroy(midi_parser); + if (chunk.id != mmioFOURCC('M', 'T', 'h', 'd')) stream_skip_chunk(stream, &chunk); diff --git a/dlls/dmime/tests/dmime.c b/dlls/dmime/tests/dmime.c index 7e1b419251e..41b34944f9d 100644 --- a/dlls/dmime/tests/dmime.c +++ b/dlls/dmime/tests/dmime.c @@ -1600,6 +1600,25 @@ static void _expect_track(IDirectMusicSegment8 *seg, REFCLSID expect, const char static void test_midi(void) { + static const DWORD message_types[] = + { + DMUS_PMSGT_MIDI, + DMUS_PMSGT_NOTE, + DMUS_PMSGT_SYSEX, + DMUS_PMSGT_NOTIFICATION, + DMUS_PMSGT_TEMPO, + DMUS_PMSGT_CURVE, + DMUS_PMSGT_TIMESIG, + DMUS_PMSGT_PATCH, + DMUS_PMSGT_TRANSPOSE, + DMUS_PMSGT_CHANNEL_PRIORITY, + DMUS_PMSGT_STOP, + DMUS_PMSGT_DIRTY, + DMUS_PMSGT_WAVE, + DMUS_PMSGT_LYRIC, + DMUS_PMSGT_SCRIPTLYRIC, + DMUS_PMSGT_USER, + }; static const char midi_meta_set_tempo[] = { 0x04, /* delta time = 4 */ @@ -1608,15 +1627,27 @@ static void test_midi(void) 0x03, /* event data lenght, 3 bytes */ 0x03,0x0d,0x40 /* tempo, 200000 us per quarter-note, i.e. 300 bpm */ }; + static const char midi_program_change[] = + { + 0x04, /* delta time = 4 */ + 0xc1, /* event type, program change, channel 1 */ + 0x30, /* event data, patch 48 */ + }; IDirectMusicSegment8 *segment = NULL; IDirectMusicTrack *track = NULL; IDirectMusicLoader8 *loader; + IDirectMusicTool *tool; + IDirectMusicPerformance *performance; + IDirectMusicGraph *graph; IPersistStream *persist; IStream *stream; LARGE_INTEGER zero = { .QuadPart = 0 }; ULARGE_INTEGER position = { .QuadPart = 0 }; WCHAR test_mid[MAX_PATH], bogus_mid[MAX_PATH]; HRESULT hr; + ULONG ret; + DMUS_PMSG *msg; + DMUS_PATCH_PMSG *patch; #include <pshpack1.h> struct { @@ -1649,7 +1680,7 @@ static void test_midi(void) &IID_IDirectMusicSegment, test_mid, (void **)&segment); ok(hr == S_OK, "got %#lx\n", hr); - todo_wine expect_track(segment, BandTrack, -1, 0); + expect_track(segment, BandTrack, -1, 0); todo_wine expect_track(segment, ChordTrack, -1, 1); todo_wine expect_track(segment, TempoTrack, -1, 2); todo_wine expect_track(segment, TimeSigTrack, -1, 3); @@ -1691,7 +1722,7 @@ static void test_midi(void) IPersistStream_Release(persist); IStream_Release(stream); /* TempoTrack and TimeSigTrack seems to be optional. */ - todo_wine expect_track(segment, BandTrack, -1, 0); + expect_track(segment, BandTrack, -1, 0); todo_wine expect_track(segment, ChordTrack, -1, 1); todo_wine expect_track(segment, SeqTrack, -1, 2); IDirectMusicSegment_Release(segment); @@ -1727,7 +1758,7 @@ static void test_midi(void) "got %lld\n", position.QuadPart); IPersistStream_Release(persist); IStream_Release(stream); - todo_wine expect_track(segment, BandTrack, -1, 0); + expect_track(segment, BandTrack, -1, 0); todo_wine expect_track(segment, ChordTrack, -1, 1); todo_wine expect_track(segment, TempoTrack, -1, 2); todo_wine expect_track(segment, SeqTrack, -1, 3); @@ -1763,11 +1794,102 @@ static void test_midi(void) ok(position.QuadPart == sizeof(header) + sizeof(track_header) + 4, "got %lld\n", position.QuadPart); IPersistStream_Release(persist); IStream_Release(stream); - todo_wine expect_track(segment, BandTrack, -1, 0); + expect_track(segment, BandTrack, -1, 0); todo_wine expect_track(segment, ChordTrack, -1, 1); /* there is no tempo track. */ todo_wine expect_track(segment, SeqTrack, -1, 2); IDirectMusicSegment_Release(segment); + + /* parse MIDI file with program change event. */ + + hr = CoCreateInstance(&CLSID_DirectMusicSegment, NULL, CLSCTX_INPROC_SERVER, + &IID_IDirectMusicSegment, (void **)&segment); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = IDirectMusicSegment_QueryInterface(segment, &IID_IPersistStream, (void **)&persist); + ok(hr == S_OK, "got %#lx\n", hr); + hr = CreateStreamOnHGlobal(0, TRUE, &stream); + ok(hr == S_OK, "got %#lx\n", hr); + header.format = GET_BE_WORD(123); + header.count = GET_BE_WORD(123); + header.ppqn = GET_BE_WORD(123); + header.length = GET_BE_DWORD(sizeof(header) - 8); + hr = IStream_Write(stream, &header, sizeof(header), NULL); + ok(hr == S_OK, "got %#lx\n", hr); + track_header.length = RtlUlongByteSwap(sizeof(track_header) - 8 + sizeof(midi_program_change)); + hr = IStream_Write(stream, &track_header, sizeof(track_header), NULL); + ok(hr == S_OK, "got %#lx\n", hr); + hr = IStream_Write(stream, midi_program_change, sizeof(midi_program_change), NULL); + ok(hr == S_OK, "got %#lx\n", hr); + hr = IStream_Seek(stream, zero, 0, NULL); + ok(hr == S_OK, "got %#lx\n", hr); + hr = IPersistStream_Load(persist, stream); + ok(hr == S_OK, "got %#lx\n", hr); + hr = IStream_Seek(stream, zero, STREAM_SEEK_CUR, &position); + ok(hr == S_OK, "got %#lx\n", hr); + ok(position.QuadPart == sizeof(header) + sizeof(track_header) + sizeof(midi_program_change), + "got %lld\n", position.QuadPart); + IPersistStream_Release(persist); + IStream_Release(stream); + expect_track(segment, BandTrack, -1, 0); + todo_wine expect_track(segment, ChordTrack, -1, 1); + todo_wine expect_track(segment, SeqTrack, -1, 2); + + hr = test_tool_create(message_types, ARRAY_SIZE(message_types), &tool); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = CoCreateInstance(&CLSID_DirectMusicPerformance, NULL, CLSCTX_INPROC_SERVER, + &IID_IDirectMusicPerformance, (void **)&performance); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = CoCreateInstance(&CLSID_DirectMusicGraph, NULL, CLSCTX_INPROC_SERVER, + &IID_IDirectMusicGraph, (void **)&graph); + ok(hr == S_OK, "got %#lx\n", hr); + hr = IDirectMusicGraph_InsertTool(graph, (IDirectMusicTool *)tool, NULL, 0, -1); + ok(hr == S_OK, "got %#lx\n", hr); + hr = IDirectMusicPerformance_SetGraph(performance, graph); + ok(hr == S_OK, "got %#lx\n", hr); + IDirectMusicGraph_Release(graph); + + /* now play the segment, and check produced messages + * wine generates: DIRTY, PATCH, DIRTY. + * native generates: DIRTY, PATCH + */ + + hr = IDirectMusicPerformance_Init(performance, NULL, 0, 0); + ok(hr == S_OK, "got %#lx\n", hr); + hr = IDirectMusicPerformance_PlaySegment(performance, (IDirectMusicSegment *)segment, 0x800, 0, NULL); + ok(hr == S_OK, "got %#lx\n", hr); + + ret = test_tool_wait_message(tool, 500, (DMUS_PMSG **)&msg); + ok(!ret, "got %#lx\n", ret); + ok(msg->dwType == DMUS_PMSGT_DIRTY, "got %#lx, expected DIRTY\n", msg->dwType); + hr = IDirectMusicPerformance_FreePMsg(performance, msg); + ok(hr == S_OK, "got %#lx\n", hr); + + ret = test_tool_wait_message(tool, 500, (DMUS_PMSG **)&msg); + ok(!ret, "got %#lx\n", ret); + ok(msg->dwType == DMUS_PMSGT_PATCH, "got msg type %#lx, expected PATCH\n", msg->dwType); + ok(msg->dwPChannel == 1, "got pchannel %lu, expected 1\n", msg->dwPChannel); + todo_wine ok(msg->mtTime == 23, "got mtTime %lu, expected 23\n", msg->mtTime); + patch = (DMUS_PATCH_PMSG *)msg; + ok(patch->byInstrument == 0x30, "got instrument %#x, expected 0x30\n", patch->byInstrument); + hr = IDirectMusicPerformance_FreePMsg(performance, msg); + ok(hr == S_OK, "got %#lx\n", hr); + + ret = test_tool_wait_message(tool, 500, (DMUS_PMSG **)&msg); + todo_wine ok(ret == WAIT_TIMEOUT, "unexpected message\n"); + if (!ret) + { + hr = IDirectMusicPerformance_FreePMsg(performance, msg); + ok(hr == S_OK, "got %#lx\n", hr); + } + + hr = IDirectMusicPerformance_CloseDown(performance); + ok(hr == S_OK, "got %#lx\n", hr); + IDirectMusicPerformance_Release(performance); + IDirectMusicTool_Release(tool); + IDirectMusicSegment_Release(segment); IDirectMusicLoader8_Release(loader); } diff --git a/dlls/dmusic/band.c b/dlls/dmusic/band.c index 2fdee5701b4..f21392bc46e 100644 --- a/dlls/dmusic/band.c +++ b/dlls/dmusic/band.c @@ -537,3 +537,17 @@ HRESULT band_send_messages(IDirectMusicBand *iface, IDirectMusicPerformance *per return hr; } + +HRESULT band_add_instrument(IDirectMusicBand *iface, DMUS_IO_INSTRUMENT *instrument) +{ + struct band *This = impl_from_IDirectMusicBand(iface); + struct instrument_entry *entry; + + TRACE("%p, %p\n", iface, instrument); + + if (!(entry = calloc(1, sizeof(*entry)))) return E_OUTOFMEMORY; + entry->instrument = *instrument; + list_add_tail(&This->instruments, &entry->entry); + + return S_OK; +} diff --git a/dlls/dmusic/band.h b/dlls/dmusic/band.h index 660f208f34a..327e84b0148 100644 --- a/dlls/dmusic/band.h +++ b/dlls/dmusic/band.h @@ -21,10 +21,12 @@ #define __WINE_BAND_H #include "dmusici.h" +#include "dmusicf.h" extern HRESULT create_dmband(REFIID riid, void **ret_iface); extern HRESULT band_connect_to_collection(IDirectMusicBand *iface, IDirectMusicCollection *collection); extern HRESULT band_send_messages(IDirectMusicBand *iface, IDirectMusicPerformance *performance, IDirectMusicGraph *graph, MUSIC_TIME time, DWORD track_id); +HRESULT band_add_instrument(IDirectMusicBand *iface, DMUS_IO_INSTRUMENT *instrument); #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5141
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/dmime/midi.c | 19 +++++++++++-------- dlls/dmime/tests/dmime.c | 10 +++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/dlls/dmime/midi.c b/dlls/dmime/midi.c index 86d11afe228..387b9ce73c9 100644 --- a/dlls/dmime/midi.c +++ b/dlls/dmime/midi.c @@ -40,6 +40,7 @@ struct midi_event struct midi_parser { + IDirectMusicTrack *chordtrack; IDirectMusicTrack *bandtrack; /* The latest band in `bandtrack`. */ IDirectMusicBand *band; @@ -223,6 +224,8 @@ HRESULT midi_parser_parse(struct midi_parser *parser, IDirectMusicSegment8 *segm music_length = (ULONGLONG)music_length * DMUS_PPQ / parser->division + 1; if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_SetLength(segment, music_length); if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_InsertTrack(segment, parser->bandtrack, 0xffff); + if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_InsertTrack(segment, parser->chordtrack, 0xffff); + return hr; } @@ -262,19 +265,18 @@ HRESULT midi_parser_new(IStream *stream, struct midi_parser **out_parser) parser = calloc(1, sizeof(struct midi_parser)); if (!parser) return E_OUTOFMEMORY; + IStream_AddRef(stream); parser->stream = stream; parser->division = division; parser->band_time = -1; hr = CoCreateInstance(&CLSID_DirectMusicBandTrack, NULL, CLSCTX_INPROC_SERVER, &IID_IDirectMusicTrack, (void **)&parser->bandtrack); - if (FAILED(hr)) - { - free(parser); - return hr; - } - - *out_parser = parser; - IStream_AddRef(stream); + if (FAILED(hr)) goto out; + hr = CoCreateInstance(&CLSID_DirectMusicChordTrack, NULL, CLSCTX_INPROC_SERVER, + &IID_IDirectMusicTrack, (void **)&parser->chordtrack); +out: + if (FAILED(hr)) midi_parser_destroy(parser); + else *out_parser = parser; return hr; } @@ -282,6 +284,7 @@ void midi_parser_destroy(struct midi_parser *parser) { IStream_Release(parser->stream); IDirectMusicTrack_Release(parser->bandtrack); + IDirectMusicTrack_Release(parser->chordtrack); if (parser->band) IDirectMusicBand_Release(parser->band); free(parser); } diff --git a/dlls/dmime/tests/dmime.c b/dlls/dmime/tests/dmime.c index 41b34944f9d..97a6088936e 100644 --- a/dlls/dmime/tests/dmime.c +++ b/dlls/dmime/tests/dmime.c @@ -1681,7 +1681,7 @@ static void test_midi(void) ok(hr == S_OK, "got %#lx\n", hr); expect_track(segment, BandTrack, -1, 0); - todo_wine expect_track(segment, ChordTrack, -1, 1); + expect_track(segment, ChordTrack, -1, 1); todo_wine expect_track(segment, TempoTrack, -1, 2); todo_wine expect_track(segment, TimeSigTrack, -1, 3); todo_wine expect_track(segment, SeqTrack, -1, 4); @@ -1723,7 +1723,7 @@ static void test_midi(void) IStream_Release(stream); /* TempoTrack and TimeSigTrack seems to be optional. */ expect_track(segment, BandTrack, -1, 0); - todo_wine expect_track(segment, ChordTrack, -1, 1); + expect_track(segment, ChordTrack, -1, 1); todo_wine expect_track(segment, SeqTrack, -1, 2); IDirectMusicSegment_Release(segment); @@ -1759,7 +1759,7 @@ static void test_midi(void) IPersistStream_Release(persist); IStream_Release(stream); expect_track(segment, BandTrack, -1, 0); - todo_wine expect_track(segment, ChordTrack, -1, 1); + expect_track(segment, ChordTrack, -1, 1); todo_wine expect_track(segment, TempoTrack, -1, 2); todo_wine expect_track(segment, SeqTrack, -1, 3); IDirectMusicSegment_Release(segment); @@ -1795,7 +1795,7 @@ static void test_midi(void) IPersistStream_Release(persist); IStream_Release(stream); expect_track(segment, BandTrack, -1, 0); - todo_wine expect_track(segment, ChordTrack, -1, 1); + expect_track(segment, ChordTrack, -1, 1); /* there is no tempo track. */ todo_wine expect_track(segment, SeqTrack, -1, 2); IDirectMusicSegment_Release(segment); @@ -1832,7 +1832,7 @@ static void test_midi(void) IPersistStream_Release(persist); IStream_Release(stream); expect_track(segment, BandTrack, -1, 0); - todo_wine expect_track(segment, ChordTrack, -1, 1); + expect_track(segment, ChordTrack, -1, 1); todo_wine expect_track(segment, SeqTrack, -1, 2); hr = test_tool_create(message_types, ARRAY_SIZE(message_types), &tool); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5141
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=143351 Your paranoid android. === debian11b (64 bit WoW report) === ntoskrnl.exe: driver.c:873: Test failed: got 0x102 driver.c:875: Test failed: DPC was not called. driver.c:878: Test failed: got 0
Rémi Bernon (@rbernon) commented about dlls/dmusic/band.h:
+/* DirectMusicBand Private Include Lets call this `dmusic_band.h`, similar to `dmusic_wave.h` and making it a bit more obvious that it's in dmusic module.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62605
Rémi Bernon (@rbernon) commented about dlls/dmusic/band.h:
+ * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_BAND_H +#define __WINE_BAND_H + You will see that we rarely bother with header guards in Wine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62606
Rémi Bernon (@rbernon) commented about dlls/dmime/midi.c:
- while ((hr = read_midi_event(parser->stream, &last_status, &length)) == S_OK) - ; + while ((hr = read_midi_event(parser->stream, &event, &last_status, &length)) == S_OK) + { + parser->time += event.delta_time; + if ((event.status & 0xf0) == MIDI_PROGRAM_CHANGE) + hr = midi_parser_handle_program_change(parser, &event); + if (FAILED(hr)) break; + }
- if (FAILED(hr)) return hr; + if (FAILED(hr)) break; TRACE("End of track %u\n", i); + if (parser->time > music_length) music_length = parser->time; + parser->time = 0;
These track time changes seem unrelated to the band parsing. You should probably split them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62608
Rémi Bernon (@rbernon) commented about dlls/dmusic/band.c:
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
-#include "dmband_private.h" + +#define COBJMACROS + +#include "wine/debug.h" +#include "wine/list.h" +#include "objbase.h" + +#include "dmusici.h" +#include "dmusicf.h" + #include "dmobject.h"
```suggestion:-11+0 #include "dmusic_private.h" ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62607
Rémi Bernon (@rbernon) commented about dlls/dmime/midi.c:
+ FIXME("MIDI event status %#02x, time +%lu, not supported\n", event->status, delta_time); }
return S_OK; }
-HRESULT midi_parser_next_track(struct midi_parser *parser, IDirectMusicTrack **out_track, MUSIC_TIME *out_length) +static HRESULT midi_parser_handle_program_change(struct midi_parser *parser, struct midi_event *event) +{ + HRESULT hr; + DMUS_IO_INSTRUMENT instrument; + instrument.dwPChannel = event->status & 0xf; + instrument.dwFlags = DMUS_IO_INST_PATCH; + instrument.dwPatch = event->data[0]; + if (parser->band_time != parser->time) + { Does it make any difference if you create one band for every event or one band for every event with the same time and add multiple instruments?
It seems to me that unless later proven that we need to do otherwise, you could just create a band on every event and then wouldn't need to keep last band and band time in the parser. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62609
Rémi Bernon (@rbernon) commented about dlls/dmime/segment.c:
case mmioFOURCC('M','T','h','d'): - hr = midi_parser_new(stream, &midi_parser); - if (FAILED(hr)) break; - This->header.mtLength = 0; - while ((hr = midi_parser_next_track(midi_parser, &track, &length)) == S_OK) - { - hr = segment_append_track(This, track, 1, 0); - IDirectMusicTrack_Release(track); - if (FAILED(hr)) break; - if (length > This->header.mtLength) - This->header.mtLength = length; - } - midi_parser_destroy(midi_parser); + if (FAILED(hr = midi_parser_new(stream, &midi_parser))) break; + if (FAILED(hr = midi_parser_parse(midi_parser, &This->IDirectMusicSegment8_iface))) break;
Now that you introduce a function that parses a MIDI file at once, do you even need midi_parser_new / midi_parser_destroy to be externally called? Imo you could call them directly in midi.c and make them static. I'm fine with the refactoring of the internal interface, but you could split it to a separate change, to make the band parsing commit simpler. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62610
On Mon Feb 26 07:39:06 2024 +0000, Rémi Bernon wrote:
These track time changes seem unrelated to the band parsing. You should probably split them. Track time is used to decide where to insert the bands in bandtrack.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62616
On Mon Feb 26 07:39:03 2024 +0000, Rémi Bernon wrote:
You will see that we rarely bother with header guards in Wine. Wait, what?
``` whatsit(a)camazotz:~/git/wine$ fgrep -L ifndef dlls/*/*.h | wc -l 163 whatsit(a)camazotz:~/git/wine$ fgrep -l ifndef dlls/*/*.h | wc -l 257 ``` (And yes, almost all of the latter are header guards.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5141#note_62690
participants (5)
-
Marvin -
Rémi Bernon -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)
-
Zebediah Figura (＠zfigura)