Skip to content

Do not yield stale frames from blocks() when out= is longer than the file - #495

Open
dylanpulver wants to merge 1 commit into
bastibe:masterfrom
dylanpulver:fix-blocks-out-overlap
Open

Do not yield stale frames from blocks() when out= is longer than the file#495
dylanpulver wants to merge 1 commit into
bastibe:masterfrom
dylanpulver:fix-blocks-out-overlap

Conversation

@dylanpulver

@dylanpulver dylanpulver commented Sep 1, 2026

Copy link
Copy Markdown

blocks() should only yield frames that came out of the file. When the caller supplies out= and that array is longer than the file, it also yields part of the caller's own array.

A 1-frame stereo file, read into a 3x2 buffer with overlap=2:

out = np.full((3, 2), -999.0)          # caller's buffer
list(sf.blocks(one_frame_stereo_file, overlap=2, out=out))
# [[   1.    2.]
#  [-999. -999.]        <- never read from the file
#  [-999. -999.]]

Cause. soundfile.py:1208 slices the block as out[:frames + overlap], where frames is the number of frames still unread. That is only the right length once the buffer already carries overlap frames from the previous block; on the first pass output_offset is 0, so the slice runs overlap frames past the real data. This uses output_offset + toread, correct on every pass.

#446 fixed the same arithmetic for out is None by allocating min(blocksize, frames). A caller-supplied array cannot be shrunk, so that path kept the defect. The existing out= tests use a file longer than out, so the intersection was untested.

Verification (libsndfile 1.2.2, same venv both legs). An oracle written from the docstring, not from the source, over a 196-cell grid: 70 failures before, 0 after on the out= path, and 0 in both legs on the blocksize= path — that second number validates the oracle rather than the fix. pytest 331 -> 339. Reverting the slice fails the 8 new tests and no others; the naive out[:frames] breaks the blocksize= path, so dropping the overlap term is not the fix.

Not covered: WAV/DOUBLE and mono int16 only, no non-seekable file. overlap_memory = np.copy(out[-overlap:]) reads the same stale region but is unreachable today, so it is untouched.

If #213 (removing out=) is still the direction, close this instead.

Written with AI assistance (Claude).

… file

The last block was sliced as out[:frames + overlap], where frames is the
number of frames still unread.  That is only the valid length when the
iteration already carries overlap frames at the front of the buffer.  On
the first pass output_offset is 0, so the slice reaches overlap frames
past the data actually read, and with out= those frames are whatever the
caller left in the array.

The valid length is output_offset + toread on every pass.  Slicing to it
also removes the need for the blocksize > frames + overlap guard, which
did not fire when blocksize == frames + overlap and yielded the whole
buffer instead.

bastibe#446 fixed the same arithmetic for the out is None path by allocating
min(blocksize, frames); a caller-supplied array cannot be shrunk, so
that path kept the defect.
@bastibe

bastibe commented Sep 5, 2026

Copy link
Copy Markdown
Owner

I must confess, I do not understand the problem, nor the solution (and don't have the time to look into it). Could you explain the problem with a few more examples of how it works currently, what is wrong with it, and how this PR fixes it?

@dylanpulver

Copy link
Copy Markdown
Author

Sorry — the description was far too long to be useful. Plainly:

blocks() should only ever hand back frames that came out of the file. When you pass your own out= array and that array is bigger than the file, it also hands you back part of your own array.

Two examples, both on master today. The -999 is only a marker so you can see it; normally it is whatever your array happened to be holding.

A 1-frame stereo file, into a 3x2 buffer, overlap=2:

now:      [[   1.    2.]      should be:  [[1. 2.]]
           [-999. -999.]
           [-999. -999.]]

A 2-frame mono file, into a 4-frame buffer, overlap=3:

now:      [   1.    2. -999. -999.]        should be:  [1. 2.]

The first file holds one frame, so one frame is all that can come back. The other two rows are the caller's own data being presented as audio.

Why: the yielded slice is out[:frames + overlap], where frames is how many frames are still unread. That length is only correct once the buffer is already carrying overlap frames from the previous block. On the very first block nothing has been carried over yet, so the slice runs overlap frames past the real data. The fix slices by how much was actually written instead.

It only appears when out is longer than the file, which is exactly the case the existing out= tests do not cover — test_blocks_with_out and test_blocks_inplace_modification both use a file longer than out.

Happy to close this instead if #213 (dropping out= from blocks()) is still the direction you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants