frabert/riff

Chokes on LA-MULANA .sgt files

crumblingstatue opened this issue · 8 comments

The old free version (not the remake) of LA-MULANA uses DirectMusic .sgt files for its music.

I'm interested in writing a music player for it in Rust. I noticed that you already have a C++ project for playing DirectMusic, but I would prefer to use Rust, if possible.

I actually tried to play the music of LA-MULANA with libdmusic, but it seems to output silence for some reason. Don't know if it's a bug in libdmusic or something on my end, but maybe you would be interested in testing your library with LA-MULANA.

You can download LA-MULANA here http://agtp.romhack.net/extra/LA-MULANA.zip. It's the old freeware version, not the remake on Steam, so it's legal.

Anyway, back on topic. This library seems to choke on the .sgt files in the music subfolder of the game.
The error is Custom { kind: UnexpectedEof, error: StringError("failed to fill whole buffer") }.

Frustrated, I rolled my own hand-written RIFF parser which seems to work on these .sgt files, but I would rather not reinvent the wheel.

My question boils down to: Since your DirectMusic project is in C++, are you still interested in maintaining this Rust library? If so, I could try to figure out what's wrong and submit a patch. I could even volunteer as a maintainer if you want.

Hi! Indeed libdmusic is still very buggy as it was really only tested to work on a single specific game, and unfortunately I have not found the time to debug it properly.
I have written this library when I was experimenting with the idea of rewriting libdmusic in Rust, but the issues I found when dealing with interoperability with C++ were many and I desisted.

I am more than happy to accept patches, and if your are willing I will gladly give you access to the repo.

A final word of caution when writing a music player for DirectMusic files: the biggest issues lie (IMO) in the possibly compressed DLS files -- the only way I found to properly decompress the samples inside is using libsndfile, which is GPL only (or LGPL, can't remember), and also only has a C api. Best of luck!

It seems like the error originates in a seqt chunk, which your library handles as a list. My own parser just treats it as a binary blob.

Should seqt really be treated as a list by a generic RIFF parser? It's not part of the RIFF standard.

Shouldn't it make more sense to just let the user parse a seqt blob as a RIFF list, if they consider seqt to be such?

Anyway, I'll try adding seqt parsing support to my own parser to confirm for sure whether it's seqt that throws a wrench into the parsing.

Microsoft themselves confess that the seqt chunk does not conform to the usual RIFF conventions: https://docs.microsoft.com/en-us/previous-versions/ms810844(v%3dmsdn.10)

It's weird that it's causing issues, because I do the exact same thing in libdmusic and the files I use are parsed correctly... Anyway, I agree it's not an elegant solution to hardcode the list-chunks, it would be better to have something like what I did with my other lib riffcpp, that instead of eagerly reading the data into memory it just lists the chunks, and the programmer is free to choose whether to interpret the chunk as a list or as a binary blob

Anyway, I agree it's not an elegant solution to hardcode the list-chunks, it would be better to have something like what I did with my other lib riffcpp, that instead of eagerly reading the data into memory it just lists the chunks, and the programmer is free to choose whether to interpret the chunk as a list or as a binary blob

I have actually been thinking about doing the same thing with my own parser. If you want, I could help rewrite this library to act this way.

And yes, seqt messes up my parser too. I'm currently investigating the details.

Wait a minute... Am I misinterpreting something, or does seqt lack the "list-id" field that RIFF and LIST have?

If so, that's the problem. We can't just treat it the same way as RIFF and LIST, and try to read the list-id.

I added a special case to my parser to not parse a list-id for seqt chunks, and the parser doesn't freak out anymore, so it seems I'm on the right track. Going to try the same with this library.

I think you are right, I was going by memory when writing this, but it turns out that I did have to do some special handling in the case of libdmusic: https://github.com/frabert/libdmusic/blob/master/src/Riff.cpp#L29

Any kind of contribution regarding the API is welcome, I believe there is currently nothing relying on the interface remaining stable.

I opened #2 to as a quick fix.

Eventually, I'll submit a pull request to rewrite the whole library with a similar design to riffcpp.