Feature: SDWriter: Add option to write INFO metadata to header of WAV file

[eyuan-creare]

SDWriter.h, SDWriter.cpp

Goal

Write channel names and input calibration to the WAV header's INFOComment chunk.

Ref

IBM's specification on the RIFF file format that WAV files use. INFO chunk described on pg-23

Existing Class and Methods for Writing an SD WAV file

Based on Main: 453c98c. Though I will update this to reflect the new 32bitquad branch. I've abbreviated this somewhat.

Writes header when starting a recording, with file size 0:

- AudioSDWriter_F32::startRecording()
  - AudioSDWriter_F32::startRecording(fname)
    - AudioSDWriter_F32::openAsWAV(fname)
      - SDWriter::openAsWAV(fname)
        - SDWriter::wavHeaderInt16(0)
          - SDWriter::wavHeaderInt16(WAV_sampleRate_Hz, WAV_nchan, 0)

Stops recording by rewriting the header with the actual file size.

- AudioSDWriter_F32::stopRecording()
  - AudioSDWriter_F32:close() 
    - SDWriter::close()
      - SDWriter::wavHeaderInt16(fileSize)
        - SDWriter::wavHeaderInt16(WAV_sampleRate_Hz, WAV_nchan, fileSize)

Scope

TBD

Notes

• Per the RIFF format, the LIST chunk can go anywhere after the FMT chunk. But it seems that some WAV readers prefer it before the data chunk, rather than after.
• Per the RIFF format, LIST chunk should contain the following fields. I am mostly concerned with the Comment subchunk, but I could provide access to the other subchunks.
  IARL IART ICMS ICMT ICOP ICRD ICRP IDIM IDPI IENG IGNR IKEY ILGT IMED INAM IPLT IPRD ISBJ ISFT ISHP ISRC ISRF ITCH.
• Not actually part of this scope, but I would like to check if the AudioSDPlayer can handle an INFO chunk after the audio data chunk. I've noticed that some WAV header editors do this.
• Also check whether writing 3-channel data is handled correctly. Currently, Adobe Audition has no trouble with 3-channel WAV files written by the existing library.

[eyuan-creare]

@chipaudette Do you have a preference regarding the use of union / struct to parse bytestreams? I am using this to read WAV file headers in a different repo. I could save this to a separate .h file so both SDWriter and SDReader have access to them.

// RIFF header uses union to type pun incoming bytestream.
using Riff_Header_u = union {
  struct {
    uint32_t chunkId;             // "RIFF" as LSB
    uint32_t chunkLenBytes;       // File length (in bytes) - 8bytes
    uint32_t format;              // "WAVE" as LSB
  } header;      
  uint8_t byteStream[12] = {0};   // represented as byte array
};

// FMT header uses union to type pun incoming bytestream.
using Fmt_Header_u = union  {
  struct {
    uint32_t chunkId;             // "fmt " as LSB
    uint32_t chunkLenBytes;       // Expect 16 bytes (this chunk - 4 bytes)
    uint16_t audioFmt;            // 1: PCM, otherwise is compressed
    uint16_t numChan;             // # of audio channels
    uint32_t sampleRate;          // bits per second
    uint32_t byteRate;            // SampleRate * NumChannels * BitsPerSample/8
    uint16_t blockAlign;          // NumChannels * BitsPerSample/8
    uint16_t bitsPerSample;       // 8: 8-bit;  16: 16-bit
  } header;
  uint8_t byteStream[24] = {0};   // represented as byte array
};

// data header uses union to type pun incoming bytestream.
using Data_Header_u = union {
  struct {
    uint32_t chunkLenBytes;                         // Number of audio bytes: NumSamples * NumChannels * BitsPerSample/8
  } header;      
  uint8_t byteStream[CHUNK_ID_LEN_BYTES] = {0};     // represented as byte array
};

[chipaudette]

Personally, I don't think that I've ever used a union. So, I'm very inexperienced with them and fear shooting myself in the foot if I were to start using them. In other words, I don't use them because I fear them, and I fear them because I don't use them. An unhelpful cycle.

Additionally, I see unions as an end-run around the strong typing upon which C/C++ is built. To me, choosing to use a union feels a lot like choosing to cast through void *. Yes, a void * cast is an important tool for solving certain problems, but it's also dangerous.

Such end-runs around the type system can be done for good and they can be done for evil. Choose wisely!

[eyuan-creare]

Good point. I'll explore the other options that c++ offers.

[eyuan-creare]

@chipaudette In case you reviewed the approach up at the top, I modified it a bit after digging into how the existing SDWriter class works. Should be a simple mod. Never mind. I just realized I need to insert the INFO chunk between the fmt and data chunks. Will revise.

