Skip to content

Reject block sizes and overlaps that prevent audio progress - #499

Open
ktz03 wants to merge 1 commit into
bastibe:masterfrom
ktz03:fix-blocks-progress
Open

ktz03 wants to merge 1 commit into
bastibe:masterfrom
ktz03:fix-blocks-progress

Conversation

@ktz03

@ktz03 ktz03 commented Oct 11, 2026

Copy link
Copy Markdown

blocks() can yield indefinitely when blocksize=0 (or out has zero frames) or when overlap is at least the effective block size. The number of newly read frames becomes zero, so iteration keeps yielding empty or repeated blocks without consuming the remaining audio. Negative overlap also produces unintended overlapping windows.

Validate that the effective block size is positive and 0 <= overlap < blocksize before reading. When out is supplied, its length determines the block size. Document these limits and add 16 regressions covering the module function, instance method, and both buffer paths. These tests consume only the first item, so they also fail safely on the previous implementation.

Validation on Windows x64, Python 3.12.14, and libsndfile 1.2.2:

  • Complete local suite: 355 passed; unchanged upstream baseline: 339 passed. All 16 new regressions fail on the baseline.
  • Real WAV/DOUBLE public-input probes: all 28 invalid parameter combinations now raise ValueError; the 112 valid combinations with overlap, limited frames, fill values, and supplied buffers produce identical data before and after the change.
  • pyright soundfile.py: the same two existing diagnostics at line 188 on both versions (_soundfile_data.__file__ can be None), with no added diagnostics. The type check is not fully passing.

This change leaves the short-file/supplied-buffer slicing in #495 unchanged; that PR addresses a separate case with valid parameters.

@bastibe

bastibe commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Looks like a reasonable change for an edge case. Do we similarly guard in read or write against negative frames?

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