dmime: Add a generic ParseDescriptor helper and use it for DMSegment.

dlls/dmime/dmobject.c

@@ -21,12 +21,152 @@
  */
 
 #define COBJMACROS
+#include <assert.h>
 #include "objbase.h"
 #include "dmusici.h"
+#include "dmusicf.h"
 #include "dmobject.h"
 #include "wine/debug.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(dmobj);
+WINE_DECLARE_DEBUG_CHANNEL(dmfile);
+
+/* RIFF format parsing */
+#define CHUNK_HDR_SIZE (sizeof(FOURCC) + sizeof(DWORD))
+
+static inline const char *debugstr_fourcc(DWORD fourcc)
+{
+    if (!fourcc) return "''";
+    return wine_dbg_sprintf("'%c%c%c%c'", (char)(fourcc), (char)(fourcc >> 8),
+            (char)(fourcc >> 16), (char)(fourcc >> 24));
+}
+
+const char *debugstr_chunk(const struct chunk_entry *chunk)
+{
+    const char *type = "";
+
+    if (!chunk)
+        return "(null)";
+    if (chunk->id == FOURCC_RIFF || chunk->id == FOURCC_LIST)
+        type = wine_dbg_sprintf("type %s, ", debugstr_fourcc(chunk->type));
+    return wine_dbg_sprintf("%s chunk, %ssize %u", debugstr_fourcc(chunk->id), type, chunk->size);
+}
+
+static HRESULT stream_read(IStream *stream, void *data, ULONG size)
+{
+    ULONG read;
+    HRESULT hr;
+
+    hr = IStream_Read(stream, data, size, &read);
+    if (FAILED(hr))
+        TRACE_(dmfile)("IStream_Read failed: %08x\n", hr);
+    else if (!read && read < size) {
+        /* All or nothing: Handle a partial read due to end of stream as an error */
+        TRACE_(dmfile)("Short read: %u < %u\n", read, size);
+        return E_FAIL;
+    }
+
+    return hr;
+}
+
+HRESULT stream_get_chunk(IStream *stream, struct chunk_entry *chunk)
+{
+    static const LARGE_INTEGER zero;
+    ULONGLONG ck_end = 0, p_end = 0;
+    HRESULT hr;
+
+    hr = IStream_Seek(stream, zero, STREAM_SEEK_CUR, &chunk->offset);
+    if (FAILED(hr))
+        return hr;
+    assert(!(chunk->offset.QuadPart & 1));
+    if (chunk->parent) {
+        p_end = chunk->parent->offset.QuadPart + CHUNK_HDR_SIZE + ((chunk->parent->size + 1) & ~1);
+        if (chunk->offset.QuadPart == p_end)
+            return S_FALSE;
+        ck_end = chunk->offset.QuadPart + CHUNK_HDR_SIZE;
+        if (ck_end > p_end) {
+            WARN_(dmfile)("No space for sub-chunk header in parent chunk: ends at offset %s > %s\n",
+                    wine_dbgstr_longlong(ck_end), wine_dbgstr_longlong(p_end));
+            return E_FAIL;
+        }
+    }
+
+    hr = stream_read(stream, chunk, CHUNK_HDR_SIZE);
+    if (hr != S_OK)
+        return hr;
+    if (chunk->parent) {
+        ck_end += (chunk->size + 1) & ~1;
+        if (ck_end > p_end) {
+            WARN_(dmfile)("No space for sub-chunk data in parent chunk: ends at offset %s > %s\n",
+                    wine_dbgstr_longlong(ck_end), wine_dbgstr_longlong(p_end));
+            return E_FAIL;
+        }
+    }
+
+    if (chunk->id == FOURCC_LIST || chunk->id == FOURCC_RIFF) {
+        hr = stream_read(stream, &chunk->type, sizeof(FOURCC));
+        if (hr != S_OK)
+            return hr != S_FALSE ? hr : E_FAIL;
+    }
+
+    TRACE_(dmfile)("Returning %s\n", debugstr_chunk(chunk));
+
+    return S_OK;
+}
+
+HRESULT stream_skip_chunk(IStream *stream, struct chunk_entry *chunk)
+{
+    LARGE_INTEGER end;
+
+    end.QuadPart = (chunk->offset.QuadPart + CHUNK_HDR_SIZE + chunk->size + 1) & ~(ULONGLONG)1;
+
+    return IStream_Seek(stream, end, STREAM_SEEK_SET, NULL);
+}
+
+HRESULT stream_next_chunk(IStream *stream, struct chunk_entry *chunk)
+{
+    HRESULT hr;
+
+    if (chunk->id) {
+        hr = stream_skip_chunk(stream, chunk);
+        if (FAILED(hr))
+            return hr;
+    }
+
+    return stream_get_chunk(stream, chunk);
+}
+
+HRESULT stream_chunk_get_data(IStream *stream, const struct chunk_entry *chunk, void *data,
+        ULONG size)
+{
+    if (chunk->size != size) {
+        WARN_(dmfile)("Chunk %s (size %u, offset %s) doesn't contains the expected data size %u\n",
+                debugstr_fourcc(chunk->id), chunk->size,
+                wine_dbgstr_longlong(chunk->offset.QuadPart), size);
+        return E_FAIL;
+    }
+    return stream_read(stream, data, size);
+}
+
+HRESULT stream_chunk_get_wstr(IStream *stream, const struct chunk_entry *chunk, WCHAR *str,
+        ULONG size)
+{
+    ULONG len;
+    HRESULT hr;
+
+    hr = IStream_Read(stream, str, min(chunk->size, size), &len);
+    if (FAILED(hr))
+        return hr;
+
+    /* Don't assume the string is properly zero terminated */
+    str[min(len, size - 1)] = 0;
+
+    if (len < chunk->size)
+        return S_FALSE;
+    return S_OK;
+}
+
+
 
 /* Generic IDirectMusicObject methods */
 static inline struct dmobject *impl_from_IDirectMusicObject(IDirectMusicObject *iface)