[eyuan-creare]

@chipaudette This is minor, and probably has no effect as the header is overwritten when closing the file.

When a WAV file is initialized, openAsWave() calls wavHeaderInt16(0) with a fileSize of 0; This is due to not knowing how much audio will be written. However it's a little strange, since it results in the RIFF chunk size being written as a neg number.

nsamp = (0-44)/(21) = -22 ; assume 1-chan
RIFF chunk size = 36 + (-221*2) = -8

int nsamp = (fileSize - WAVheader_bytes) / (nbytes * nchan);
*(int32_t*)(wheader + 4) = 36 + nsamp * nchan * nbytes;  //what is this?  Why 36???

Propose that we change it so that wavHeaderInt16(44) is called. This is the file size if writing just the header, no audio data. It results in nsamp = 0;

And since fileSize is specified, instead of RIFF chunk size expressed as this:

*(int32_t*)(wheader + 4) = 36 + nsamp * nchan * nbytes;  //what is this?  Why 36???

It is really just the total file size subtracted by the two fields that are not included in the size (8 bytes for "RIFF" and RIFF chunk size).

if (filesize>=8) {
    *(int32_t*)(wheader + 4) = fileSize-8;
} else {
    *(int32_t*)(wheader + 4) = 0;
}

Note, in the original code, here is where the 36 is from. (Essentially everything in the header but "RIFF" and RIFF chunk size.)

• Skip "RIFF" (4B) and RIFF chunk size (4B)
• 4 bytes for "WAVE"
• 8 bytes for "fmt " and fmt size
• 16 bytes for contents of fmt chunk
• 8 bytes for "data" and data size

[chipaudette]

Propose that we change it so that wavHeaderInt16(44) is called. This is the file size if writing just the header, no audio data. It results in nsamp = 0;

Sounds good. And thanks for the explanation.

[eyuan-creare]

wavHeaderInt16(0) returns static char wheader[48].

However, only 44 bytes are written to the file, as int WAVheader_bytes = 44;

file.write(wavHeaderInt16(0), WAVheader_bytes)

Not sure if I am missing something. Without the added INFO chunk, audio should start at index 44. But why is wheader 48 bytes long?

[eyuan-creare]

