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)%5C 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)?redi... 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@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.\