Implement the builtin sound/null.wav file

[illwieckz]

Implement the builtin sound/null.wav file.

[illwieckz]

One drawback of such implementation is that:

] which sound/null.wav
File not found: "sound/null.wav"

[illwieckz]

The good thing in that implementation is that the null samples now only takes 2 bytes in memory when loaded!

Once compiled the code implementing it is probably smaller than the 46 bytes of the “smallest silent WAV file ever” that could be embedded instead.

[slipher]

How about simply filename == "sound/null" || filename == "sound/null.wav"? This would be more correct in the case of something like sound/null.notarealextension

[illwieckz]

Hmm, right, someone may look for null.not expecting to load null.not.opus.

So the best would be to look for the basename with known extensions instead.

[illwieckz]

For the moment the code doesn't care for unknown extensions:

]/playMusic sound/ui/heartbeat.plop
Debug: Loading Sample 'sound/ui/heartbeat.plop'
]/playMusic sound/ui/heartbeat1.haha
Debug: Loading Sample 'sound/ui/heartbeat1.haha'

So there may be redesigns to do that are not part of this PR.

[illwieckz]

I taken care of not misdetecting non-existent extensions, but actually the existing code is very perfectible and should be revamped at some point (see below).

Also it's funny that the sound/null sample is now the only sound file that fails to load when attempting to load it with an unknown extension.

[slipher]

What's wrong with just filename == "sound/null" || filename == "sound/null.wav"? The loader iteration thing is unnecessarily complex. There is no reason for anyone to ever write sound/null.opus.

[illwieckz]

Well, I prefer to be exhaustive… I did in Sample.cpp without ext check.

[slipher]

Nit: I think the more conceptually correct place for this would be in Sample::Load, since in this case we aren't using a codec.

[illwieckz]

The soundExtToLoaderMap_t isn't known from Sample.cpp so testing for the extension would not keep the code self-contained in a single file.

[slipher]

out.rawSamples.resize(2) (fills with zeroes) ? The current version does a memory leak. Also why not 8-bit? :-)

[illwieckz]

Also why not 8-bit? :-)

If the engine allows it, why not. I'll try.

[illwieckz]

The engine itself doesn't complain, but when I do:

]/playMusic sound/null

I hear a click noise, likely from the audio subsystem itself when switching the resampling mode or so. That should not happen, but we better do not randomly change the size and keep the things done as usual.

[slipher]

The null_wav memory leak still needs to be fixed. One alternative would be out.rawSamples.resize(2) to fill with two zeroes.

[illwieckz]

Oh lol that's the worst occurrence of ghost code resurfacing on rebase I ever seen!

Reimplemented the .resize(2) way.

[slipher]

Also don't forget to delete it from daemon_src.dpkdir.

[illwieckz]

I reworked the detection of the null sound file to avoid misdetecting sound/null.not as sound/null.wav instead of sound/null.not.opus, and I kept the code in LoadSoundCodec() because with the current design it's the most convenient place.

A lot of things is messy in the existing code: the code reports as empty a missing file, it reports the loading of sound files with any extension that doesn't exist and you can reload it as many time as you request a different extension, it does string copies all the way down, etc. But revamping that would require its own PR and that would be a large PR. That's why I stick to the least intrusive effort of hijacking LoadSoundCodec() for now.

I now deleted the file from the dpkdir.

[illwieckz]

@slipher any new comment?

[slipher]

LGTM