@chipaudette AudioSDPlayer_F32 currently rejects reading WAV files with more than 2 channels, right? What was the reason for this? (Is it due to the RIFF format not having a channel field to specify something like "front rear speaker"?

Tympan_Library/src/AudioSDPlayer_F32.cpp

Line 890 in ef3c80c

} else if (channels == 2) {

I believe that AudioSDPlayBack_F32 does not accept the WAV format 0x0003
WAVE_FORMAT_IEEE_FLOAT, just the standard 0x0001 WAVE_FORMAT_PCM? Are we planning to allow playback of F32 WAV files as well? I do see that you added the 0x0003 format to SDWriter.

Tympan_Library/src/AudioSDPlayer_F32.cpp

Line 861 in ef3c80c

if (format != 1) {

I plan to insert the LIST metadata chunk before the data chunk, as it seems that some WAV players prefer this. To accommodate this insertion, I am revising SDWriter so that the buffer for storing the WAV header is not a single, fixed length. I am about ready to pull in your changes from the branch (https://github.com/Tympan/Tympan_Library/tree/feature/32bit_quad_i2s). Also, I have code to parse WAV headers that is currently in the OpenHearing project that I may be able to merge with Tympan's AudioSDPlayback_F32.

[chipaudette]

@eyuan-creare

In reply to your question, I do believe that AudioSDPlayer_F32 only does 2 channels. If so, it's because I built it directly from Teensy's AudioSDPlayer, which only does two channels. So, it wasn't a choice, it was simply expedient. I would gladly have expanded its capabilities, but no one has asked for more (until now). It could certainly be expanded to 4 channels (or whatever).

As for your question of WAV format 0x0003 and the playing back of Float data type, the inability to do this again traces back to the Teensy's AudioSDPlayer, which lacked this capability. As you noted, I've now added the ability to write Float32 WAV data, but I didn't add the ability to read it. I would be very happy to add Float32 playback, but I've been asked to hold back in order to reserve resources for higher-priority tasks. If you need it, let Brian know!

Finally, when you go to add the LIST metadata chunk, be aware that my addition of Float32 writing has already forced me to alter the handling of the WAV header:

• The WAV header is now created by a more-generic method: makeWavHeader()

• The char array acting as a buffer for the WAV header is now created at a larger size (here). As a result, it can now handle both the smaller Int16 header and the longer Float32 header. You can further enlarge this max size, if you want...just be sure to increase the value of the line right below it, too. Or, maybe it could be a vector or something.

• Because different WAV formats need different header lengths, I also have the class keep track of the length of the header buffer that it's actually using. The data member keeping track of the length is here

• I set the value of the length of the header in makeWavHeader() here.

To add a LIST metadata chunk, you'll probably have to dig into the new makeWavHeader(). I am absolutely not a master of the WAV header format and options. As a result, the logic wtihin makeWavHeader() is a bit convoluted. I'm sorry! While it does work, it could probably be cleaned up. If you see improvements (that don't break anything), please do the improvements!

If you add the LIST chunk to the header (is it in the header? or is it added to the end of the file as a footer?), be aware that altering the header will break the Tympan's ability to read its own WAV files. It is true that my own modifications to add Float32 writing means that the Tympan can't read its own Float32 WAV files, but at least the system can still read its own Int16 WAV files. If you're going to alter the Int16 header that's written, please make sure that you have the budget to also alter the Tympan's reading of the Int16 WAV header. We don't want to break it! Ideally, the Tympan would be able to read both the old Int16 header format and your new one.

[eyuan-creare]

@chipaudette Thanks for providing background on the limitation of playing F32 WAV files and WAV files with > 2 channels. Good idea to wait on this until our minimum requirements are satisfied.

My intention was to add the List metadata chunk in the header, as that seemed to be compatible with more players. (Sorry, I remember looking into this but didn't document my sources). Thanks for the reminder that the Tympan WAV player relies on the data chunk being right after the fmt chunk. In the OpenHearing project, I extended AudioSDPlayer_F32 with a child class that searches for chunkID strings instead of relying on bit position. As the RIFF standard allows the metadata to be placed anywhere after the fmt chunk, this will be more compliant with various WAV editors. I will try to pull this into the Tympan Library, if I can, using a new feature branch. Or as a fallback, write to the footer instead.

Below Updated
@chipaudette To demonstrate writing and reading metadata, should I create (2) new example sketches:

• SDWrite_wMetadata.ino: Write an F32 WAV file w/metadata
• SDWavPlayer.ino: Read I16 WAV file w/ metadata

[eyuan-creare]

Obsoleting WavHeaderInt16()

@chipaudette I noticed that the 32bit_quad branch comments out the public method, wavHeaderInt16(). Should we retain this for backwards compatibility? Perhaps with a switch based on whether Int16 writing is enabled?

[eyuan-creare]

Side note. I don't mean to use this, but Windows ships with C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\um\mmeapi.h, which provides structures for the WAV header that can be referenced in the Visual Studio IDE:

#include "Windows.h"
WAVEFORMATEX 

\\mmeapi.h

/* OLD general waveform format structure (information common to all formats) */
typedef struct waveformat_tag {
    WORD    wFormatTag;        /* format type */
    WORD    nChannels;         /* number of channels (i.e. mono, stereo, etc.) */
    DWORD   nSamplesPerSec;    /* sample rate */
    DWORD   nAvgBytesPerSec;   /* for buffer estimation */
    WORD    nBlockAlign;       /* block size of data */
} WAVEFORMAT, *PWAVEFORMAT, NEAR *NPWAVEFORMAT, FAR *LPWAVEFORMAT;

typedef struct tWAVEFORMATEX
{
    WORD        wFormatTag;         /* format type */
    WORD        nChannels;          /* number of channels (i.e. mono, stereo...) */
    DWORD       nSamplesPerSec;     /* sample rate */
    DWORD       nAvgBytesPerSec;    /* for buffer estimation */
    WORD        nBlockAlign;        /* block size of data */
    WORD        wBitsPerSample;     /* number of bits per sample of mono data */
    WORD        cbSize;             /* the count in bytes of the size of */
                                    /* extra information (after cbSize) */
} WAVEFORMATEX, *PWAVEFORMATEX, NEAR *NPWAVEFORMATEX, FAR *LPWAVEFORMATEX;

[chipaudette]

Obsoleting WavHeaderInt16()

@chipaudette I noticed that the 32bit_quad branch comments out the public method, wavHeaderInt16(). Should we retain this for backwards compatibility? Perhaps with a switch based on whether Int16 writing is enabled?

@eyuan-creare , we could keep it present for backwards compatibility. In truth, this method should always have been private (or protected) method, as there is no need to call it externally. But, you're right, it has been public, so it is possible that someone uses it...someone who is not one of us. Therefore, I agree that we should consider re-enabling it.

My reason for wanting to remove it from the API is that I didn't want to have to handle the case where the class is configured to write Float32 data yet someone goes and invokes wavHeaderInt16(). If we're considering re-enabling waveHeaderInt16(), how should we handle this conflicted request?