@@ -110,6 +250,78 @@ 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)
+{
+    struct chunk_entry chunk = {.parent = info};
+    char name[DMUS_MAX_NAME];
+    ULONG len;
+    HRESULT hr = E_FAIL;
+
+    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);
+
+    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;
+    }
+}
+
+static inline void unfo_get_name(IStream *stream, const struct chunk_entry *unfo,
+        DMUS_OBJECTDESC *desc, BOOL inam)
+{
+    struct chunk_entry chunk = {.parent = unfo};
+
+    while (stream_next_chunk(stream, &chunk) == S_OK)
+        if (chunk.id == DMUS_FOURCC_UNAM_CHUNK || (inam && chunk.id == mmioFOURCC('I','N','A','M')))
+            if (stream_chunk_get_wstr(stream, &chunk, desc->wszName, sizeof(desc->wszName)) == S_OK)
+                desc->dwValidData |= DMUS_OBJ_NAME;
+}
+
+HRESULT dmobj_parsedescriptor(IStream *stream, const struct chunk_entry *riff,
+        DMUS_OBJECTDESC *desc, DWORD supported)
+{
+    struct chunk_entry chunk = {.parent = riff};
+    HRESULT hr;
+
+    TRACE("Looking for %#x in %p: %s\n", supported, stream, debugstr_chunk(riff));
+
+    desc->dwValidData = 0;
+    desc->dwSize = sizeof(*desc);
+
+    while ((hr = stream_next_chunk(stream, &chunk) == S_OK)) {
+        switch (chunk.id) {
+            case DMUS_FOURCC_GUID_CHUNK:
+                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_CATEGORY_CHUNK:
+                if ((supported & DMUS_OBJ_CATEGORY) && stream_chunk_get_wstr(stream, &chunk,
+                            desc->wszCategory, sizeof(desc->wszCategory)) == S_OK)
+                    desc->dwValidData |= DMUS_OBJ_CATEGORY;
+                break;
+            case DMUS_FOURCC_VERSION_CHUNK:
+                if ((supported & DMUS_OBJ_VERSION) && stream_chunk_get_data(stream, &chunk,
+                            &desc->vVersion, sizeof(desc->vVersion)) == S_OK)
+                    desc->dwValidData |= DMUS_OBJ_VERSION;
+                break;
+            case FOURCC_LIST:
+                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);
+                break;
+        }
+    }
+    TRACE("Found %#x\n", desc->dwValidData);
+
+    return hr;
+}
+
 /* Generic IPersistStream methods */
 static inline struct dmobject *impl_from_IPersistStream(IPersistStream *iface)
 {

dlls/dmime/dmobject.h

@@ -19,6 +19,31 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
+#include "wine/debug.h"
+
+/* RIFF stream parsing */
+struct chunk_entry;
+struct chunk_entry {
+    FOURCC id;
+    DWORD size;
+    FOURCC type;                        /* valid only for LIST and RIFF chunks */
+    ULARGE_INTEGER offset;              /* chunk offset from start of stream */
+    const struct chunk_entry *parent;   /* enclosing RIFF or LIST chunk */
+};
+
+HRESULT stream_get_chunk(IStream *stream, struct chunk_entry *chunk) DECLSPEC_HIDDEN;
+HRESULT stream_next_chunk(IStream *stream, struct chunk_entry *chunk) DECLSPEC_HIDDEN;
+HRESULT stream_skip_chunk(IStream *stream, struct chunk_entry *chunk) DECLSPEC_HIDDEN;
+
+HRESULT stream_chunk_get_data(IStream *stream, const struct chunk_entry *chunk, void *data,
+        ULONG size) DECLSPEC_HIDDEN;
+HRESULT stream_chunk_get_wstr(IStream *stream, const struct chunk_entry *chunk, WCHAR *str,
+        ULONG size) DECLSPEC_HIDDEN;
+
+const char *debugstr_chunk(const struct chunk_entry *chunk) DECLSPEC_HIDDEN;
+
+
+/* IDirectMusicObject base object */
 struct dmobject {
     IDirectMusicObject IDirectMusicObject_iface;
     IPersistStream IPersistStream_iface;
@@ -38,6 +63,14 @@ HRESULT WINAPI dmobj_IDirectMusicObject_GetDescriptor(IDirectMusicObject *iface,
 HRESULT WINAPI dmobj_IDirectMusicObject_SetDescriptor(IDirectMusicObject *iface,
         DMUS_OBJECTDESC *desc) DECLSPEC_HIDDEN;
 
+/* Helper for IDirectMusicObject::ParseDescriptor */
+HRESULT dmobj_parsedescriptor(IStream *stream, const struct chunk_entry *riff,
+        DMUS_OBJECTDESC *desc, DWORD supported) DECLSPEC_HIDDEN;
+/* Additional supported flags for dmobj_parsedescriptor.
+   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 */
+
 /* Generic IPersistStream methods */
 HRESULT WINAPI dmobj_IPersistStream_QueryInterface(IPersistStream *iface, REFIID riid,
         void **ret_iface) DECLSPEC_HIDDEN;

dlls/dmime/segment.c

@@ -576,176 +576,40 @@ static const IDirectMusicSegment8Vtbl dmsegment8_vtbl = {
 };
 
 /* IDirectMusicSegment8Impl IDirectMusicObject part: */
-static HRESULT WINAPI IDirectMusicObjectImpl_ParseDescriptor(IDirectMusicObject *iface,
-        IStream *pStream, DMUS_OBJECTDESC *pDesc)
+static HRESULT WINAPI seg_IDirectMusicObject_ParseDescriptor(IDirectMusicObject *iface,
+        IStream *stream, DMUS_OBJECTDESC *desc)
 {
-	DMUS_PRIVATE_CHUNK Chunk;
-	DWORD StreamSize, StreamCount, ListSize[1], ListCount[1];
-	LARGE_INTEGER liMove; /* used when skipping chunks */
-
-        TRACE("(%p,%p, %p)\n", iface, pStream, pDesc);
-
-	/* FIXME: should this be determined from stream? */
-	pDesc->dwValidData |= DMUS_OBJ_CLASS;
-	pDesc->guidClass = CLSID_DirectMusicSegment;
-
-	IStream_Read (pStream, &Chunk, sizeof(FOURCC)+sizeof(DWORD), NULL);
-	TRACE_(dmfile)(": %s chunk (size = 0x%04x)", debugstr_fourcc (Chunk.fccID), Chunk.dwSize);
-	switch (Chunk.fccID) {	
-		case FOURCC_RIFF: {
-			IStream_Read (pStream, &Chunk.fccID, sizeof(FOURCC), NULL);				
-			TRACE_(dmfile)(": RIFF chunk of type %s", debugstr_fourcc(Chunk.fccID));
-			StreamSize = Chunk.dwSize - sizeof(FOURCC);
-			StreamCount = 0;
-			if (Chunk.fccID == DMUS_FOURCC_SEGMENT_FORM) {
-				TRACE_(dmfile)(": segment form\n");
-				do {
-					IStream_Read (pStream, &Chunk, sizeof(FOURCC)+sizeof(DWORD), NULL);
-					StreamCount += sizeof(FOURCC) + sizeof(DWORD) + Chunk.dwSize;
-					TRACE_(dmfile)(": %s chunk (size = 0x%04x)", debugstr_fourcc (Chunk.fccID), Chunk.dwSize);
-					switch (Chunk.fccID) {
-						case DMUS_FOURCC_GUID_CHUNK: {
-							TRACE_(dmfile)(": GUID chunk\n");
-							pDesc->dwValidData |= DMUS_OBJ_OBJECT;
-							IStream_Read (pStream, &pDesc->guidObject, Chunk.dwSize, NULL);
-							break;
-						}
-						case DMUS_FOURCC_VERSION_CHUNK: {
-							TRACE_(dmfile)(": version chunk\n");
-							pDesc->dwValidData |= DMUS_OBJ_VERSION;
-							IStream_Read (pStream, &pDesc->vVersion, Chunk.dwSize, NULL);
-							break;
-						}
-						case DMUS_FOURCC_CATEGORY_CHUNK: {
-							TRACE_(dmfile)(": category chunk\n");
-							pDesc->dwValidData |= DMUS_OBJ_CATEGORY;
-							IStream_Read (pStream, pDesc->wszCategory, Chunk.dwSize, NULL);
-							break;
-						}
-						case FOURCC_LIST: {
-							IStream_Read (pStream, &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) {
-								/* evil M$ UNFO list, which can (!?) contain INFO elements */
-								case DMUS_FOURCC_UNFO_LIST: {
-									TRACE_(dmfile)(": UNFO list\n");
-									do {
-										IStream_Read (pStream, &Chunk, sizeof(FOURCC)+sizeof(DWORD), NULL);
-										ListCount[0] += sizeof(FOURCC) + sizeof(DWORD) + Chunk.dwSize;
-										TRACE_(dmfile)(": %s chunk (size = 0x%04x)", debugstr_fourcc (Chunk.fccID), Chunk.dwSize);
-										switch (Chunk.fccID) {
-											/* don't ask me why, but M$ puts INFO elements in UNFO list sometimes
-											   (though strings seem to be valid unicode) */
-											case mmioFOURCC('I','N','A','M'):
-											case DMUS_FOURCC_UNAM_CHUNK: {
-												TRACE_(dmfile)(": name chunk\n");
-												pDesc->dwValidData |= DMUS_OBJ_NAME;
-												IStream_Read (pStream, pDesc->wszName, Chunk.dwSize, NULL);
-												break;
-											}
-											case mmioFOURCC('I','A','R','T'):
-											case DMUS_FOURCC_UART_CHUNK: {
-												TRACE_(dmfile)(": artist chunk (ignored)\n");
-												liMove.QuadPart = Chunk.dwSize;
-												IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-												break;
-											}
-											case mmioFOURCC('I','C','O','P'):
-											case DMUS_FOURCC_UCOP_CHUNK: {
-												TRACE_(dmfile)(": copyright chunk (ignored)\n");
-												liMove.QuadPart = Chunk.dwSize;
-												IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-												break;
-											}
-											case mmioFOURCC('I','S','B','J'):
-											case DMUS_FOURCC_USBJ_CHUNK: {
-												TRACE_(dmfile)(": subject chunk (ignored)\n");
-												liMove.QuadPart = Chunk.dwSize;
-												IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-												break;
-											}
-											case mmioFOURCC('I','C','M','T'):
-											case DMUS_FOURCC_UCMT_CHUNK: {
-												TRACE_(dmfile)(": comment chunk (ignored)\n");
-												liMove.QuadPart = Chunk.dwSize;
-												IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-												break;
-											}
-											default: {
-												TRACE_(dmfile)(": unknown chunk (irrelevant & skipping)\n");
-												liMove.QuadPart = Chunk.dwSize;
-												IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-												break;						
-											}
-										}
-										TRACE_(dmfile)(": ListCount[0] = %d < ListSize[0] = %d\n", ListCount[0], ListSize[0]);
-									} while (ListCount[0] < ListSize[0]);
-									break;
-								}
-								case DMUS_FOURCC_TRACK_LIST: {
-								  TRACE_(dmfile)(": TRACK list\n");
-								  do {
-								    IStream_Read (pStream, &Chunk, sizeof(FOURCC)+sizeof(DWORD), NULL);
-								    ListCount[0] += sizeof(FOURCC) + sizeof(DWORD) + Chunk.dwSize;
-                                                                    TRACE_(dmfile)(": %s chunk (size = %d)", debugstr_fourcc (Chunk.fccID), Chunk.dwSize);
-								    switch (Chunk.fccID) {
-								    default: {
-								      TRACE_(dmfile)(": unknown chunk (irrelevant & skipping)\n");
-								      liMove.QuadPart = Chunk.dwSize;
-								      IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-								      break;						
-								    }
-								    }
-                                                                    TRACE_(dmfile)(": ListCount[0] = %d < ListSize[0] = %d\n", ListCount[0], ListSize[0]);
-								  } while (ListCount[0] < ListSize[0]);
-								  break;
-								}
-							        default: {
-								  TRACE_(dmfile)(": unknown (skipping)\n");
-								  liMove.QuadPart = Chunk.dwSize - sizeof(FOURCC);
-								  IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-								  break;						
-								}
-							}
-							break;
-						}	
-						default: {
-							TRACE_(dmfile)(": unknown chunk (irrelevant & skipping)\n");
-							liMove.QuadPart = Chunk.dwSize;
-							IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL);
-							break;						
-						}
-					}
-					TRACE_(dmfile)(": StreamCount[0] = %d < StreamSize[0] = %d\n", StreamCount, StreamSize);
-				} while (StreamCount < StreamSize);
-				break;
-			} else if (Chunk.fccID == mmioFOURCC('W','A','V','E')) {
-				TRACE_(dmfile)(": wave form (loading not yet implemented)\n");
-				liMove.QuadPart = StreamSize;
-				IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL); /* skip the rest of the chunk */
-			} else {
-				TRACE_(dmfile)(": unexpected chunk (loading failed)\n");
-				liMove.QuadPart = StreamSize;
-				IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL); /* skip the rest of the chunk */
-				return E_FAIL;
-			}
-		
-			TRACE_(dmfile)(": reading finished\n");
-			break;
-		}
-		default: {
-			TRACE_(dmfile)(": unexpected chunk; loading failed)\n");
-			liMove.QuadPart = Chunk.dwSize;
-			IStream_Seek (pStream, liMove, STREAM_SEEK_CUR, NULL); /* skip the rest of the chunk */
-			return DMUS_E_INVALIDFILE;
-		}
-	}	
-	
-	TRACE(": returning descriptor:\n%s\n", debugstr_DMUS_OBJECTDESC (pDesc));
-	
-	return S_OK;
+    struct chunk_entry riff = {0};
+    DWORD supported = DMUS_OBJ_OBJECT | DMUS_OBJ_VERSION;
+    HRESULT hr;
+
+    TRACE("(%p, %p, %p)\n", iface, stream, desc);
+
+    if (!stream || !desc)
+        return E_POINTER;
+
+    if ((hr = stream_get_chunk(stream, &riff)) != S_OK)
+        return hr;
+    if (riff.id != FOURCC_RIFF || !(riff.type == DMUS_FOURCC_SEGMENT_FORM ||
+                riff.type == mmioFOURCC('W','A','V','E'))) {
+        TRACE("loading failed: unexpected %s\n", debugstr_chunk(&riff));
+        stream_skip_chunk(stream, &riff);
+        return E_FAIL;
+    }
+
+    if (riff.type == DMUS_FOURCC_SEGMENT_FORM)
+        supported |= DMUS_OBJ_NAME | DMUS_OBJ_CATEGORY;
+    else
+        supported |= DMUS_OBJ_NAME_INFO;
+    hr = dmobj_parsedescriptor(stream, &riff, desc, supported);
+    if (FAILED(hr))
+        return hr;
+
+    desc->guidClass = CLSID_DirectMusicSegment;
+    desc->dwValidData |= DMUS_OBJ_CLASS;
+
+    TRACE("returning descriptor:\n%s\n", debugstr_DMUS_OBJECTDESC (desc));
+    return S_OK;
 }
 
 static const IDirectMusicObjectVtbl dmobject_vtbl = {
@@ -754,7 +618,7 @@ static const IDirectMusicObjectVtbl dmobject_vtbl = {
     dmobj_IDirectMusicObject_Release,
     dmobj_IDirectMusicObject_GetDescriptor,
     dmobj_IDirectMusicObject_SetDescriptor,
-    IDirectMusicObjectImpl_ParseDescriptor
+    seg_IDirectMusicObject_ParseDescriptor
 };
 
 /* IDirectMusicSegment8Impl IPersistStream part: */

dlls/dmime/tests/dmime.c

@@ -794,7 +794,8 @@ static void test_parsedescriptor(void)
                     "ParseDescriptor failed: %08x, expected DMUS_E_CHUNKNOTFOUND\n", hr);
         else
             ok(hr == E_FAIL, "ParseDescriptor failed: %08x, expected E_FAIL\n", hr);
