Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cc.audio.wav module #1928

Open
wants to merge 2 commits into
base: mc-1.20.x
Choose a base branch
from

Conversation

MCJack123
Copy link
Contributor

This PR adds a new cc.audio.wav module, which assists in decoding WAV files. Derived from my WAV code in AUKit, it's able to properly read an entire WAV file, including metadata (which is now used to show the file's title and artist in speaker play if present). This module makes it much easier to work with WAV audio files, and I hope it encourages more adoption of WAV over raw DFPWM.

The module consists of a main function, readWAV, which reads full WAV file data and returns a table with the contents of the file; and readWAVFile, which is a simple wrapper that reads from a file instead. The table contains the following elements:

  • codec: A string with information about the codec used in the file (one of u8, s16, s24, s32, f32, dfpwm)
  • sampleRate: The sample rate of the audio in Hz. If this is not 48000, the file will need to be resampled to play correctly.
  • channels: The number of channels in the file (1 = mono, 2 = stereo).
  • length: The number of samples in the file. Divide by sample rate to get seconds.
  • metadata: If the WAV file contains INFO metadata, this table contains the metadata.
    Known keys are converted to friendly names like artist, album, and track, while unknown keys are kept the same.
    Otherwise, this table is empty.
  • read(length: number): number[]...: This is a function that reads the audio data in chunks.
    It takes the number of samples to read, and returns each channel chunk as multiple return values.
    Channel data is in the same format as speaker.playAudio takes: 8-bit signed numbers.

The reader supports 8/16/24/32-bit PCM, 32-bit float, and DFPWM codecs. It does not perform any resampling if the file isn't 48kHz. Any further file support is left to 3rd-party libraries (such as AUKit). The WAV parser in speaker.lua has been replaced with cc.audio.wav, which makes WAV parsing a bit more robust.

I also updated some docs in cc.audio.dfpwm for the most recent information about FFmpeg.

Copy link
Member

@SquidDev SquidDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

It is a bit of a shame that the interface is so different to the DFPWM module. It might be possible to add a more reader-based approach to DFPWM to match readWAV here, will have a think.

I think it would be useful to add some additional comments throughout the code here. As it stands, this is currently ~100 LOC of pretty dense parsing code, and having some references back to the original spec would be useful for people coming back to this code in the future.

I am a little concerned about error handling. There's a lot of cases right now where string.unpack will blow up if the input is truncated in any way, and not quite sure what to do about that.

expect(1, data, "string")
local bitDepth, dataType, blockAlign
local temp, pos = str_unpack("c4", data)
if temp ~= "RIFF" then error("bad argument #1 (not a WAV file)", 2) end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realise CC is a little inconsistent here1, but I think we should probably follow what Lua does, and return nil + the error message instead of erroring here.

Footnotes

  1. For example, textutils.unserialiseJSON should probably return nil, message rather than erroring.

local function readWAVFile(path)
expect(1, path, "string")
local file = assert(fs.open(path, "rb"))
local data = file.readAll()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could avoid reading the whole file into memory at once. I wonder if readWAV could take a "reader" function (like function(bytes: number): string|nil), so you can just do readWAV(file.read) directly.

This would require some changes to parsing the INFO section (which uses string.unpack("s")), but I think otherwise should be possible?

I guess this would then mean there needs to be a close method on the WAV handle, to clean up the underlying file handle. Ughr.

elseif dataType == "signed" then
if bitDepth == 16 then function transform(n) return math_floor(n / 0x100) end
elseif bitDepth == 24 then function transform(n) return math_floor(n / 0x10000) end
elseif bitDepth == 32 then function transform(n) return math_floor(n / 0x1000000) end end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be an error here for an invalid bit depth?

end
function retval.read(samples)
if pos > #data then return nil end
local chunk = { ("<" .. format:rep(math.min(samples * channels, (#data - pos + 1) / (bitDepth / 8)))):unpack(data, pos) }
Copy link
Member

@SquidDev SquidDev Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I don't feel good about this line (the extra format string allocation, and then putting it into a table), but some quick benchmarks do show it's the quickest approach :(. The crimes we must commit sometimes!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I didn't like it either, but I found the same results after a comparison in AUKit. It's gross, but it's for the best.

@SquidDev SquidDev added enhancement An extension of a feature or a new feature. area-CraftOS This affects CraftOS, or any other Lua portions of the mod. labels Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CraftOS This affects CraftOS, or any other Lua portions of the mod. enhancement An extension of a feature or a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants