Add Wave Game audio cartridge support by MBilderbeek · Pull Request #2126 · openMSX/openMSX

MBilderbeek · 2026-05-14T19:20:54Z

Introduce WaveGame class for MSX Pico audio cartridge emulation
Implement command handling for audio playback via WaveGameCommand
Add XML configuration support for loading WAV files and settings
Register Wave Game as a media provider in the system
Update project files to include new source files

Closes #1975

MBilderbeek · 2026-05-14T21:48:56Z

@m9710797 is this along the lines we discussed? It seems to work well, but I guess more testing would be helpful.

I did not yet implement the wavegame <mediaprovider> subcommand. Maybe it's overkill? But OTOH, maybe it's easy to add :)

m9710797

Quick initial review comments. Many comments are about details, only a few bigger issues.
Later I'd like to do a more detailed review (e.g. really try to understand the details of song loading).

MBilderbeek · 2026-05-16T22:22:37Z

@m9710797 I was thinking about another simplification: replace the .valid field in Song by using a std::array<std::optional<Song>, NUM_SONGS + 1>> for songs. What do you think?

m9710797 · 2026-05-17T07:27:51Z

@m9710797 I was thinking about another simplification: replace the .valid field in Song by using a std::array<std::optional<Song>, NUM_SONGS + 1>> for songs. What do you think?

Good idea. The alternative I had in mind was using wavIdx == -1 to mean invalid. (You can still have a method to abstract this: bool valid() const { return wavIdx != size_t(-1); }.)

m9710797

I did a 2nd partial review pass. Partial because I ran out of time, I will continue later (I reached line 350).

m9710797

Finished the 2nd review pass.

m9710797 · 2026-05-17T17:56:51Z

+				parseCfg(cfgPath, loopOff, startOff);
+			}
+			loopOff  = std::min(loopOff,  sz);
+			startOff = std::min(startOff, sz);


I think this should be
loopOff = std::min(loopOff, sz - 1);

Because we have the invariant:
startSample <= loopSample < endSample
We have half-open intervals (good): start and loop are inclusive, end is exclusive. The original expression could set start/loop equal to end.

And this is the reason I suggested to reject empty wav files (wav.getSize == 0, around line 353). For an empty wav we cannot set start/loop strictly smaller than end.
You could say that start==end means an empty song, but the current implementation in generateChannels() (and with looping == true) assumes songs with at least 1 sample.

Also should we silently correct invalid loop/start values (via this std::min() correction)? Or should we give a warning and reject those invalidly configured songs?

Similar problem on line 449.

@jeroentaverne what does the Pico do with such invalid loop/start values?

m9710797 · 2026-05-17T18:00:35Z

+			size_t end   = (i + 1 < multiStarts.size())
+			             ? std::min(multiStarts[i + 1], mSize)
+			             : mSize;
+			if (start >= end) continue;


Because of this condition, I think the start offsets in multi.cfg must be (strictly?) ascending.
Should we silently reject songs for which this isn't true, or also give a warning?

OK, so @jeroentaverne says:

For multi.wav the offsets are not required to be ascending.

So, I'm not sure what this means for the code. (I haven't fully understood this AI generated code yet...)

See my next push.

m9710797 · 2026-05-17T18:02:04Z

+				.wavIdx      = *multiIdx,
+				.startSample = start,
+				.endSample   = end,
+				.loopSample  = std::min(multiLoop, mSize),


Because multiLoop is the same for all songs, for all but one of the songs, loopSample will be outside the range [startSample, endSample) is that OK?

@jeroentaverne and @MauricioBraga - can you please comment?

The idea of multi.wav is to have one "big song", that has multiple start points (that are also loop offsets), one to each stage of the game. Imagine you are playing King's Valley. The 1st stage "song" starts, but the player is taking too long to finish the stage. So the music will be done playing the first stage song and it will "invade" the region of the 2nd stage music, and if necessary, the region of the 3rd stage song, 4th stage song... Until the user finishes the stage or the wave file ends (which will restart the music at the 1st stage start (loop) offset. When you reach the 2nd stage, it will now start from the 2nd stage start (loop offset) point and so on.