-        todo_wine ok(!desc.dwValidData, "Got valid data %#x, expected 0\n", desc.dwValidData);
+        todo_wine_if(forms[i].needs_size)
+            ok(!desc.dwValidData, "Got valid data %#x, expected 0\n", desc.dwValidData);
 
         /* All desc chunks */
         alldesc[1] = forms[i].form;
@@ -806,14 +807,15 @@ static void test_parsedescriptor(void)
         valid = DMUS_OBJ_OBJECT | DMUS_OBJ_CLASS | DMUS_OBJ_VERSION;
         if (forms[i].form != mmioFOURCC('W','A','V','E'))
             valid |= DMUS_OBJ_NAME | DMUS_OBJ_CATEGORY;
-        todo_wine_if(forms[i].form == mmioFOURCC('W','A','V','E'))
-            ok(desc.dwValidData == valid, "Got valid data %#x, expected %#x\n", desc.dwValidData,
-                    valid);
+        ok(desc.dwValidData == valid, "Got valid data %#x, expected %#x\n", desc.dwValidData, valid);
         ok(IsEqualGUID(&desc.guidClass, forms[i].clsid), "Got class guid %s, expected CLSID_%s\n",
                 wine_dbgstr_guid(&desc.guidClass), forms[i].class);
         ok(IsEqualGUID(&desc.guidObject, &GUID_NULL), "Got object guid %s, expected GUID_NULL\n",
                 wine_dbgstr_guid(&desc.guidClass));
