Skip to content

Return final short buffers from non-seekable audio - #503

Open
antonsoo wants to merge 1 commit into
bastibe:masterfrom
antonsoo:fix/short-nonseekable-buffer-reads
Open

antonsoo wants to merge 1 commit into
bastibe:masterfrom
antonsoo:fix/short-nonseekable-buffer-reads

Conversation

@antonsoo

Copy link
Copy Markdown
Contributor

buffer_read() assumes every requested frame was read. That fails at EOF on non-seekable files: a valid five-frame XI file raises AssertionError for buffer_read(99, dtype="int16"), while read(99, dtype="int16") returns its five samples.

Size the returned CFFI buffer using the actual native frame count. The buffer retains ownership of its allocation. This also handles a zero-frame read at EOF.

Five regressions cover all four supported dtypes, retained buffers after close/GC, and a stereo RAW pipe. They fail against unchanged master. On two recorded speech clips, all 24 XI/RAW-pipe loading flows now finish with exactly the reference samples and frame-energy/spectral features. Ten existing array/seekable-buffer controls remain exact.

Full suite: 376 passed on CPython and PyPy with libsndfile 1.2.2. The two existing pyright errors at dirname(_libname) reproduce on unchanged master; no new diagnostics. Linux validation only; this does not change blocks() or virtual-I/O callback handling.

@bastibe

bastibe commented Oct 24, 2026

Copy link
Copy Markdown
Owner

Presumably you're using XI because it's not seekable? Or why does this use XI?

@antonsoo

Copy link
Copy Markdown
Contributor Author

Yes, XI was chosen because libsndfile reports it as non-seekable, as in the existing non-seekable-file tests. It makes the short read reproducible with a regular temporary file.

The bug also occurs with RAW audio from a POSIX pipe: requesting more frames than remain raises the same assertion. The added stereo pipe test covers that case too. All five new cases pass locally, including reads at EOF and buffers retained after closing the file.

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