So, you have multiple start points in multi.cfg, that are also the offset points of each song in the big song (multi.wav). In another words, there's no single loop offset that is valid to all music, I guess the document is not very clear about this.

If it helps to clarify, this is the multi.cfg file of the (still unreleased) Boulder Dash wave patch.

0
2648832
5313792
7940864
10588288
13235328
15883360
18525440
21174048
23816896
26466688
29109504

Yes, so the loopsample is the same for all and the end is just the size. I'll adjust it in my next push.

If by loop sample you mean the loop offset, each line in that multi.cfg file I posted it is both a start point but also a loop offset (if the music is supposed to be played in loop). if the game tells that a particular region of the multi.wav should be played in loop, it will comeback to to loop offset used from the point (line) where the music started. it will comeback to that point when the file ends. But if the game tells to play a particular index (line) without loop, music playing will end when the file ends.

But that means there cannot be a real loop offset per song in a multi.wav, only for the first song? Isn't that a bit weird? Also, it means that if you have song 1, 2, 3, 4, start playing at 3, it will then play 4 and then continue with 3. Is that really the intention?

m9710797 · 2026-05-17T18:14:09Z

+	ar.serialize("currentDir", currentDir);
+	if constexpr (Archive::IS_LOADER) {
+		if (!currentDir.empty()) {
+			loadSongs();


This works when the saving/loading the savestate on the same system. We also have (limited) support for saving on one system and loading an another with a slightly different directory structure. We should try to do the same here.

How do you propose we could do that? I don't immediately see how.

OK, I made a start with it, see my next push.

I think we should also serialize autoSamplesDir, and then on loadState restore either "auto" or "dir".
But the handling of currentDir itself seems fine.

Should be done in the lastest push.

jeroentaverne · 2026-05-17T18:59:46Z

Hi. Just found out about the work on WaveGame. Let me know if you have any questions. I can share some code pieces if needed.

MBilderbeek · 2026-05-17T19:02:06Z

Hi. Just found out about the work on WaveGame. Let me know if you have any questions. I can share some code pieces if needed.

Please review the code of this PR and check whether the behaviour will be the same as on the Pico....

jeroentaverne · 2026-05-17T19:46:07Z

Comments for now:

MSX Pico ignores the amount of channels. Playing stereo for instance is just possible. Mono is just a recommendation to reduce the SD card data processing because there are some plans to add a way to play 2 wave files at the same time. One for music and one for sound effects.
For multi.wav the offsets are not required to be ascending.
Continue previous song just assumes it was looping.
When the same song is started as the current one, the current one will not be stored as previous one. But I guess this is already managed.

I will have another look later. Thanks for adding this feature!

MBilderbeek · 2026-05-17T21:13:53Z

* MSX Pico ignores the amount of channels. Playing stereo for instance is just possible. Mono is just a recommendation to reduce the SD card data processing because there are some plans to add a way to play 2 wave files at the same time. One for music and one for sound effects.

And what about the other properties? Does it require 48kHz, 16-bit? Or will it play anything you throw at it? We now have strict checks on this, but I'm not sure if the actual Pico checks that as well.

jeroentaverne · 2026-05-18T20:08:15Z

* MSX Pico ignores the amount of channels. Playing stereo for instance is just possible. Mono is just a recommendation to reduce the SD card data processing because there are some plans to add a way to play 2 wave files at the same time. One for music and one for sound effects.
And what about the other properties? Does it require 48kHz, 16-bit? Or will it play anything you throw at it? We now have strict checks on this, but I'm not sure if the actual Pico checks that as well.

The samplingrate is actually not checked as well. But should be 48kHz, else the PSG and SCC will run at incorrect frequency. 16 bits signed is a must. Currently on holiday. I will do some more review next week.

m9710797 · 2026-05-19T17:50:22Z

+{
+	auto& waveGame = OUTER(WaveGame, command);
+	if (tokens.size() == 1) {
+		result = waveGame.autoSamplesDir ? "auto" : std::string_view{waveGame.currentDir.getResolved()};


Maybe instead of getResolved() we should return getOriginal(). You want to return the string as was originally passed? Or do you want to see the resolved version? You could argue either way:

Setting followed by getting should give the save result. (E.g. that allows to get the current value, temporarily change it, and afterwards restore the original).

Showing the resolved path could also be useful.

How to decide? :)

This is an extension cartridge that implements the wave-game protocol that is currently in use in the MSX-Pico. Use the new wavegame command to manually tweak the folder with the wav files. By default it tries to auto-load these from the same folder as the currently inserted media. It is also possible to let it use a specific media slot's media folder.

MBilderbeek · 2026-05-19T19:44:12Z

@m9710797 is this along the lines we discussed? It seems to work well, but I guess more testing would be helpful.

I did not yet implement the wavegame <mediaprovider> subcommand. Maybe it's overkill? But OTOH, maybe it's easy to add :)

Is now done, so this is obsolete.

jeroentaverne · 2026-05-24T19:20:18Z

Find attached the updated readme file. Hope it's clear now. :-)
WaveGameReadme_updated.txt

jeroentaverne · 2026-05-24T19:31:39Z

As reference also the updated C code used on MSX Pico. (Function file_load_number returns the number on a certain line in a file. The top line is line 0. When file or line doesn't exist, 0 is returned.)
wavegame.c

MBilderbeek · 2026-05-25T08:42:41Z

My comments:

why the memory address read? How does that work? It seems a violation of the MSX standard to have some memory address always return a specific value. It also made the openMSX implementation therefore harder. To me, it would sound much more logical to have the I/O port return the version when being read.
already mentioned in my first comment, but I'd propose to make this less MSX-Pico specific. Suppose someone wants to make a WaveGame compatible separate cartridge. (This is basically what the openMSX implementation is doing.)
What should the implementation do if:
- the wav file does not meet the requirements (not 48kHz, not 16-bit or not mono). Apparently not mono is no problem. But what about the others? Should it reject these? Should they just be used? What happens on a Pico? (Should we emulate that where possible?)
- loop offsets or start offsets are wrong, e.g. beyond the end of file. Ignore them? Clip them?
did I understand correctly that there are no loop offsets specified/possible for multi.wav songs? What happens if loop is enabled for these songs, do they loop at the start offset, after the full wav file is played?

jeroentaverne · 2026-05-25T09:43:38Z

Memory read: your opinion makes sense indeed, I will make IO read possible. No game is currently using the memory read anyway.
Special wavegame cartridge: perfect idea
When the frequency is different, the pitch of SCC and PSG emulation are affected. I have some ideas to fix this in future firmware. I would not emulate this behavior on OpenMSX. 😀
Loop offset for multi.wav: always 0, so starting from first song

MBilderbeek · 2026-05-25T16:48:50Z

@jeroentaverne OK, but what about my other questions? Basically: what to do in case of "illegal" situations. I think when using openMSX one should get warned if things will not work on a "real" WaveGame hardware (like Pico currently). So it's important to know what happens there and then we can replicate it.

jeroentaverne · 2026-05-25T17:14:08Z

I thought I answered all questions. But a proper warning when the wav file isn't 48kHz 16 bits signed should be enough.

MBilderbeek · 2026-05-25T18:39:32Z

@jeroentaverne what about illegal offsets or start positions?

jeroentaverne · 2026-05-25T18:44:43Z

@jeroentaverne what about illegal offsets or start positions?

Illegal loop offset: wav file will play only once
Illegal start position: wav file will not play at all

MBilderbeek · 2026-05-25T19:14:56Z

@jeroentaverne @m9710797 asked on IRC:

Quibus: I also reviewed the attached file "wavegame.c". The only difference I see is that we implement the stop command according to the spec: 0000'xxxx, but in wavegame.c sound_fadeout() is called for all of xx00'0000. But the implementation if sound_fadeout() is not included, possibly that still rejects values of xx'xxxx larger than 15? Do you know if the full source code is available somewhere?

Can you comment on this?

jeroentaverne · 2026-05-25T19:36:27Z

The fadeout can be 63 seconds, so there actually is an error in the manual.

MBilderbeek · 2026-05-29T21:48:47Z

The fadeout can be 63 seconds, so there actually is an error in the manual.

Can you please fix it and paste/upload the update, for completeness sake?

MBilderbeek · 2026-05-29T22:00:51Z

The fadeout can be 63 seconds, so there actually is an error in the manual.

Can you please fix it and paste/upload the update, for completeness sake?

I'm asking, because I'd like to establish a "definitive spec" which we can put in the openMSX implementation. Also including the updated version info byte and all other things discussed here.

MBilderbeek · 2026-05-29T22:01:47Z

@m9710797 any comments and/or questions you still have on that spec in https://github.com/user-attachments/files/28197978/WaveGameReadme_updated.txt?

MBilderbeek · 2026-06-06T19:28:15Z

@jeroentaverne did you see my questions from last week?

MBilderbeek · 2026-06-19T20:15:52Z

@jeroentaverne reminder.... I can't really proceed with this without clarity on what the exact specification is.

MBilderbeek · 2026-07-13T17:02:52Z

@jeroentaverne do you still care about this?

MBilderbeek self-assigned this May 14, 2026

MBilderbeek added the device emulation About emulation of a device (for an emulated machine) label May 14, 2026

MBilderbeek mentioned this pull request May 14, 2026

feat(wavegame): add Wave Game audio cartridge support #2125

Closed

MBilderbeek force-pushed the wavegame branch from 91c681a to 6cac8e5 Compare May 14, 2026 19:28

MBilderbeek marked this pull request as draft May 14, 2026 19:28

MBilderbeek force-pushed the wavegame branch from 6cac8e5 to 114668e Compare May 14, 2026 21:43

MBilderbeek requested a review from m9710797 May 14, 2026 21:47

MBilderbeek force-pushed the wavegame branch 2 times, most recently from d8efd09 to 767a940 Compare May 14, 2026 22:23

m9710797 reviewed May 15, 2026

View reviewed changes

MBilderbeek force-pushed the wavegame branch from 767a940 to 5ca19d5 Compare May 16, 2026 19:01

MBilderbeek marked this pull request as ready for review May 16, 2026 19:02

MBilderbeek force-pushed the wavegame branch from 5ca19d5 to 06dad69 Compare May 16, 2026 22:17

m9710797 reviewed May 17, 2026

View reviewed changes

MBilderbeek force-pushed the wavegame branch from 06dad69 to cb9f22a Compare May 17, 2026 15:16

m9710797 reviewed May 17, 2026

View reviewed changes

MBilderbeek force-pushed the wavegame branch 3 times, most recently from 34b6943 to 518da4e Compare May 17, 2026 22:32

MBilderbeek force-pushed the wavegame branch 2 times, most recently from b945545 to 6d36703 Compare May 18, 2026 22:19

m9710797 reviewed May 19, 2026

View reviewed changes

MBilderbeek force-pushed the wavegame branch from 6d36703 to 8c1b442 Compare May 19, 2026 19:43

Conversation

MBilderbeek commented May 14, 2026

Uh oh!

MBilderbeek commented May 14, 2026

Uh oh!

m9710797 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MBilderbeek commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m9710797 commented May 17, 2026

Uh oh!

m9710797 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

m9710797 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MBilderbeek May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MauricioBraga May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MauricioBraga May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

MBilderbeek commented May 16, 2026 •

edited

Loading

MBilderbeek May 17, 2026 •

edited

Loading

MauricioBraga May 17, 2026 •

edited

Loading

MauricioBraga May 19, 2026 •

edited

Loading

jeroentaverne commented May 24, 2026 •

edited

Loading

jeroentaverne commented May 25, 2026 •

edited

Loading

jeroentaverne commented May 25, 2026 •

edited

Loading