-        if (forms[i].form == DMUS_FOURCC_SEGMENT_FORM)
+        ok(desc.vVersion.dwVersionMS == 5 && desc.vVersion.dwVersionLS == 8,
+            "Got version %u.%u, expected 5.8\n", desc.vVersion.dwVersionMS,
+            desc.vVersion.dwVersionLS);
+        if (forms[i].form != mmioFOURCC('W','A','V','E'))
             ok(!memcmp(desc.wszName, s_unam, sizeof(s_unam)), "Got name '%s', expected 'UNAM'\n",
                     wine_dbgstr_w(desc.wszName));
         IStream_Release(stream);
@@ -825,9 +827,7 @@ static void test_parsedescriptor(void)
         desc.dwSize = sizeof(desc);
         hr = IDirectMusicObject_ParseDescriptor(dmo, stream, &desc);
         ok(hr == S_OK, "ParseDescriptor failed: %08x, expected S_OK\n", hr);
-        todo_wine_if(forms[i].form == mmioFOURCC('W','A','V','E'))
-            ok(desc.dwValidData == valid, "Got valid data %#x, expected %#x\n", desc.dwValidData,
-                    valid);
+        ok(desc.dwValidData == valid, "Got valid data %#x, expected %#x\n", desc.dwValidData, valid);
         IStream_Release(stream);
 
         /* UNFO list with INAM */
