[PATCH v3 0/6] MR3795: dmusic: Rewrite collection parsing with the new stream chunk primitives.
-- v3: dmusic: Add more parsed instruments traces. dmusic: Parse instrument name from INFO list. dmusic: Rewrite collection DLS chunk parsing. dmusic: Rewrite collection INFO list parsing. dmusic: Rewrite collection ptbl chunk 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 | 46 +++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index acd85768cec..4d690f597db 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 | 109 ++++++----------------------------- dlls/dmusic/dmobject.c | 7 +++ dlls/dmusic/dmobject.h | 1 + dlls/dmusic/dmusic_main.c | 5 -- dlls/dmusic/dmusic_private.h | 2 - 5 files changed, 25 insertions(+), 99 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index 4d690f597db..168a49fa8c0 100644 --- a/dlls/dmusic/collection.c +++ b/dlls/dmusic/collection.c @@ -44,7 +44,6 @@ struct collection LONG ref; IStream *pStm; /* stream from which we load collection and later instruments */ - CHAR *szCopyright; /* FIXME: should probably be placed somewhere else */ DLSHEADER *pHeader; struct pool *pool; @@ -277,9 +276,10 @@ static const IDirectMusicObjectVtbl collection_object_vtbl = static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, IStream *stream) { + struct chunk_entry dls_chunk = {0}; 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 */ @@ -307,6 +307,21 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, return E_FAIL; } + dls_chunk.id = FOURCC_RIFF; + dls_chunk.size = chunk.dwSize; + dls_chunk.type = chunk.fccID; + liMove.QuadPart = 0; + IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &dls_chunk.offset); + dls_chunk.offset.QuadPart -= 12; + if (FAILED(dmobj_parsedescriptor(stream, &dls_chunk, &This->dmobj.desc, + DMUS_OBJ_NAME_INFO|DMUS_OBJ_VERSION|DMUS_OBJ_OBJECT|DMUS_OBJ_GUID_DLID))) + { + liMove.QuadPart = StreamSize; + IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); /* skip the rest of the chunk */ + return E_FAIL; + } + stream_reset_chunk_data(stream, &dls_chunk); + TRACE_(dmfile)(": collection form\n"); do { IStream_Read(stream, &chunk, sizeof(FOURCC) + sizeof(DWORD), NULL); @@ -319,18 +334,6 @@ static HRESULT WINAPI collection_stream_Load(IPersistStream *iface, IStream_Read(stream, This->pHeader, chunk.dwSize, NULL); break; } - case FOURCC_DLID: { - TRACE_(dmfile)(": DLID (GUID) chunk\n"); - This->dmobj.desc.dwValidData |= DMUS_OBJ_OBJECT; - IStream_Read(stream, &This->dmobj.desc.guidObject, chunk.dwSize, NULL); - break; - } - case FOURCC_VERS: { - TRACE_(dmfile)(": version chunk\n"); - This->dmobj.desc.dwValidData |= DMUS_OBJ_VERSION; - IStream_Read(stream, &This->dmobj.desc.vVersion, chunk.dwSize, NULL); - break; - } case FOURCC_PTBL: { struct chunk_entry ptbl_chunk = {.id = FOURCC_LIST, .size = chunk.dwSize, .type = chunk.fccID}; TRACE_(dmfile)(": pool table chunk\n"); @@ -344,85 +347,7 @@ 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: { - 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]); - break; - } case FOURCC_WVPL: { TRACE_(dmfile)(": wave pool list (mark & skip)\n"); liMove.QuadPart = chunk.dwSize - sizeof(FOURCC); diff --git a/dlls/dmusic/dmobject.c b/dlls/dmusic/dmobject.c index 2bef5511851..3007aceef3e 100644 --- a/dlls/dmusic/dmobject.c +++ b/dlls/dmusic/dmobject.c @@ -587,7 +587,14 @@ HRESULT dmobj_parsedescriptor(IStream *stream, const struct chunk_entry *riff, desc->wszFileName, sizeof(desc->wszFileName)) == S_OK) desc->dwValidData |= DMUS_OBJ_FILENAME; break; + case FOURCC_DLID: + if (!(supported & DMUS_OBJ_GUID_DLID)) break; + if ((supported & DMUS_OBJ_OBJECT) && stream_chunk_get_data(stream, &chunk, + &desc->guidObject, sizeof(desc->guidObject)) == S_OK) + desc->dwValidData |= DMUS_OBJ_OBJECT; + break; case DMUS_FOURCC_GUID_CHUNK: + if ((supported & DMUS_OBJ_GUID_DLID)) break; if ((supported & DMUS_OBJ_OBJECT) && stream_chunk_get_data(stream, &chunk, &desc->guidObject, sizeof(desc->guidObject)) == S_OK) desc->dwValidData |= DMUS_OBJ_OBJECT; diff --git a/dlls/dmusic/dmobject.h b/dlls/dmusic/dmobject.h index 98069de8cc4..ae06935dfea 100644 --- a/dlls/dmusic/dmobject.h +++ b/dlls/dmusic/dmobject.h @@ -92,6 +92,7 @@ HRESULT dmobj_parsedescriptor(IStream *stream, const struct chunk_entry *riff, DMUS_OBJ_NAME is 'UNAM' chunk in UNFO list */ #define DMUS_OBJ_NAME_INAM 0x1000 /* 'INAM' chunk in UNFO list */ #define DMUS_OBJ_NAME_INFO 0x2000 /* 'INAM' chunk in INFO list */ +#define DMUS_OBJ_GUID_DLID 0x4000 /* 'dlid' chunk instead of 'guid' */ /* 'DMRF' (reference list) helper */ HRESULT dmobj_parsereference(IStream *stream, const struct chunk_entry *list, 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 | 176 +++++++++++++---------------------- dlls/dmusic/dmusic_private.h | 6 -- 2 files changed, 63 insertions(+), 119 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index 168a49fa8c0..f26806a1afb 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,7 @@ struct collection struct dmobject dmobj; LONG ref; - IStream *pStm; /* stream from which we load collection and later instruments */ - DLSHEADER *pHeader; - + DLSHEADER header; struct pool *pool; struct list instruments; }; @@ -232,6 +229,49 @@ 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; + + if (FAILED(hr = dmobj_parsedescriptor(stream, parent, &This->dmobj.desc, + DMUS_OBJ_NAME_INFO|DMUS_OBJ_VERSION|DMUS_OBJ_OBJECT|DMUS_OBJ_GUID_DLID)) + || FAILED(hr = stream_reset_chunk_data(stream, parent))) + return 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): + /* already parsed by dmobj_parsedescriptor */ + 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) { @@ -273,121 +313,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 chunk_entry dls_chunk = {0}; 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; - } - - dls_chunk.id = FOURCC_RIFF; - dls_chunk.size = chunk.dwSize; - dls_chunk.type = chunk.fccID; - liMove.QuadPart = 0; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, &dls_chunk.offset); - dls_chunk.offset.QuadPart -= 12; - if (FAILED(dmobj_parsedescriptor(stream, &dls_chunk, &This->dmobj.desc, - DMUS_OBJ_NAME_INFO|DMUS_OBJ_VERSION|DMUS_OBJ_OBJECT|DMUS_OBJ_GUID_DLID))) + if ((hr = stream_next_chunk(stream, &chunk)) == S_OK) { - liMove.QuadPart = StreamSize; - IStream_Seek(stream, liMove, STREAM_SEEK_CUR, NULL); /* skip the rest of the chunk */ - return E_FAIL; - } - stream_reset_chunk_data(stream, &dls_chunk); - - 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_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 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; - } - } - TRACE_(dmfile)(": StreamCount = %ld < StreamSize = %ld\n", StreamCount, StreamSize); - } while (StreamCount < StreamSize); - - TRACE_(dmfile)(": reading finished\n"); + switch (MAKE_IDTYPE(chunk.id, chunk.type)) + { + case MAKE_IDTYPE(FOURCC_RIFF, FOURCC_DLS): + hr = parse_dls_chunk(This, stream, &chunk); + break; + default: + WARN("Invalid collection chunk %s %s\n", debugstr_fourcc(chunk.id), debugstr_fourcc(chunk.type)); + hr = DMUS_E_UNSUPPORTED_STREAM; + break; + } + } - /* DEBUG: dumps whole collection object tree: */ - if (TRACE_ON(dmusic)) + if (SUCCEEDED(hr) && TRACE_ON(dmusic)) { struct instrument_entry *entry; int i = 0; @@ -396,13 +345,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 | 17 +++++++++++------ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/dlls/dmusic/collection.c b/dlls/dmusic/collection.c index f26806a1afb..cbcdabb37c1 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; }; @@ -156,13 +157,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; } @@ -190,7 +185,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..dbbfd6f0c78 100644 --- a/dlls/dmusic/instrument.c +++ b/dlls/dmusic/instrument.c @@ -266,11 +266,16 @@ 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; + if (FAILED(hr = dmobj_parsedescriptor(stream, parent, desc, DMUS_OBJ_NAME_INFO|DMUS_OBJ_GUID_DLID)) + || FAILED(hr = stream_reset_chunk_data(stream, parent))) + return hr; + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) { switch (MAKE_IDTYPE(chunk.id, chunk.type)) @@ -280,7 +285,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): + /* already parsed by dmobj_parsedescriptor */ break; case MAKE_IDTYPE(FOURCC_LIST, FOURCC_LRGN): @@ -303,7 +309,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 +320,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 +329,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 dbbfd6f0c78..025bba940e5 100644 --- a/dlls/dmusic/instrument.c +++ b/dlls/dmusic/instrument.c @@ -328,14 +328,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
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.
Of course, I didn't mean them to do the same, but the parsing code could be shared, and the chunks required only for Load would be skipped when called from ParseDescriptor. Anyway, I left the controversial parts for a later time :). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3795#note_45341
On Thu Sep 14 19:36:36 2023 +0000, Rémi Bernon wrote:
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. Of course, I didn't mean them to do the same, but the parsing code could be shared, and the chunks required only for Load would be skipped when called from ParseDescriptor. Anyway, I left the controversial parts for a later time :). Well sure, but the descriptor parsing is shared now too in both functions.
Anyway, much easier to review now without unrelated en passant changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3795#note_45375
This merge request was approved by Michael Stefaniuc. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3795
participants (2)
-
Michael Stefaniuc (＠mstefani) -
Rémi Bernon