Incorrect padding to 4 byte length for WaveFile with 8bit samples

[kevinjwalters]

This code looks wrong for some values:

https://github.com/adafruit/circuitpython/blob/82427612d15ac85d0a34132ace7f8369dacd66ef/shared-module/audiocore/WaveFile.c#L223=L237

For length_read of 4 it's ok, 6 happens to work, but 5 and 7 would be problematic as they would yield pad lengths of 1 and 3, respectively, rather than the desired 3 and 1. I suspect that was intended to say something like:

            uint32_t pad = sizeof(uint32_t) - length_read % sizeof(uint32_t);

I haven't tested this, this is just from reading the code.

Another pertinent question here is whether the relevant buffer is guaranteed to be in sizeof(uint32_t) lumps.

[jepler]

I think you're right about the first thing, please pull request the change especially if you have a test case.

I also think you're right that the intent of this code is to ensure that the size of the buffer returned to the caller is always a multiple of 4 bytes long, no matter whether it's 1 or 2 samples, 8 or 16 bit samples. (however, raw samples can be any length so the actual audio hal implementations can't depend on this in general)

Note that when looping samples are involved, they should actually be a multiple of the buffer size. Any other size increases the chance of an audio glitch when the sample is looped.

[heygauri]

Hello @kevinjwalters, I'm eager to work on this issue, but I'm unsure about the testing process for the proposed changes. Could you please provide guidance on how I should test the above modifications?

[kevinjwalters]

I didn't do any testing with problematic data for this, I just spotted the code looks suspect. Perhaps make some good wav files and then read them in and check the data is ok. That should fail for ones where the % 4 length is wrong. The code pads it with a mid value, 128 (0x80) for unsigned 8 bit, 0 for signed 16 bit.

Something like this in big python

#!/usr/bin/python3

import wave

sample_rate = 16000
data = bytes(range(32, 32 + 8 + 1))

for length in (4,5,6,7,8):
    with wave.open("testdata-u8-{:d}.wav".format(length), "w") as fh:
        fh.setsampwidth(1)  # (unsigned) 8bit samples
        fh.setnchannels(1)
        fh.setframerate(sample_rate)
        fh.writeframes(data[:length])