WIP: resync parser, dont kill the stream#80
Open
alexanderadam wants to merge 5 commits into
Open
Conversation
EBML element bigger than MAX_EBML used to return WEBM_ERROR. callers ignore the return value so playback just stops a few seconds in. people see it as "song cuts off early". new behaviour: scan the buffer for the next CLUSTER id and pick up there. if no cluster boundary is in the buffer yet, drop most of the buffer and wait for more bytes. we might lose a fragment but the song keeps going, which is what users want.
alexanderadam
changed the title
pass the youtube video id into WebM->new and store it in _webm. later commits will use it to tag warn lines so the log is greppable without hunting up to the matching new() line.
[videoId] prefix on every warn and error so grep yt:videoId server.log shows everything that went wrong with one track. also split EBML too large into in cues / in headers / resync so the failure stage is visible.
when EBML reports a bogus huge size and there is no ID_CLUSTER in the next few MB of data, the prebuffer dries out and the player stops anyway. count dropped bytes, give up past 4 MB, propagate WEBM_ERROR up so sysread_URL ends the stream cleanly and LMS skips to the next track instead of just sitting there.
on a bad element_id length walk the parser was throwing away EBML_NEED (12) bytes and leaving id/size stale. that confused the next iteration into a cascade of EBML too large warns and around 57 KB of dropped opus, which sounded like a clear stutter. drop 1 byte instead, clear id/size, let the caller loop. cluster resync usually catches up within a packet, so the audio gap shrinks from ~3s to under 100ms.
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.
EBML element bigger than MAX_EBML used to return WEBM_ERROR. callers ignore the return value so playback just stops a few seconds in. people see it as "song cuts off early".
new behaviour: scan the buffer for the next CLUSTER id and pick up there. if no cluster boundary is in the buffer yet, drop most of the buffer and wait for more bytes. we might lose a fragment but the song keeps going, which is what users want.
It's a little strange that this happens but it works for me 🤷♂️
I'm still not happy with the solution because something leads to broken chunks. It still has this 'stuttering' every in-between. But then then again: better that than killing the stream alltogether. I'm using it right now.
I'm undecided on this one, that's why I wrote WIP before it.
Probably it's still better to merge this, I guess? 🤷♂️
We'd need to find the culprit for the broken chunks though because despite being 'better' it's still not good yet.