fix(utils): keep whole samples when flushing a partial trailing sample - #6876
Open
rahul2002m wants to merge 1 commit into
Open
fix(utils): keep whole samples when flushing a partial trailing sample#6876rahul2002m wants to merge 1 commit into
rahul2002m wants to merge 1 commit into
Conversation
AudioByteStream.flush() returned an empty list when the buffer did not end on a sample boundary, so a single stray byte discarded every complete sample with it. On an 8 kHz mono stream with a 200 ms target frame that is up to 200 ms of speech, and the warning called it an incomplete frame while dropping the whole buffer. The buffer was also left in place on that path, because only the success path cleared it. The misaligned byte stayed in the stream and offset every later frame by one byte, which comes out as noise rather than as missing audio. Drop just the trailing bytes that cannot form a sample, return the samples before them, and leave the buffer empty either way. The warning now names the number of bytes discarded. Closes livekit#6874
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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
2 participants
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 #6874.
AudioByteStream.flush()bailed out with an empty list whenever the buffer did not end on a sample boundary, so one stray byte took every complete sample with it. On an 8 kHz mono stream with a 200 ms target frame that is up to 200 ms of speech, and the warning called it an incomplete frame while what it actually dropped was the whole buffer.The buffer was also left in place on that path, since only the success path called
clear(). So the misaligned byte stayed in the stream and offset every later frame by a byte, which comes out as noise rather than as missing audio. That second part is the one that worried me more, because it is silent.The fix drops only the trailing bytes that cannot form a sample, returns the samples before them, and leaves the buffer empty on both paths. The warning now says how many bytes went.
I ran into this on a telephony feed at 8 kHz where the source can hand over an odd number of bytes. It may also be worth a look next to #5158, which tracks truncation on the PCM path from the other end, though I have not confirmed they are the same thing.
Tests are in a new
tests/test_utils_audio.py, sinceAudioByteStreamdid not have a module of its own. Ten cases: whole samples surviving a partial tail, only the tail being dropped, the buffer ending up empty, the warning naming the byte count, a lone partial byte, an empty buffer staying quiet, flush after whole frames were emitted, idempotence, and the stereo case where a sample is 4 bytes. Eight of the ten fail on main; the other two are guards for behaviour this does not change.uv run pytest --unitgives 1566 passed against 1556 on main, which is the ten new ones and no regressions.make checkis clean, formatting, ruff and mypy. There are 9 errors in the unit run on my machine from the OTLP metrics exporter at teardown, but they are identical on a clean checkout of main so they are not from this change.