Repository navigation
Conversation
StreamableSourceWrapper.seek and BufferedIOBaseWrapper.seek only handled io.SEEK_SET; for io.SEEK_CUR and io.SEEK_END they silently returned the current position, making callers believe the seek succeeded. During metadata parsing (get_buffered_io_metadata) TinyTag issues seek(0, SEEK_END) to probe file size. With the broken implementation that call returned the current position (0) without moving; TinyTag inferred the file had zero length and fell back to a linear read. For an HTTP source backed by PatchedIceCastClient the linear read quickly exhausted the 64 KiB pre-buffer and entered the polling loop, always hitting the full DEFAULT_TIMEOUT (10 s) before the read was satisfied. Fix: * io.SEEK_CUR: convert to an absolute offset and delegate to the existing SEEK_SET path on the underlying source/buffer. Raise OSError if the source rejects the resulting absolute position (e.g. headroom exhausted). * io.SEEK_SET: propagate a False return from the underlying source as OSError instead of silently returning the stale position. * Any other whence (io.SEEK_END): raise OSError immediately, since streaming sources do not know their total size. Raising OSError causes TinyTag to abort metadata parsing at the first unsupported seek rather than walking the entire stream; the exception is caught by the existing except-block in get_buffered_io_metadata, which returns EMPTY_METADATA. The 10 s stall is eliminated. Also route the 'Failed to parse metadata' log message through the module logger (_LOGGER) rather than the root logger so the failure is attributable to pyatv.protocols.raop.audio_source.
SEEK_END now returns the total buffered size for pos=0 and raises OSError for pos>0 (seeking past end). When seek(0, SEEK_END) parks the cursor at a position that SemiSeekableBuffer cannot represent (total = position + size, which is one past the last valid index), a _virtual_position is stored and returned by tell() so that callers such as TinyTag can read the file size without a subsequent tell() returning the stale pre-seek position (0). The virtual position is cleared by the next seek() call. Without this, TinyTag 2.x called seek(0, SEEK_END) then tell() to obtain the filesize; tell() returned 0, so TinyTag skipped _load() entirely and returned empty metadata for every stream, including small ones that were fully buffered. Tests added: - seek(0, SEEK_END) followed by tell() asserts the correct size for both StreamableSourceWrapper and BufferedIOBaseWrapper - test_metadata_small_buffered_stream_preserved: a 2548-byte tagged WAV fully within the seekable headroom yields title/artist via get_buffered_io_metadata (guards against the always-raise regression) - test_metadata_large_stream_returns_fast: a source larger than BUFFER_SIZE returns in < 2 s instead of stalling ~10 s (DEFAULT_TIMEOUT) docs/development/stream.md: note on HTTP metadata extraction window
seek() now raises OSError on failure instead of silently returning the
wrong position. The finally-block in get_buffered_io_metadata called
buffer.seek(0) and then buffer.seek(before) to restore the stream
cursor after metadata parsing; either call could now propagate an
OSError out of the function, breaking playback.
Wrap the restore in try/except OSError and log the existing warning
('Failed to restore position to %d') on any failure so the caller
always receives a MediaMetadata (or EMPTY_METADATA) without an
exception.
Also verified: total = buffer.position + buffer.size is correct for all
SemiSeekableBuffer states. size returns bytes remaining from position,
so position + size = total bytes buffered, which is invariant across
seek and read operations. Tests confirm this at a non-zero cursor.
Tests added:
- test_seek_end_returns_buffered_size (both classes): now reads 50 bytes
first to confirm the total invariant holds when position != 0
- test_metadata_restore_failure_does_not_raise: patches seek to raise
OSError during the restore phase; asserts no exception propagates and
that the warning is emitted
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2910
Streaming an HTTP source stalled for about 10 s before the first sample, as diagnosed in the issue.
StreamableSourceWrapper.seekandBufferedIOBaseWrapper.seekonly handledSEEK_SET. ForSEEK_CURandSEEK_ENDthey did nothing but still returned a position that looked like success, so TinyTag fell back to reading the stream linearly, ran past the parked 64 KiB buffer, andPatchedIceCastClient.readwaited outDEFAULT_TIMEOUT.Changes
pyatv/protocols/raop/audio_source.py:seekon both wrappers:SEEK_CURis converted to an absolute position.SEEK_ENDresolves against the bytes buffered so far (position + size).seek(0, SEEK_END)parks a virtual position sotell()reports the size, which keeps metadata working for small, fully buffered streams.OSErrorinstead of returning a stale position, so TinyTag gives up right away rather than walking the stream.get_buffered_io_metadata:docs/development/stream.md: a short note on the HTTP metadata window and what happens with larger sources.This doesn't touch
PatchedIceCastClient.readortests/protocols/raop/test_audio_source.py, so it shouldn't conflict with #2850.Testing
tests/protocols/raop/test_audio_source_seek.py:SEEK_SET,SEEK_CURandSEEK_ENDbehavior and error paths for both wrappers, includingseek(0, SEEK_END)after a partial read.get_buffered_io_metadata; this passes on master and with the fix.BUFFER_SIZEnow returns in well under 2 s; on master it takes about 10 s and fails.