bastibe / python-soundfile

SoundFile is an audio library based on libsndfile, CFFI, and NumPy
BSD 3-Clause "New" or "Revised" License
717 stars 111 forks source link

Fixing blocks read with overlap for files shorter than block size #446

Closed illesguy closed 2 weeks ago

illesguy commented 2 weeks ago

Summary

The blocks method to read an audio in frame blocks with an optional overlap has a bug in the edge case when the file has less frames than the block size specified and the overlap specified is not 0. The full file and an overlap number of random values are returned in the output in this case.

Explanation

When the method is called without a provided out array, it creates an empty array to store the output with the np.empty method with the shape corresponding to the block size argument (and the number of channels but that is not relevant for this problem). The default values in an array created by np.empty are not deterministic.

The file is then read into this out array up to the block size or the end of the file, whichever is shorter. The number of frames in the file plus the overlap size is copied into a block from the out array which is returned.

Example

Let's take an example of a mono file with 5 frames, a block size of 10 and an overlap of 1.

The test_block_longer_than_file_with_overlap_mono test was added to cover this case. It did fail as expected before adding the fix.

What happens when the block size is smaller than the file length but there is an overlap?

In case the where the file has multiple blocks in it, the error doesn't occur because at the end of each block read, the overlapping frames are added to the beginning of the out array and the next frames for the file are read on top of that. Due to this, when returning the final block, we will again return the number of frames left in the file plus the overlap frames, but in this case the overlap frames will be the frames added to the beginning of the output in the previous run so this case isn't impacted by the bug.

The fix

When creating the initial out array (in case it was not provided as an argument) we just need to make sure to create it with a length that is equal to the block size OR the number of frames in the file, whichever is smaller. If the block size is smaller than the number of frames in the file, it will work as previously and as discussed above, that case is not affected by the bug. If however the block size is greater, the output array will be equal to the number of frames in the file but in this case there's only going to be one block anyway so having an overlap does not make sense.

bastibe commented 2 weeks ago

Congratulations! It's quite rare that someone finds a real bug in this project.

And thank you for fixing it right away as well!

illesguy commented 2 weeks ago

Thanks for being responsive and for the merge! Will there be a new version released with this commit?

bastibe commented 2 weeks ago

There are still a few cross-compilation issues with the CI that need to be resolved before I'll cut a new version. Unfortunately, this is not my area of expertise, and I don't have a whole lot of time available to devote to this. I can't promise any particular time frame, but I hope to find time for it during the Christmas holidays at the latest.