[PATCH v2 0/7] MR3795: dmusic: Rewrite collection parsing with the new stream chunk primitives.
-- v2: dmusic: Add more parsed instruments traces. dmusic: Parse instrument name from INFO list. dmusic: Rewrite collection DLS chunk parsing. dmusic: Parse desc using stream_chunk_parse_desc. dmusic: Rewrite collection ptbl chunk parsing. dmusic: Rewrite collection INFO list parsing. dmusic: Rewrite collection lins list parsing. https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/buffer.c | 1 - dlls/dmusic/clock.c | 1 - dlls/dmusic/collection.c | 67 ++++++++++++++++++------------------ dlls/dmusic/dmusic.c | 1 - dlls/dmusic/dmusic_main.c | 1 - dlls/dmusic/dmusic_private.h | 5 ++- dlls/dmusic/download.c | 1 - dlls/dmusic/instrument.c | 28 +++------------ dlls/dmusic/port.c | 1 - 9 files changed, 43 insertions(+), 63 deletions(-) diff --git a/dlls/dmusic/buffer.c b/dlls/dmusic/buffer.c index a2ad137fe4b..7f6087fcaaf 100644 --- a/dlls/dmusic/buffer.c +++ b/dlls/dmusic/buffer.c @@ -20,7 +20,6 @@ */ #include "dmusic_private.h" -#include "dmobject.h" #include "initguid.h" #include "dmksctrl.h" diff --git a/dlls/dmusic/clock.c b/dlls/dmusic/clock.c index 7b574dd4432..35df27056fb 100644 --- a/dlls/dmusic/clock.c +++ b/dlls/dmusic/clock.c @@ -19,7 +19,6 @@ */ #include "dmusic_private.h" -#include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index 2d4a7f855ae..acd85768cec 100644 --- a/dlls/dmusic/collection.c +++ b/dlls/dmusic/collection.c @@ -19,7 +19,6 @@ */ #include "dmusic_private.h" -#include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); WINE_DECLARE_DEBUG_CHANNEL(dmfile); @@ -177,6 +176,34 @@ static const IDirectMusicCollectionVtbl collection_vtbl = collection_EnumInstrument, }; +static HRESULT parse_lins_list(struct collection *This, IStream *stream, struct chunk_entry *parent) +{ + struct chunk_entry chunk = {.parent = parent}; + struct instrument_entry *entry; + HRESULT hr; + + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (MAKE_IDTYPE(chunk.id, chunk.type)) + { + case MAKE_IDTYPE(FOURCC_LIST, FOURCC_INS): + if (!(entry = malloc(sizeof(*entry)))) return E_OUTOFMEMORY; + hr = instrument_create_from_chunk(stream, &chunk, &entry->instrument); + if (SUCCEEDED(hr)) list_add_tail(&This->instruments, &entry->entry); + else free(entry); + break; + + default: + FIXME("Ignoring chunk %s %s\n", debugstr_fourcc(chunk.id), debugstr_fourcc(chunk.type)); + break; + } + + if (FAILED(hr)) break; + } + + return hr; +} + static HRESULT WINAPI collection_object_ParseDescriptor(IDirectMusicObject *iface, IStream *stream, DMUS_OBJECTDESC *desc) { @@ -373,39 +400,13 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, break; } case FOURCC_LINS: { + struct chunk_entry lins_chunk = {.id = FOURCC_LIST, .size = chunk.dwSize, .type = chunk.fccID}; TRACE_(dmfile)(": instruments list\n"); - do { - IStream_Read(stream, &chunk, sizeof(FOURCC) + sizeof(DWORD), NULL); - ListCount[0] += sizeof(FOURCC) + sizeof(DWORD) + chunk.dwSize; - TRACE_(dmfile)(": %s chunk (size = %#04lx)", debugstr_fourcc(chunk.fccID), chunk.dwSize); - switch (chunk.fccID) { - case FOURCC_LIST: { - IStream_Read(stream, &chunk.fccID, sizeof(FOURCC), NULL); - TRACE_(dmfile)(": LIST chunk of type %s", debugstr_fourcc(chunk.fccID)); - ListSize[1] = chunk.dwSize - sizeof(FOURCC); - ListCount[1] = 0; - switch (chunk.fccID) { - case FOURCC_INS: { - struct instrument_entry *entry = calloc(1, sizeof(*entry)); - TRACE_(dmfile)(": instrument list\n"); - liMove.QuadPart = -12; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - if (FAILED(instrument_create_from_stream(stream, &entry->instrument))) free(entry); - else list_add_tail(&This->instruments, &entry->entry); - break; - } - } - break; - } - default: { - TRACE_(dmfile)(": unknown chunk (irrelevant & skipping)\n"); - liMove.QuadPart = chunk.dwSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } - } - TRACE_(dmfile)(": ListCount[0] = %ld < ListSize[0] = %ld\n", ListCount[0], ListSize[0]); - } while (ListCount[0] < ListSize[0]); + liMove.QuadPart = 0; + IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &lins_chunk.offset); + lins_chunk.offset.QuadPart -= 12; + parse_lins_list(This, stream, &lins_chunk); + stream_skip_chunk(stream, &lins_chunk); break; } default: { diff --git a/dlls/dmusic/dmusic.c b/dlls/dmusic/dmusic.c index 57a39d1465c..822c35ba616 100644 --- a/dlls/dmusic/dmusic.c +++ b/dlls/dmusic/dmusic.c @@ -22,7 +22,6 @@ #include <stdio.h> #include "dmusic_private.h" -#include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); diff --git a/dlls/dmusic/dmusic_main.c b/dlls/dmusic/dmusic_main.c index f0aaca8947a..52607e1d936 100644 --- a/dlls/dmusic/dmusic_main.c +++ b/dlls/dmusic/dmusic_main.c @@ -35,7 +35,6 @@ #include "dmusici.h" #include "dmusic_private.h" -#include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); diff --git a/dlls/dmusic/dmusic_private.h b/dlls/dmusic/dmusic_private.h index 2ae47217ddf..fe2cbaef9b8 100644 --- a/dlls/dmusic/dmusic_private.h +++ b/dlls/dmusic/dmusic_private.h @@ -41,6 +41,8 @@ #include "dmusics.h" #include "dmksctrl.h" +#include "dmobject.h" + /***************************************************************************** * Interfaces */ @@ -95,7 +97,8 @@ extern HRESULT DMUSIC_CreateReferenceClockImpl (LPCGUID lpcGUID, LPVOID* ppobj, extern HRESULT download_create(DWORD size, IDirectMusicDownload **ret_iface); -extern HRESULT instrument_create_from_stream(IStream *stream, IDirectMusicInstrument **ret_iface); +extern HRESULT instrument_create_from_chunk(IStream *stream, struct chunk_entry *parent, + IDirectMusicInstrument **ret_iface); /***************************************************************************** * IDirectMusic8Impl implementation structure diff --git a/dlls/dmusic/download.c b/dlls/dmusic/download.c index 6dac3e1b725..008e52edbfd 100644 --- a/dlls/dmusic/download.c +++ b/dlls/dmusic/download.c @@ -19,7 +19,6 @@ */ #include "dmusic_private.h" -#include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); diff --git a/dlls/dmusic/instrument.c b/dlls/dmusic/instrument.c index b62b053c542..aacbe6e8c88 100644 --- a/dlls/dmusic/instrument.c +++ b/dlls/dmusic/instrument.c @@ -19,7 +19,6 @@ */ #include "dmusic_private.h" -#include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); @@ -303,9 +302,9 @@ static HRESULT parse_ins_chunk(struct instrument *This, IStream *stream, struct return hr; } -HRESULT instrument_create_from_stream(IStream *stream, IDirectMusicInstrument **ret_iface) +HRESULT instrument_create_from_chunk(IStream *stream, struct chunk_entry *parent, + IDirectMusicInstrument **ret_iface) { - struct chunk_entry chunk = {0}; IDirectMusicInstrument *iface; struct instrument *This; HRESULT hr; @@ -315,23 +314,12 @@ HRESULT instrument_create_from_stream(IStream *stream, IDirectMusicInstrument ** if (FAILED(hr = instrument_create(&iface))) return hr; This = impl_from_IDirectMusicInstrument(iface); - if ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + if (FAILED(hr = parse_ins_chunk(This, stream, parent))) { - switch (MAKE_IDTYPE(chunk.id, chunk.type)) - { - case MAKE_IDTYPE(FOURCC_LIST, FOURCC_INS): - hr = parse_ins_chunk(This, stream, &chunk); - break; - - default: - WARN("Invalid instrument chunk %s %s\n", debugstr_fourcc(chunk.id), debugstr_fourcc(chunk.type)); - hr = E_INVALIDARG; - break; - } + IDirectMusicInstrument_Release(iface); + return DMUS_E_UNSUPPORTED_STREAM; } - if (FAILED(hr)) goto error; - if (TRACE_ON(dmusic)) { TRACE("Created DirectMusicInstrument (%p) ***\n", This); @@ -347,10 +335,4 @@ HRESULT instrument_create_from_stream(IStream *stream, IDirectMusicInstrument ** *ret_iface = iface; return S_OK; - -error: - IDirectMusicInstrument_Release(iface); - - stream_skip_chunk(stream, &chunk); - return DMUS_E_UNSUPPORTED_STREAM; } diff --git a/dlls/dmusic/port.c b/dlls/dmusic/port.c index a5ff9bac16e..675cf05a003 100644 --- a/dlls/dmusic/port.c +++ b/dlls/dmusic/port.c @@ -21,7 +21,6 @@ #include <assert.h> #include "dmusic_private.h" -#include "dmobject.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/collection.c | 82 +++--------------------------------- dlls/dmusic/dmobject.c | 50 +++++++++++++++------- dlls/dmusic/dmobject.h | 2 + dlls/dmusic/dmusic_main.c | 5 --- dlls/dmusic/dmusic_private.h | 2 - 5 files changed, 44 insertions(+), 97 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index acd85768cec..f64e8de258f 100644 --- a/dlls/dmusic/collection.c +++ b/dlls/dmusic/collection.c @@ -250,7 +250,7 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, { struct collection *This = impl_from_IPersistStream(iface); DMUS_PRIVATE_CHUNK chunk; - DWORD StreamSize, StreamCount, ListSize[2], ListCount[2]; + DWORD StreamSize, StreamCount; LARGE_INTEGER liMove; /* used when skipping chunks */ IStream_AddRef(stream); /* add count for later references */ @@ -314,83 +314,15 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, case FOURCC_LIST: { IStream_Read(stream, &chunk.fccID, sizeof(FOURCC), NULL); TRACE_(dmfile)(": LIST chunk of type %s", debugstr_fourcc(chunk.fccID)); - ListSize[0] = chunk.dwSize - sizeof(FOURCC); - ListCount[0] = 0; switch (chunk.fccID) { case DMUS_FOURCC_INFO_LIST: { + struct chunk_entry info_chunk = {.id = FOURCC_LIST, .size = chunk.dwSize, .type = chunk.fccID}; TRACE_(dmfile)(": INFO list\n"); - do { - IStream_Read(stream, &chunk, sizeof(FOURCC) + sizeof(DWORD), NULL); - ListCount[0] += sizeof(FOURCC) + sizeof(DWORD) + chunk.dwSize; - TRACE_(dmfile)(": %s chunk (size = %#04lx)", debugstr_fourcc(chunk.fccID), chunk.dwSize); - switch (chunk.fccID) { - case mmioFOURCC('I','N','A','M'): { - CHAR szName[DMUS_MAX_NAME]; - TRACE_(dmfile)(": name chunk\n"); - This->dmobj.desc.dwValidData |= DMUS_OBJ_NAME; - IStream_Read(stream, szName, chunk.dwSize, NULL); - MultiByteToWideChar(CP_ACP, 0, szName, -1, This->dmobj.desc.wszName, DMUS_MAX_NAME); - if (even_or_odd(chunk.dwSize)) { - ListCount[0]++; - liMove.QuadPart = 1; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - } - break; - } - case mmioFOURCC('I','A','R','T'): { - TRACE_(dmfile)(": artist chunk (ignored)\n"); - if (even_or_odd(chunk.dwSize)) { - ListCount[0]++; - chunk.dwSize++; - } - liMove.QuadPart = chunk.dwSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } - case mmioFOURCC('I','C','O','P'): { - TRACE_(dmfile)(": copyright chunk\n"); - This->szCopyright = calloc(1, chunk.dwSize); - IStream_Read(stream, This->szCopyright, chunk.dwSize, NULL); - if (even_or_odd(chunk.dwSize)) { - ListCount[0]++; - liMove.QuadPart = 1; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - } - break; - } - case mmioFOURCC('I','S','B','J'): { - TRACE_(dmfile)(": subject chunk (ignored)\n"); - if (even_or_odd(chunk.dwSize)) { - ListCount[0]++; - chunk.dwSize++; - } - liMove.QuadPart = chunk.dwSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } - case mmioFOURCC('I','C','M','T'): { - TRACE_(dmfile)(": comment chunk (ignored)\n"); - if (even_or_odd(chunk.dwSize)) { - ListCount[0]++; - chunk.dwSize++; - } - liMove.QuadPart = chunk.dwSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } - default: { - TRACE_(dmfile)(": unknown chunk (irrelevant & skipping)\n"); - if (even_or_odd(chunk.dwSize)) { - ListCount[0]++; - chunk.dwSize++; - } - liMove.QuadPart = chunk.dwSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } - } - TRACE_(dmfile)(": ListCount[0] = %ld < ListSize[0] = %ld\n", ListCount[0], ListSize[0]); - } while (ListCount[0] < ListSize[0]); + liMove.QuadPart = 0; + IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &info_chunk.offset); + info_chunk.offset.QuadPart -= 12; + stream_chunk_parse_info_list(stream, &info_chunk, &This->dmobj.desc); + stream_skip_chunk(stream, &info_chunk); break; } case FOURCC_WVPL: { diff --git a/dlls/dmusic/dmobject.c b/dlls/dmusic/dmobject.c index 2bef5511851..5bf856e2a04 100644 --- a/dlls/dmusic/dmobject.c +++ b/dlls/dmusic/dmobject.c @@ -528,24 +528,44 @@ HRESULT WINAPI dmobj_IDirectMusicObject_SetDescriptor(IDirectMusicObject *iface, return ret; } -/* Helper for IDirectMusicObject::ParseDescriptor */ -static inline void info_get_name(IStream *stream, const struct chunk_entry *info, - DMUS_OBJECTDESC *desc) +HRESULT stream_chunk_parse_info_list(IStream *stream, const struct chunk_entry *parent, DMUS_OBJECTDESC *desc) { - struct chunk_entry chunk = {.parent = info}; - char name[DMUS_MAX_NAME]; - ULONG len; - HRESULT hr = E_FAIL; + struct chunk_entry chunk = {.parent = parent}; + char *buffer; + HRESULT hr; - while (stream_next_chunk(stream, &chunk) == S_OK) - if (chunk.id == mmioFOURCC('I','N','A','M')) - hr = IStream_Read(stream, name, min(chunk.size, sizeof(name)), &len); + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (MAKE_IDTYPE(chunk.id, chunk.type)) + { + case mmioFOURCC('I','N','A','M'): + if (desc->dwValidData & DMUS_OBJ_NAME) continue; + if (!(buffer = malloc(chunk.size))) return E_OUTOFMEMORY; + if ((hr = stream_chunk_get_data(stream, &chunk, buffer, chunk.size)) == S_OK) + { + DWORD len = MultiByteToWideChar(CP_ACP, 0, buffer, chunk.size, + desc->wszName, ARRAY_SIZE(desc->wszName)); + desc->wszName[min(len, sizeof(desc->wszName) - 1)] = 0; + desc->dwValidData |= DMUS_OBJ_NAME; + } + free(buffer); + break; + + case mmioFOURCC('I','C','O','P'): + case mmioFOURCC('I','E','N','G'): + case mmioFOURCC('I','S','B','J'): + case mmioFOURCC('I','S','F','T'): + break; + + default: + FIXME("Ignoring chunk %s %s\n", debugstr_fourcc(chunk.id), debugstr_fourcc(chunk.type)); + break; + } - if (SUCCEEDED(hr)) { - len = MultiByteToWideChar(CP_ACP, 0, name, len, desc->wszName, sizeof(desc->wszName)); - desc->wszName[min(len, sizeof(desc->wszName) - 1)] = 0; - desc->dwValidData |= DMUS_OBJ_NAME; + if (FAILED(hr)) break; } + + return hr; } static inline void unfo_get_name(IStream *stream, const struct chunk_entry *unfo, @@ -606,7 +626,7 @@ HRESULT dmobj_parsedescriptor(IStream *stream, const struct chunk_entry *riff, if (chunk.type == DMUS_FOURCC_UNFO_LIST && (supported & DMUS_OBJ_NAME)) unfo_get_name(stream, &chunk, desc, supported & DMUS_OBJ_NAME_INAM); else if (chunk.type == DMUS_FOURCC_INFO_LIST && (supported & DMUS_OBJ_NAME_INFO)) - info_get_name(stream, &chunk, desc); + stream_chunk_parse_info_list(stream, &chunk, desc); break; } } diff --git a/dlls/dmusic/dmobject.h b/dlls/dmusic/dmobject.h index 98069de8cc4..8aed294c43c 100644 --- a/dlls/dmusic/dmobject.h +++ b/dlls/dmusic/dmobject.h @@ -44,6 +44,8 @@ HRESULT stream_chunk_get_data(IStream *stream, const struct chunk_entry *chunk, HRESULT stream_chunk_get_wstr(IStream *stream, const struct chunk_entry *chunk, WCHAR *str, ULONG size); +HRESULT stream_chunk_parse_info_list(IStream *stream, const struct chunk_entry *parent, DMUS_OBJECTDESC *desc); + static inline HRESULT stream_reset_chunk_data(IStream *stream, const struct chunk_entry *chunk) { LARGE_INTEGER offset; diff --git a/dlls/dmusic/dmusic_main.c b/dlls/dmusic/dmusic_main.c index 52607e1d936..eabf9f131b9 100644 --- a/dlls/dmusic/dmusic_main.c +++ b/dlls/dmusic/dmusic_main.c @@ -165,11 +165,6 @@ void Patch2MIDILOCALE (DWORD dwPatch, LPMIDILOCALE pLocale) { pLocale->ulBank |= (dwPatch & F_INSTRUMENT_DRUMS); /* get drum bit */ } -/* check whether the given DWORD is even (return 0) or odd (return 1) */ -int even_or_odd (DWORD number) { - return (number & 0x1); /* basically, check if bit 0 is set ;) */ -} - /* generic flag-dumping function */ static const char* debugstr_flags (DWORD flags, const flag_info* names, size_t num_names){ char buffer[128] = "", *ptr = &buffer[0]; diff --git a/dlls/dmusic/dmusic_private.h b/dlls/dmusic/dmusic_private.h index fe2cbaef9b8..bb5c8ce44c6 100644 --- a/dlls/dmusic/dmusic_private.h +++ b/dlls/dmusic/dmusic_private.h @@ -211,8 +211,6 @@ extern DWORD MIDILOCALE2Patch (const MIDILOCALE *pLocale); /* MIDILOCALE from dwPatch */ extern void Patch2MIDILOCALE (DWORD dwPatch, LPMIDILOCALE pLocale); -/* check whether the given DWORD is even (return 0) or odd (return 1) */ -extern int even_or_odd (DWORD number); /* Dump whole DMUS_PORTPARAMS struct */ extern void dump_DMUS_PORTPARAMS(LPDMUS_PORTPARAMS params); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/collection.c | 46 +++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index f64e8de258f..3b3350780bc 100644 --- a/dlls/dmusic/collection.c +++ b/dlls/dmusic/collection.c @@ -29,6 +29,14 @@ struct instrument_entry IDirectMusicInstrument *instrument; }; +struct pool +{ + POOLTABLE table; + POOLCUE cues[]; +}; + +C_ASSERT(sizeof(struct pool) == offsetof(struct pool, cues[0])); + struct collection { IDirectMusicCollection IDirectMusicCollection_iface; @@ -38,10 +46,8 @@ struct collection IStream *pStm; /* stream from which we load collection and later instruments */ CHAR *szCopyright; /* FIXME: should probably be placed somewhere else */ DLSHEADER *pHeader; - /* pool table */ - POOLTABLE *pPoolTable; - POOLCUE *pPoolCues; + struct pool *pool; struct list instruments; }; @@ -204,6 +210,29 @@ static HRESULT parse_lins_list(struct collection *This, IStream *stream, struct return hr; } +static HRESULT parse_ptbl_chunk(struct collection *This, IStream *stream, struct chunk_entry *chunk) +{ + struct pool *pool; + POOLTABLE table; + HRESULT hr; + UINT size; + + if (chunk->size < sizeof(table)) return E_INVALIDARG; + if (FAILED(hr = stream_read(stream, &table, sizeof(table)))) return hr; + if (chunk->size != table.cbSize + sizeof(POOLCUE) * table.cCues) return E_INVALIDARG; + if (table.cbSize != sizeof(table)) return E_INVALIDARG; + + size = offsetof(struct pool, cues[table.cCues]); + if (!(pool = malloc(size))) return E_OUTOFMEMORY; + pool->table = table; + + size = sizeof(POOLCUE) * table.cCues; + if (FAILED(hr = stream_read(stream, pool->cues, size))) free(pool); + else This->pool = pool; + + return hr; +} + static HRESULT WINAPI collection_object_ParseDescriptor(IDirectMusicObject *iface, IStream *stream, DMUS_OBJECTDESC *desc) { @@ -303,12 +332,13 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, break; } case FOURCC_PTBL: { + struct chunk_entry ptbl_chunk = {.id = FOURCC_LIST, .size = chunk.dwSize, .type = chunk.fccID}; TRACE_(dmfile)(": pool table chunk\n"); - This->pPoolTable = calloc(1, sizeof(POOLTABLE)); - IStream_Read(stream, This->pPoolTable, sizeof(POOLTABLE), NULL); - chunk.dwSize -= sizeof(POOLTABLE); - This->pPoolCues = calloc(This->pPoolTable->cCues, sizeof(POOLCUE)); - IStream_Read(stream, This->pPoolCues, chunk.dwSize, NULL); + liMove.QuadPart = 0; + IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &ptbl_chunk.offset); + ptbl_chunk.offset.QuadPart -= 12; + parse_ptbl_chunk(This, stream, &ptbl_chunk); + stream_skip_chunk(stream, &ptbl_chunk); break; } case FOURCC_LIST: { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/collection.c | 18 +++++++++++++----- dlls/dmusic/dmobject.c | 33 +++++++++++++++++++++++++++++++-- dlls/dmusic/dmobject.h | 2 +- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index 3b3350780bc..a284bad0a81 100644 --- a/dlls/dmusic/collection.c +++ b/dlls/dmusic/collection.c @@ -320,15 +320,23 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, break; } case FOURCC_DLID: { + struct chunk_entry info_chunk = {.id = chunk.fccID, .size = chunk.dwSize}; TRACE_(dmfile)(": DLID (GUID) chunk\n"); - This->dmobj.desc.dwValidData |= DMUS_OBJ_OBJECT; - IStream_Read(stream, &This->dmobj.desc.guidObject, chunk.dwSize, NULL); + liMove.QuadPart = 0; + IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &info_chunk.offset); + info_chunk.offset.QuadPart -= 12; + stream_chunk_parse_desc(stream, &info_chunk, &This->dmobj.desc); + stream_skip_chunk(stream, &info_chunk); break; } case FOURCC_VERS: { + struct chunk_entry info_chunk = {.id = chunk.fccID, .size = chunk.dwSize}; TRACE_(dmfile)(": version chunk\n"); - This->dmobj.desc.dwValidData |= DMUS_OBJ_VERSION; - IStream_Read(stream, &This->dmobj.desc.vVersion, chunk.dwSize, NULL); + liMove.QuadPart = 0; + IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &info_chunk.offset); + info_chunk.offset.QuadPart -= 12; + stream_chunk_parse_desc(stream, &info_chunk, &This->dmobj.desc); + stream_skip_chunk(stream, &info_chunk); break; } case FOURCC_PTBL: { @@ -351,7 +359,7 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, liMove.QuadPart = 0; IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &info_chunk.offset); info_chunk.offset.QuadPart -= 12; - stream_chunk_parse_info_list(stream, &info_chunk, &This->dmobj.desc); + stream_chunk_parse_desc(stream, &info_chunk, &This->dmobj.desc); stream_skip_chunk(stream, &info_chunk); break; } diff --git a/dlls/dmusic/dmobject.c b/dlls/dmusic/dmobject.c index 5bf856e2a04..cac54cfaa33 100644 --- a/dlls/dmusic/dmobject.c +++ b/dlls/dmusic/dmobject.c @@ -528,7 +528,7 @@ HRESULT WINAPI dmobj_IDirectMusicObject_SetDescriptor(IDirectMusicObject *iface, return ret; } -HRESULT stream_chunk_parse_info_list(IStream *stream, const struct chunk_entry *parent, DMUS_OBJECTDESC *desc) +static HRESULT parse_info_list(IStream *stream, const struct chunk_entry *parent, DMUS_OBJECTDESC *desc) { struct chunk_entry chunk = {.parent = parent}; char *buffer; @@ -579,6 +579,35 @@ static inline void unfo_get_name(IStream *stream, const struct chunk_entry *unfo desc->dwValidData |= DMUS_OBJ_NAME; } +HRESULT stream_chunk_parse_desc(IStream *stream, const struct chunk_entry *chunk, DMUS_OBJECTDESC *desc) +{ + HRESULT hr; + + switch (MAKE_IDTYPE(chunk->id, chunk->type)) + { + case FOURCC_DLID: + if (desc->dwValidData & DMUS_OBJ_OBJECT) return S_OK; + hr = stream_chunk_get_data(stream, chunk, &desc->guidObject, sizeof(desc->guidObject)); + if (hr == S_OK) desc->dwValidData |= DMUS_OBJ_OBJECT; + return hr; + + case FOURCC_VERS: + if (desc->dwValidData & DMUS_OBJ_VERSION) return S_OK; + hr = stream_chunk_get_data(stream, chunk, &desc->vVersion, sizeof(desc->vVersion)); + if (hr == S_OK) desc->dwValidData |= DMUS_OBJ_VERSION; + return hr; + + case MAKE_IDTYPE(FOURCC_LIST, DMUS_FOURCC_INFO_LIST): + return parse_info_list(stream, chunk, desc); + + default: + FIXME("Unsupported chunk %s %s\n", debugstr_fourcc(chunk->id), debugstr_fourcc(chunk->type)); + break; + } + + return E_NOTIMPL; +} + HRESULT dmobj_parsedescriptor(IStream *stream, const struct chunk_entry *riff, DMUS_OBJECTDESC *desc, DWORD supported) { @@ -626,7 +655,7 @@ HRESULT dmobj_parsedescriptor(IStream *stream, const struct chunk_entry *riff, if (chunk.type == DMUS_FOURCC_UNFO_LIST && (supported & DMUS_OBJ_NAME)) unfo_get_name(stream, &chunk, desc, supported & DMUS_OBJ_NAME_INAM); else if (chunk.type == DMUS_FOURCC_INFO_LIST && (supported & DMUS_OBJ_NAME_INFO)) - stream_chunk_parse_info_list(stream, &chunk, desc); + parse_info_list(stream, &chunk, desc); break; } } diff --git a/dlls/dmusic/dmobject.h b/dlls/dmusic/dmobject.h index 8aed294c43c..ecd9d5401fb 100644 --- a/dlls/dmusic/dmobject.h +++ b/dlls/dmusic/dmobject.h @@ -44,7 +44,7 @@ HRESULT stream_chunk_get_data(IStream *stream, const struct chunk_entry *chunk, HRESULT stream_chunk_get_wstr(IStream *stream, const struct chunk_entry *chunk, WCHAR *str, ULONG size); -HRESULT stream_chunk_parse_info_list(IStream *stream, const struct chunk_entry *parent, DMUS_OBJECTDESC *desc); +extern HRESULT stream_chunk_parse_desc(IStream *stream, const struct chunk_entry *chunk, DMUS_OBJECTDESC *desc); static inline HRESULT stream_reset_chunk_data(IStream *stream, const struct chunk_entry *chunk) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/collection.c | 184 +++++++++++------------------------ dlls/dmusic/dmusic_private.h | 6 -- 2 files changed, 58 insertions(+), 132 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index a284bad0a81..c3093d10a77 100644 --- a/dlls/dmusic/collection.c +++ b/dlls/dmusic/collection.c @@ -21,7 +21,6 @@ #include "dmusic_private.h" WINE_DEFAULT_DEBUG_CHANNEL(dmusic); -WINE_DECLARE_DEBUG_CHANNEL(dmfile); struct instrument_entry { @@ -43,9 +42,8 @@ struct collection struct dmobject dmobj; LONG ref; - IStream *pStm; /* stream from which we load collection and later instruments */ CHAR *szCopyright; /* FIXME: should probably be placed somewhere else */ - DLSHEADER *pHeader; + DLSHEADER header; struct pool *pool; struct list instruments; @@ -233,6 +231,44 @@ static HRESULT parse_ptbl_chunk(struct collection *This, IStream *stream, struct return hr; } +static HRESULT parse_dls_chunk(struct collection *This, IStream *stream, struct chunk_entry *parent) +{ + struct chunk_entry chunk = {.parent = parent}; + HRESULT hr; + + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (MAKE_IDTYPE(chunk.id, chunk.type)) + { + case FOURCC_DLID: + case FOURCC_VERS: + case MAKE_IDTYPE(FOURCC_LIST, DMUS_FOURCC_INFO_LIST): + hr = stream_chunk_parse_desc(stream, &chunk, &This->dmobj.desc); + break; + + case FOURCC_COLH: + hr = stream_chunk_get_data(stream, &chunk, &This->header, sizeof(This->header)); + break; + + case FOURCC_PTBL: + hr = parse_ptbl_chunk(This, stream, &chunk); + break; + + case MAKE_IDTYPE(FOURCC_LIST, FOURCC_LINS): + hr = parse_lins_list(This, stream, &chunk); + break; + + default: + FIXME("Ignoring chunk %s %s\n", debugstr_fourcc(chunk.id), debugstr_fourcc(chunk.type)); + break; + } + + if (FAILED(hr)) break; + } + + return hr; +} + static HRESULT WINAPI collection_object_ParseDescriptor(IDirectMusicObject *iface, IStream *stream, DMUS_OBJECTDESC *desc) { @@ -274,135 +310,30 @@ static const IDirectMusicObjectVtbl collection_object_vtbl = collection_object_ParseDescriptor, }; -static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, - IStream *stream) +static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, IStream *stream) { struct collection *This = impl_from_IPersistStream(iface); - DMUS_PRIVATE_CHUNK chunk; - DWORD StreamSize, StreamCount; - LARGE_INTEGER liMove; /* used when skipping chunks */ - - IStream_AddRef(stream); /* add count for later references */ - This->pStm = stream; - - IStream_Read(stream, &chunk, sizeof(FOURCC) + sizeof(DWORD), NULL); - TRACE_(dmfile)(": %s chunk (size = %#04lx)", debugstr_fourcc(chunk.fccID), chunk.dwSize); - - if (chunk.fccID != FOURCC_RIFF) { - TRACE_(dmfile)(": unexpected chunk; loading failed)\n"); - liMove.QuadPart = chunk.dwSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); /* skip the rest of the chunk */ - return E_FAIL; - } + struct chunk_entry chunk = {0}; + HRESULT hr; - IStream_Read(stream, &chunk.fccID, sizeof(FOURCC), NULL); - TRACE_(dmfile)(": RIFF chunk of type %s", debugstr_fourcc(chunk.fccID)); - StreamSize = chunk.dwSize - sizeof(FOURCC); - StreamCount = 0; + TRACE("(%p, %p)\n", This, stream); - if (chunk.fccID != FOURCC_DLS) { - TRACE_(dmfile)(": unexpected chunk; loading failed)\n"); - liMove.QuadPart = StreamSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); /* skip the rest of the chunk */ - return E_FAIL; - } + if ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (MAKE_IDTYPE(chunk.id, chunk.type)) + { + case MAKE_IDTYPE(FOURCC_RIFF, FOURCC_DLS): + hr = parse_dls_chunk(This, stream, &chunk); + break; - TRACE_(dmfile)(": collection form\n"); - do { - IStream_Read(stream, &chunk, sizeof(FOURCC) + sizeof(DWORD), NULL); - StreamCount += sizeof(FOURCC) + sizeof(DWORD) + chunk.dwSize; - TRACE_(dmfile)(": %s chunk (size = %#04lx)", debugstr_fourcc(chunk.fccID), chunk.dwSize); - switch (chunk.fccID) { - case FOURCC_COLH: { - TRACE_(dmfile)(": collection header chunk\n"); - This->pHeader = calloc(1, chunk.dwSize); - IStream_Read(stream, This->pHeader, chunk.dwSize, NULL); - break; - } - case FOURCC_DLID: { - struct chunk_entry info_chunk = {.id = chunk.fccID, .size = chunk.dwSize}; - TRACE_(dmfile)(": DLID (GUID) chunk\n"); - liMove.QuadPart = 0; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &info_chunk.offset); - info_chunk.offset.QuadPart -= 12; - stream_chunk_parse_desc(stream, &info_chunk, &This->dmobj.desc); - stream_skip_chunk(stream, &info_chunk); - break; - } - case FOURCC_VERS: { - struct chunk_entry info_chunk = {.id = chunk.fccID, .size = chunk.dwSize}; - TRACE_(dmfile)(": version chunk\n"); - liMove.QuadPart = 0; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &info_chunk.offset); - info_chunk.offset.QuadPart -= 12; - stream_chunk_parse_desc(stream, &info_chunk, &This->dmobj.desc); - stream_skip_chunk(stream, &info_chunk); - break; - } - case FOURCC_PTBL: { - struct chunk_entry ptbl_chunk = {.id = FOURCC_LIST, .size = chunk.dwSize, .type = chunk.fccID}; - TRACE_(dmfile)(": pool table chunk\n"); - liMove.QuadPart = 0; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &ptbl_chunk.offset); - ptbl_chunk.offset.QuadPart -= 12; - parse_ptbl_chunk(This, stream, &ptbl_chunk); - stream_skip_chunk(stream, &ptbl_chunk); - break; - } - case FOURCC_LIST: { - IStream_Read(stream, &chunk.fccID, sizeof(FOURCC), NULL); - TRACE_(dmfile)(": LIST chunk of type %s", debugstr_fourcc(chunk.fccID)); - switch (chunk.fccID) { - case DMUS_FOURCC_INFO_LIST: { - struct chunk_entry info_chunk = {.id = FOURCC_LIST, .size = chunk.dwSize, .type = chunk.fccID}; - TRACE_(dmfile)(": INFO list\n"); - liMove.QuadPart = 0; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &info_chunk.offset); - info_chunk.offset.QuadPart -= 12; - stream_chunk_parse_desc(stream, &info_chunk, &This->dmobj.desc); - stream_skip_chunk(stream, &info_chunk); - break; - } - case FOURCC_WVPL: { - TRACE_(dmfile)(": wave pool list (mark & skip)\n"); - liMove.QuadPart = chunk.dwSize - sizeof(FOURCC); - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } - case FOURCC_LINS: { - struct chunk_entry lins_chunk = {.id = FOURCC_LIST, .size = chunk.dwSize, .type = chunk.fccID}; - TRACE_(dmfile)(": instruments list\n"); - liMove.QuadPart = 0; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &lins_chunk.offset); - lins_chunk.offset.QuadPart -= 12; - parse_lins_list(This, stream, &lins_chunk); - stream_skip_chunk(stream, &lins_chunk); - break; - } - default: { - TRACE_(dmfile)(": unknown (skipping)\n"); - liMove.QuadPart = chunk.dwSize - sizeof(FOURCC); - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } - } - break; - } - default: { - TRACE_(dmfile)(": unknown chunk (irrelevant & skipping)\n"); - liMove.QuadPart = chunk.dwSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); - break; - } + default: + WARN("Invalid collection chunk %s %s\n", debugstr_fourcc(chunk.id), debugstr_fourcc(chunk.type)); + hr = DMUS_E_UNSUPPORTED_STREAM; + break; } - TRACE_(dmfile)(": StreamCount = %ld < StreamSize = %ld\n", StreamCount, StreamSize); - } while (StreamCount < StreamSize); - - TRACE_(dmfile)(": reading finished\n"); - + } - /* DEBUG: dumps whole collection object tree: */ - if (TRACE_ON(dmusic)) + if (SUCCEEDED(hr) && TRACE_ON(dmusic)) { struct instrument_entry *entry; int i = 0; @@ -411,13 +342,14 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, dump_DMUS_OBJECTDESC(&This->dmobj.desc); TRACE(" - Collection header:\n"); - TRACE(" - cInstruments: %ld\n", This->pHeader->cInstruments); + TRACE(" - cInstruments: %ld\n", This->header.cInstruments); TRACE(" - Instruments:\n"); LIST_FOR_EACH_ENTRY(entry, &This->instruments, struct instrument_entry, entry) TRACE(" - Instrument[%i]: %p\n", i++, entry->instrument); } + stream_skip_chunk(stream, &chunk); return S_OK; } diff --git a/dlls/dmusic/dmusic_private.h b/dlls/dmusic/dmusic_private.h index bb5c8ce44c6..d200d3f0e15 100644 --- a/dlls/dmusic/dmusic_private.h +++ b/dlls/dmusic/dmusic_private.h @@ -192,12 +192,6 @@ static inline struct instrument *impl_from_IDirectMusicInstrument(IDirectMusicIn */ void dmusic_remove_port(IDirectMusic8Impl *dmusic, IDirectMusicPort *port); -/* for simpler reading */ -typedef struct _DMUS_PRIVATE_CHUNK { - FOURCC fccID; /* FOURCC ID of the chunk */ - DWORD dwSize; /* size of the chunk */ -} DMUS_PRIVATE_CHUNK, *LPDMUS_PRIVATE_CHUNK; - /* used for generic dumping (copied from ddraw) */ typedef struct { DWORD val; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/collection.c | 11 +++-------- dlls/dmusic/dmusic_private.h | 5 +---- dlls/dmusic/instrument.c | 13 +++++++------ 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index c3093d10a77..aaeb62e2cd9 100644 --- a/dlls/dmusic/collection.c +++ b/dlls/dmusic/collection.c @@ -25,6 +25,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(dmusic); struct instrument_entry { struct list entry; + DMUS_OBJECTDESC desc; IDirectMusicInstrument *instrument; }; @@ -158,13 +159,7 @@ static HRESULT WINAPI collection_EnumInstrument(IDirectMusicCollection *iface, { if (index--) continue; if (FAILED(hr = IDirectMusicInstrument_GetPatch(entry->instrument, patch))) return hr; - if (name) - { - struct instrument *instrument = impl_from_IDirectMusicInstrument(entry->instrument); - DWORD length = min(lstrlenW(instrument->wszName), name_length - 1); - memcpy(name, instrument->wszName, length * sizeof(WCHAR)); - name[length] = '\0'; - } + if (name) lstrcpynW(name, entry->desc.wszName, name_length); return S_OK; } @@ -192,7 +187,7 @@ static HRESULT parse_lins_list(struct collection *This, IStream *stream, struct { case MAKE_IDTYPE(FOURCC_LIST, FOURCC_INS): if (!(entry = malloc(sizeof(*entry)))) return E_OUTOFMEMORY; - hr = instrument_create_from_chunk(stream, &chunk, &entry->instrument); + hr = instrument_create_from_chunk(stream, &chunk, &entry->desc, &entry->instrument); if (SUCCEEDED(hr)) list_add_tail(&This->instruments, &entry->entry); else free(entry); break; diff --git a/dlls/dmusic/dmusic_private.h b/dlls/dmusic/dmusic_private.h index d200d3f0e15..31ff63a954d 100644 --- a/dlls/dmusic/dmusic_private.h +++ b/dlls/dmusic/dmusic_private.h @@ -98,7 +98,7 @@ extern HRESULT DMUSIC_CreateReferenceClockImpl (LPCGUID lpcGUID, LPVOID* ppobj, extern HRESULT download_create(DWORD size, IDirectMusicDownload **ret_iface); extern HRESULT instrument_create_from_chunk(IStream *stream, struct chunk_entry *parent, - IDirectMusicInstrument **ret_iface); + DMUS_OBJECTDESC *desc, IDirectMusicInstrument **ret_iface); /***************************************************************************** * IDirectMusic8Impl implementation structure @@ -173,10 +173,7 @@ struct instrument IDirectMusicInstrument IDirectMusicInstrument_iface; LONG ref; - GUID id; INSTHEADER header; - WCHAR wszName[DMUS_MAX_NAME]; - /* instrument data */ struct list articulations; struct list regions; diff --git a/dlls/dmusic/instrument.c b/dlls/dmusic/instrument.c index aacbe6e8c88..761c2e22822 100644 --- a/dlls/dmusic/instrument.c +++ b/dlls/dmusic/instrument.c @@ -266,7 +266,8 @@ static HRESULT parse_lrgn_list(struct instrument *This, IStream *stream, struct return hr; } -static HRESULT parse_ins_chunk(struct instrument *This, IStream *stream, struct chunk_entry *parent) +static HRESULT parse_ins_chunk(struct instrument *This, IStream *stream, struct chunk_entry *parent, + DMUS_OBJECTDESC *desc) { struct chunk_entry chunk = {.parent = parent}; HRESULT hr; @@ -280,7 +281,8 @@ static HRESULT parse_ins_chunk(struct instrument *This, IStream *stream, struct break; case FOURCC_DLID: - hr = stream_chunk_get_data(stream, &chunk, &This->id, sizeof(This->id)); + case MAKE_IDTYPE(FOURCC_LIST, DMUS_FOURCC_INFO_LIST): + hr = stream_chunk_parse_desc(stream, &chunk, desc); break; case MAKE_IDTYPE(FOURCC_LIST, FOURCC_LRGN): @@ -303,7 +305,7 @@ static HRESULT parse_ins_chunk(struct instrument *This, IStream *stream, struct } HRESULT instrument_create_from_chunk(IStream *stream, struct chunk_entry *parent, - IDirectMusicInstrument **ret_iface) + DMUS_OBJECTDESC *desc, IDirectMusicInstrument **ret_iface) { IDirectMusicInstrument *iface; struct instrument *This; @@ -314,7 +316,7 @@ HRESULT instrument_create_from_chunk(IStream *stream, struct chunk_entry *parent if (FAILED(hr = instrument_create(&iface))) return hr; This = impl_from_IDirectMusicInstrument(iface); - if (FAILED(hr = parse_ins_chunk(This, stream, parent))) + if (FAILED(hr = parse_ins_chunk(This, stream, parent, desc))) { IDirectMusicInstrument_Release(iface); return DMUS_E_UNSUPPORTED_STREAM; @@ -323,14 +325,13 @@ HRESULT instrument_create_from_chunk(IStream *stream, struct chunk_entry *parent if (TRACE_ON(dmusic)) { TRACE("Created DirectMusicInstrument (%p) ***\n", This); - if (!IsEqualGUID(&This->id, &GUID_NULL)) - TRACE(" - GUID = %s\n", debugstr_dmguid(&This->id)); TRACE(" - Instrument header:\n"); TRACE(" - cRegions: %ld\n", This->header.cRegions); TRACE(" - Locale:\n"); TRACE(" - ulBank: %ld\n", This->header.Locale.ulBank); TRACE(" - ulInstrument: %ld\n", This->header.Locale.ulInstrument); TRACE(" => dwPatch: %ld\n", MIDILOCALE2Patch(&This->header.Locale)); + if (desc->dwValidData & DMUS_OBJ_OBJECT) TRACE(" - guid: %s\n", debugstr_dmguid(&desc->guidObject)); } *ret_iface = iface; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/instrument.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/dlls/dmusic/instrument.c b/dlls/dmusic/instrument.c index 761c2e22822..9022efa4a98 100644 --- a/dlls/dmusic/instrument.c +++ b/dlls/dmusic/instrument.c @@ -324,14 +324,38 @@ HRESULT instrument_create_from_chunk(IStream *stream, struct chunk_entry *parent if (TRACE_ON(dmusic)) { - TRACE("Created DirectMusicInstrument (%p) ***\n", This); - TRACE(" - Instrument header:\n"); - TRACE(" - cRegions: %ld\n", This->header.cRegions); - TRACE(" - Locale:\n"); - TRACE(" - ulBank: %ld\n", This->header.Locale.ulBank); - TRACE(" - ulInstrument: %ld\n", This->header.Locale.ulInstrument); - TRACE(" => dwPatch: %ld\n", MIDILOCALE2Patch(&This->header.Locale)); + struct region *region; + UINT i; + + TRACE("Created DirectMusicInstrument %p\n", This); + TRACE(" - header:\n"); + TRACE(" - regions: %ld\n", This->header.cRegions); + TRACE(" - locale: {bank: %#lx, instrument: %#lx} (patch %#lx)\n", + This->header.Locale.ulBank, This->header.Locale.ulInstrument, + MIDILOCALE2Patch(&This->header.Locale)); if (desc->dwValidData & DMUS_OBJ_OBJECT) TRACE(" - guid: %s\n", debugstr_dmguid(&desc->guidObject)); + if (desc->dwValidData & DMUS_OBJ_NAME) TRACE(" - name: %s\n", debugstr_w(desc->wszName)); + + TRACE(" - regions:\n"); + LIST_FOR_EACH_ENTRY(region, &This->regions, struct region, entry) + { + TRACE(" - region:\n"); + TRACE(" - header: {key: %u - %u, vel: %u - %u, options: %#x, group: %#x}\n", + region->header.RangeKey.usLow, region->header.RangeKey.usHigh, + region->header.RangeVelocity.usLow, region->header.RangeVelocity.usHigh, + region->header.fusOptions, region->header.usKeyGroup); + TRACE(" - wave_link: {options: %#x, group: %u, channel: %lu, index: %lu}\n", + region->wave_link.fusOptions, region->wave_link.usPhaseGroup, + region->wave_link.ulChannel, region->wave_link.ulTableIndex); + TRACE(" - wave_sample: {size: %lu, unity_note: %u, fine_tune: %d, attenuation: %ld, options: %#lx, loops: %lu}\n", + region->wave_sample.cbSize, region->wave_sample.usUnityNote, + region->wave_sample.sFineTune, region->wave_sample.lAttenuation, + region->wave_sample.fulOptions, region->wave_sample.cSampleLoops); + for (i = 0; i < region->wave_sample.cSampleLoops; i++) + TRACE(" - wave_loop[%u]: {size: %lu, type: %lu, start: %lu, length: %lu}\n", i, + region->wave_loop.cbSize, region->wave_loop.ulType, + region->wave_loop.ulStart, region->wave_loop.ulLength); + } } *ret_iface = iface; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
v2: Remove the WARN for knowingly ignored chunks, avoid parsing again duplicate chunks as some to-be-sent-later tests suggest. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3795#note_45200
This would let us parse streams in single pass instead of rewinding the stream when parsing the descriptor chunks
Is that an issue? Rewinding the stream isn't expensive. Stream cloning is also an option.\ It isn't beautiful but it works. The descriptor is independent of the rest of the data.\ And IMHO our DMusic implementation has bigger fish to fry.
imho will better fit inside the parsing loops.
Actually I was trying to get away from parsing loops for the most part. Chunk order is important and an error must be thrown if e.g. a header chunk is not the first chunk, see https://learn.microsoft.com/en-us/previous-versions/ms810875(v=msdn.10)\ Also some stuff should appear only once while others can appear multiple times.\ I was more thinking of going for an "expect type for next chunk and get its data" helper but didn't follow through as it didn't seem simpler.
although I wanted to make sure that it's doable for every object.
We have extensive ParseDescriptor tests for each object implementing IDirectMusicObject.\ Also using the DLS Collection for a new generic DMusic descriptor parsing is suboptimal.\ DLS predates DMusic and isn't a DMusic RIFF file format so it makes it unsuitable to show the merits of the different implementation.\ https://learn.microsoft.com/en-us/previous-versions/ms807157(v=msdn.10)?redirectedfrom=MSDN\ So it is fine if DLS collection gets its own oddball implementation to deal with 'diid' instead of 'guid'.
Both would use the same supported flag and would require a separate, private, flag to distinguish them.
Well we already have to use a private flag DMUS_OBJ_NAME_INFO to distinguish between 'INAM' and 'UNAM'.
Sure, I just wanted to be explicit about "known" ignored chunks, vs "unknown" ignored chunks (ie: possibly missing implementation).
Absolutely! But where it makes sense. I still fail to see what you expect to find in an INFO or UNFO list. We had that "explicit ignored" and WARN() stuff there for 10+ years now and nothing came out of it. Just a lot of noise when reading the debug output. There's a reason I didn't add that to the generic implementation. And if needed we still have +dmfile to see all chunks if needed.
I meant to use stream_chunk_parse_desc in dmobj_parsedescriptor later (see rbernon/wine(a)44538293), and progressively change existing code to directly use stream_chunk_parse_desc.
That makes no sense to me. Why do you want to change dmobj_parsedescriptor if you plan to get rid of it?\ The linked commit shows the issue with that approach. If both 'INAM' and 'UNAM' would be present the last chunk would win and override the name even when it isn't the supported one. Afair I have that in some tests as I used those to figure out what's going on.
merge IDirectMusicObject_ParseDescriptor and IPersistStream_Load together
NACK. ParseDescriptor is there for a reason as a lightweight info gathering. And the object is immediately destroyed afterwards. Loading the full object is excessive in that case.\ The above is used for caching. Important for "Reference List" if, e.g. it references is by name or guid or similar as opposed to path based.\ So a merged IDirectMusicObject_ParseDescriptor and IPersistStream_Load would let IDirectMusicObject_ParseDescriptor fail if the reference cannot be resolved. Which is incorrect, especially for circular dependencies which are allowed.\ Full loading and caching info in excruciating detail on https://learn.microsoft.com/en-us/previous-versions/ms811062(v=msdn.10)?redi... **Long story short:**\ I'm not opposed to look at a new way of parsing the descriptor.\ But please make the the only thing in the patch series and don't touch dmobj_parsedescriptor if you plan to anyway get rid of it.\ Also please pick for that an already cleaned up object. E.g. dlls/dmime/segtriggertrack.c\ That allows for an easy comparison of the full solution against the existing cleaned up version.\ -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3795#note_45255
participants (2)
-
Michael Stefaniuc (＠mstefani) -
Rémi Bernon