audiocore: fix WaveFile 8-bit sample padding at end of file - #11320
Conversation
The last buffer was padded by length_read % 4 bytes instead of 4 - length_read % 4, so a 5 or 7 byte tail still ended unaligned. Round caller-supplied buffer halves down to a multiple of 4 so the pad always fits, and add a unix coverage test.
There was a problem hiding this comment.
I wonder if we should actually just validate that the buffer size is a multiple of 4. Right now the code works but the buffer sizes chosen may be surprising to the user. For instance, a passed-in buffer size of 8-15 is going to end up being 2 4-byte buffers.
Rather than rounding each half of a caller-supplied buffer down to a multiple of 4, reject lengths that are not a multiple of 8 up front so the buffer the user passes is the buffer that gets used.
|
Agreed, silently shrinking the buffer is surprising. In bd9b603 the constructor now raises |
dhalbert
left a comment
dhalbert
left a comment
There was a problem hiding this comment.
I like this; thanks for the fix. And thanks for correcting my 4 vs the correct 8.
2 participants
WaveFile pads its last buffer to a multiple of four bytes, but shared-module/audiocore/WaveFile.c computed the pad as
length_read % 4. An 8-bit file ending in 5 or 7 bytes was padded to 6 or 10, still unaligned; 16-bit files were unaffected since the remainder is always 2. A caller-supplied buffer was also split into unaligned halves, so the pad could overrun the second half.The pad is now
4 - length_read % 4, and caller-supplied halves are rounded down to a multiple of four. A new test writes 8-bit mono WAV files of 4 to 8 samples to a VfsFat RAM disk and checks the buffers from get_buffer; it fails on main and passes with this change.Fixes #3145