@@ -838,7 +838,7 @@ static void test_parsedescriptor(void)
         desc.dwSize = sizeof(desc);
         hr = IDirectMusicObject_ParseDescriptor(dmo, stream, &desc);
         ok(hr == S_OK, "ParseDescriptor failed: %08x, expected S_OK\n", hr);
-        todo_wine_if(forms[i].form != mmioFOURCC('W','A','V','E'))
+        todo_wine_if(forms[i].needs_size)
             ok(desc.dwValidData == DMUS_OBJ_CLASS, "Got valid data %#x, expected DMUS_OBJ_CLASS\n",
                     desc.dwValidData);
         IStream_Release(stream);
@@ -853,12 +853,10 @@ static void test_parsedescriptor(void)
         valid = DMUS_OBJ_CLASS;
         if (forms[i].form == mmioFOURCC('W','A','V','E'))
             valid |= DMUS_OBJ_NAME;
-        todo_wine_if(forms[i].form == mmioFOURCC('W','A','V','E'))
-            ok(desc.dwValidData == valid, "Got valid data %#x, expected %#x\n", desc.dwValidData,
-                    valid);
+        ok(desc.dwValidData == valid, "Got valid data %#x, expected %#x\n", desc.dwValidData, valid);
         if (forms[i].form == mmioFOURCC('W','A','V','E'))
-            todo_wine ok(!memcmp(desc.wszName, s_inam, sizeof(s_inam)),
-                    "Got name '%s', expected 'I'\n", wine_dbgstr_w(desc.wszName));
+            ok(!memcmp(desc.wszName, s_inam, sizeof(s_inam)), "Got name '%s', expected 'I'\n",
+                    wine_dbgstr_w(desc.wszName));
         IStream_Release(stream);
 
         IDirectMusicObject_Release(dmo);