Skip to content

Fall back to read for unimplemented readinto adapters - #501

Merged
bastibe merged 2 commits into
bastibe:masterfrom
antonsoo:fix/virtual-readinto-fallback
Oct 24, 2026
Merged

bastibe merged 2 commits into
bastibe:masterfrom
antonsoo:fix/virtual-readinto-fallback

Conversation

@antonsoo

@antonsoo antonsoo commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

A read-only adapter can implement read, seek and tell while inheriting an unimplemented io.RawIOBase.readinto. SoundFile calls the inherited method and fails format detection instead of using its existing read() fallback.

Treat NotImplementedError like an absent readinto method. Adapters with a working readinto keep that path.

import io
import numpy as np
import soundfile as sf

class Reader(io.RawIOBase):
    def __init__(self, data):
        self.source = io.BytesIO(data)
    def read(self, size=-1):
        return self.source.read(size)
    def seek(self, offset, whence=0):
        return self.source.seek(offset, whence)
    def tell(self):
        return self.source.tell()

file = io.BytesIO()
sf.write(file, np.array([1, 256, -2345], dtype="int16"), 8000,
         format="WAV", subtype="PCM_16")
print(sf.read(Reader(file.getvalue()), dtype="int16")[0])

Before: a callback NotImplementedError, followed by LibsndfileError: Format not recognised. After: [1, 256, -2345].

Four WAV/FLAC mono/stereo regressions fail before and pass after; a working-readinto control stays on its original path. All 16 recorded LJSpeech/AMI whole-file/block decoding workflows now complete with exactly the direct-reader samples and independent NumPy frame-energy/spectral features.

Validation: full native suite: 344 passed; CI error lint and diff checks pass. Combined validation with the separate byte-order correction passes 368 tests and all 29 new regressions. Pyright retains the same two existing dirname(_libname) errors as untouched master. Python 3.12/Linux with libsndfile 1.0.31; no cloud-provider, non-blocking-stream or other-platform claim.

The virtual-I/O compatibility probe uses a constructed RawIOBase adapter; no production adapter failure was observed. The regression supplies its unimplemented method explicitly for PyPy portability. Full CPython 3.12 and PyPy 3.10 suites pass (344 cases each), and all sixteen recorded adapter workflows agree on both interpreters.

@bastibe

bastibe commented Oct 12, 2026

Copy link
Copy Markdown
Owner

That's a very obscure error case. Did you stumble upon this naturally?

@antonsoo

Copy link
Copy Markdown
Contributor Author

This came from a deliberate check of the virtual-I/O interface, using a constructed io.RawIOBase adapter with read, seek, and tell. On CPython it inherits a readinto stub that raises NotImplementedError; no production adapter failure was observed. The recorded WAV/FLAC runs check that the existing read fallback decodes the same samples.

The failing PyPy jobs exposed a test assumption: PyPy's RawIOBase has no readinto attribute. The regression now supplies the unimplemented method explicitly, so it exercises the same capability failure on both interpreters. The runtime patch is unchanged.

@bastibe

bastibe commented Oct 13, 2026

Copy link
Copy Markdown
Owner

I'm not sure if this is really a bug worth fixing, as it kind of relys on a maliciously ill-constructed data structure. But the change is small and simple enough, and the tests are worth having. Is this ready to merge from your end?

@antonsoo

Copy link
Copy Markdown
Contributor Author

Yes, the current head is ready from the technical side. The full native suite still passes all 344 tests, and all 16 recorded-audio adapter workflows match the independent sample and feature references. The runtime correction remains unchanged; the latest commit only makes the regression portable to PyPy.

@bastibe
bastibe merged commit 550a47d into bastibe:master Oct 24, 2026
34 checks passed
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