Revert "Recover the total length for formats that omit contentLength (#235)" - #237
Revert "Recover the total length for formats that omit contentLength (#235)"#237lpaiu-cs wants to merge 1 commit into
Conversation
…avalink-devs#235)" This reverts commit f45bbb7. The probe compensated for the stream adopting a partial response's Content-Length as the length of the resource, which is fixed in lavaplayer instead. With that fix the length simply stays unknown and the existing range handling carries the stream to the end, so the extra request is no longer buying anything.
|
Ran into something on my stack that might be worth adding here, missing Lavalink 4.2.2, lavaplayer 2.2.6 youtube-source b33460b, clients Three unique tracks with a total of five track starts
Row 5 is the control: same client, same itag 18, longest of the three, no restart, because that URL carried a Appears as silent with no TrackException, no TrackEnd, and the reported position keeps advancing past the audio before snapping backwards when it updates. Watching the connections across a restart shows the media connected to one googlevideo host disappear, a brief request to a different Google host, then a new media connection to a different IP, which was a fresh resolve rather than a resumed range request. Seeking forward after the track starts seems to avoid re-resolve entirely. A track that normally restarts when played straight through will play to the end if the position is moved manually early on, including seeking to before the point where it would otherwise break and allowing it to playthrough. I couldn't characterize the break point in terms of time, and #235's description of stopping at the first range boundary may be why, since a byte offset would land at a different fraction of every file, which fits the ~75% and ~82% above against the 1.6% in lavaplayer#198. Converting mine to bytes isn't possible from this side, since the affected URLs are the ones with no Pinning f45bbb7 seems to have fixed it on the tracks that reproduced it reliably. #234 describes the same missing-length condition as losing only the final seconds. |
…ot semver ordering
Reverts #235, which I opened and which you merged with the reservation that the extra request gives YouTube one more thing to fingerprint. That reservation was right, and the request turns out not to be necessary.
Why it is no longer needed
The probe existed because the stream ended at the first range boundary when a format arrived without a
contentLength. The cause is in lavaplayer rather than here:PersistentHttpStream.attemptConnectfills in an unknown length from the response'sContent-Length, and sinceYoutubePersistentHttpStreamcarries its range in the query, that response is a plain200whoseContent-Lengthdescribes the window rather than the resource. The unknown marker is replaced by one window's worth on the first connect, and the stream stops there.lavalink-devs/lavaplayer#198 leaves the length alone when the request was narrowed by the subclass. Nothing is needed here: with the length left unknown,
CONTENT_LENGTH_UNKNOWNbeingLong.MAX_VALUEmeans the clamp ingetNextRangeUrl()does not fire, andinternalRead'srangeEnd == contentLengthcheck falls through to reconnecting for the next window, which is what carries the stream to the end.Measured
Lavalink 4.2.2 built against a lavaplayer carrying that fix, running this revert. A 298s track played 297s and ended
finished; it was losing 7s before. NoMpegAacTrackConsumerEOF in the window, and no probe request anywhere.The range sequence shows the difference:
With #235 in place the same track ended
range=12026824-12164812, clamped to a total the probe had fetched.Sequencing
This should not be merged before lavaplayer#198 lands and Lavalink ships a build carrying it. Until then, this repo running against a released lavaplayer would lose the last seconds of any track that falls back to itag 18 — which is what #235 was fixing. Raising it now so the two can be lined up; happy for it to sit until the other side is in, or to close it and re-open later if that is tidier.