Do not take a partial response's Content-Length as the resource length - #198
Do not take a partial response's Content-Length as the resource length#198lpaiu-cs wants to merge 1 commit into
Conversation
A subclass can narrow a connect request to part of the resource by rewriting the URL rather than by sending a Range header, which is what useHeadersForRange() returning false is for. The response to that is an ordinary 200, so nothing in it says it is partial, and its Content-Length describes only the part that came back. Filling in an unknown length from that header therefore replaces the unknown marker with the size of one window, and the stream stops there. Skip the header when the request was narrowed, leaving the length unknown so the subclass can keep asking for further windows.
36f6778 to
c27dcb1
Compare
|
End-to-end now, on a live Lavalink rather than a harness. Built Lavalink 4.2.2 from source against a 2.2.6 checkout carrying this change, and ran youtube-source main with the A 298s track played 297s and ended The range sequence for that track is the part I would point at: That last window is unclamped because the length was still unknown when it was built, and the server simply returns the remainder. With the probe in place the same track ended The only error in the window is an unrelated metadata-stage login wall two seconds before playback started, which does not involve the streaming client. For the record on the earlier numbers in the description: those came from a standalone harness driving |
|
Something I must ask -- is YouTube deployed a change years ago that meant invalid, or too large ranges (including those with a range end larger than the content length itself) would automatically throttle the stream down to about 80KB/s. Setting the correct Does this behaviour remain unaffected or are we going to run into issues with throttling? |
|
@devoxin Clamping when the length is known is untouched — On throttling: I ran this on the host where the truncation shows up. Same URL, The 80KB/s behaviour seems to have turned into a hard rejection somewhere along One other thing while I was in there — of 107 itag 18 streams I looked at, 16% |
Fixes #197.
useHeadersForRange()exists so a subclass can ask for part of a resource by rewriting the URL instead of sending aRangeheader. The server answers such a request with an ordinary200— nothing marks it partial — and itsContent-Lengthdescribes only the part that came back.attemptConnectfills in an unknown length from that header, so the unknown marker is replaced by the size of one window on the first connect, and the stream stops there believing it has reached the end.This skips the header when the request was narrowed, leaving the length unknown as it was. Requests that were not narrowed are unaffected.
Measured
On a host where this reproduces, reading a real URL to EOF and counting bytes.
youtube-sourceis the caller that hits this: itsYoutubePersistentHttpStreamsetsuseHeadersForRange()tofalse, and its itag 18 fallback arrives with nocontentLengthof its own. True size 752848816:Rows two and four are there because I had assumed the subclass would also need work to keep going without a length. It does not — with the length left unknown, the existing
rangeEnd == contentLengthcheck inYoutubePersistentHttpStream.internalReadalready falls through to reconnecting for the next window.CONTENT_LENGTH_UNKNOWNisLong.MAX_VALUE, so the clamp ingetNextRangeUrl()does not fire either. Only this change is needed.Effect downstream
youtube-sourcecurrently carries a workaround for this (lavalink-devs/youtube-source#235): it issues aRange: bytes=0-0before streaming purely to learn the total. Its maintainer merged that with the reservation that a request no real client makes gives YouTube one more thing to fingerprint. With this, that request can be dropped.The stock-lavaplayer rows above are the current behaviour, i.e. a track whose last ~7 to ~12 seconds are silently lost while the player reports it as finished.