Downloading files with dnamic block sizes for faster seek and playback

[kaymes]

Quite a while ago, I analysed why librespot is slow when loading files and wrote up my findings in issue #306. Back then, I also started work on a new mechanism to download the files. Unfortunately, I got busy and forgot about it and never submitted it despite it being almost finished. Until today.

Here is my work on a new downloading strategy for librespot.

The idea is, that it uses dynamic block sizes. That is, blocks are small while seeking and during initial play and grow for downloading the grunt of the file. Also, while seeking in a file, it only downloads the requested blocks whereas while streaming, it reads ahead until the entire file is downloaded and placed in the cache.

I also implemented some heuristics based on the observed round-trip (ping) times to determine how much to buffer before playback.

Please test this version of the code.

The thing I'm most worried about is the possibility of not buffering enough because I can't exhaustively test this. This must be tested with different internet connections. If you notice the playback starting and then there's a short pause after which it resumes (a buffer under run), then the ping-time-buffer-heuristic must be adapted.

[kaymes]

I just checked why the Travis-ci tests have failed. The message is not conclusive, but it seems like an issue with Travis-ci instead of the code.

[sashahilton00]

Have restarted the build, just read up on the issue, it's caused by the rust build cache getting too large, so I deleted it. It should take a while, but new cache should be smaller. Thanks for the PR. Also closes #306

[kaymes]

I don't think, this PR is ready for merge yet. I discovered some issues that cause bad behaviour.

When I tested the behaviour of the server connection back in the day, it seemed to me, that requests are always responded to in the order they are sent. (i.e. requests are processed in a FIFO manner). This means sending additional requests wouldn't hurt the one we actually need right now.
Now, it seems that requests are served in an interleaved fashion which means additional requests can slow down the current one. Not sure whether this was a change in the comms protocol or whether my initial testing was flawed, but it should be accounted for.

An even bigger issue is, that it seems like not all requests are being served in full. I had buffer underruns and I could see that the missing piece was requested long ago but only partially received and then the connection only served further pieces. Not sure why. Possible causes I can think of are:

1. an unknown bug that drops a data receiver before it's request is complete.
2. request priorisation by Spotify (other requests we send in parallel are served and the one we need is starved).
3. constraints on block sizes that spotify is willing to serve.
4. evil woodoo.

In any case, this issue causes unacceptable behaviour at the moment.

[kaymes]

Just a quick update: I found the bug that caused the erratic loading behaviour. (I mixed up seconds and milliseconds at one place leading to all pre-fetch requests being sent at once.)

Currently, I'm doing a bit of cleaning up and refining of the heuristics to such that they should perform better in more circumstances. I'm also making them easier to tune later on.

I've got a question though: librespot used to pre-load the next track once the current track is fully downloaded. This doesn't seem to happen any more (neither this PR nor the current dev branch seem to do it). Why was this feature removed? Are there plans to bring it back?

Btw: Travis-CI seems to be playing up again.

[ashthespy]

I've got a question though: librespot used to pre-load the next track once the current track is fully downloaded. This doesn't seem to happen any more (neither this PR nor the current dev branch seem to do it). Why was this feature removed? Are there plans to bring it back?

I believe you mean #263? I didn't make time to finish it, I believe there is a branch somewhere that implements a buffer POC as well, but yeah never finished it.

Btw: Travis-CI seems to be playing up again.

Cleared the cache out, so its all good for a few builds..

[kaymes]

I've now got everything in this PR that I wanted to have in there at this stage. So I guess, it's time for other people to test it as well and give some feedback.

The code currently produces lots of trace!() messages which I will remove before this PR gets merged. But during development and debugging, they really help to understand what is going on.

What I did:
I beefed up how fetch.rs downloads files. It can now request arbitrary data ranges and handle multiple range requests at once. It can also serve partially downloaded ranges to the reader process. It is much more sophisticated when it comes to pre-fetching and takes into account the data rate of the stream, ping time and bandwidth of the connection.

The fetch heuristics are controlled by a bunch of constants near the top of fetch.rs. The comments explain what the various settings do, so it should be easy to tune this if problems with the tuning come up later down the track.

The prefetch and read ahead rules seem over-engineered when you look at the number of constants there. However, they were the easy part. The grunt of the code was for the infrastructure. Once that was in place, putting in a few formulas here and there was the easy part. I guess, for people with lots of bandwidth and small ping times it hardly matters what we do anyway. However, I tried to also think of people with small bandwidth and long ping times.

How it behaves with this PR:
When playing from the beginning, playback should start after a single round trip. The initial range request asks for enough data to satisfy the read-ahead constraints and the player is started. While reading the stream, a read-ahead range dependent on ping time and stream data rate is always maintained (requests being sent). In addition, there are pre-fetch requests that attempt to download the entire file. The rate of those is dependent on the measured bandwidth and ping time. They are paced to download as fast as we can while not having too many open requests so when the user does a seek, the incoming data dies down quickly to free the line for seek related requests.

While seeking, only small chunks of the file are requested as needed and no pre-fetch occurs. Thus, the line is free to respond to those requests as quickly as possible so the time needed to seek is primarily dependent on the ping time. Once the correct position is found, the fetcher starts with all the read ahead and pre-fetch business as before.

I'm doing some calculations based on the data rate of the stream. I'm aware that this is not technically correct since we have variable bitrates. So I'm always giving a bit of a margin to have some more data than we nominally need. I don't think, there's a better way. I reckon, it is better to use the nominal bitrate than to treat all streams the same.

What's next:
I think, this PR maxes out what we can do at the moment with getting playback start time down. The official client does seek so quickly, it must have additional information from somewhere. But as long as we don't know where it gets this from, I don't think we can speed up seeking any further.

What I'd like to see next is pre-fetching the next track as a step towards gap-less playback. I'm thinking, that this could even be implemented such, that the beginning of the next track is loaded very early on. But only the beginning. This doesn't waste much bandwidth, but at the same time is enough to start the track immediately if the user presses "next".

But I guess, these changes are best made in another PR down the track and in conjunction with the work on PR #263 .

[kaymes]

Looks like older versions of Rust don't like Duration::as_millis(). That's why the Travis check failed. I'll make a workaround for this.

[sashahilton00]

Don't bother, just kick the minimum version up to 1.33. Rust is on 1.39 atm, so we're due a version bump anyway.

[kaymes]

@sashahilton00 How do I kick up the minimum version?

[sashahilton00]

edit .travis.yml and change the entry with 1.32.0 to 1.33.0

[sashahilton00]

Have been testing this for 24 hours or so, seeking is noticeably improved. If there are no objections, will merge this tomorrow.

[kingosticks]

So, has this been tested with a slow connection also? I've been away, I've not had a chance to look at the changes yet.

[sashahilton00]

So, has this been tested with a slow connection also? I've been away, I've not had a chance to look at the changes yet.

I have fiber, so haven't been able to test on slow speed, but seeking was noticeably better, presumably because it's no longer loading/decrypting a bunch of useless chunks.

[kaymes]

I'm on 17000Kbps ADSL with a ping time to gae2-accesspoint-a-w908.ap.spotify.com of 160-200 ms. Not exactly fibre, but not really slow either.

@sashahilton00: hold off the merge a little bit. I'll make a change to remove a bunch of debug messages. At the moment it is a bit too chatty when you enable trace!() messages.

[sashahilton00]

Will do, I only tested with the debug log level, so it was just a stream of messages anyway.

…

-- Sent from Canary (https://canarymail.io)

On Sunday, Nov 10, 2019 at 4:24 am, kaymes ***@***.*** ***@***.***)> wrote: I'm on 17000Kbps ADSL with a ping time to gae2-accesspoint-a-w908.ap.spotify.com of 160-200 ms. Not exactly fibre, but not really slow either. @sashahilton00 (https://github.com/sashahilton00): hold off the merge a little bit. I'll make a change to remove a bunch of debug messages. At the moment it is a bit too chatty when you enable trace!() messages. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (#393?email_source=notifications&email_token=AA752EUPXU3G3HJP7XPPIPDQS55GPA5CNFSM4JIBOT4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDUULTQ#issuecomment-552158670), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AA752EXWTEYGQM4S3GSDCB3QS55GPANCNFSM4JIBOT4A).

[kaymes]

I removed the superfluous trace! messages. I added debug! messages that only show up when the player thread is actually waiting for file content to be downloaded while it is streaming. This is usually the case at the beginning of a track, but also during a buffer-underrun.

@sashahilton00 With these changes this PR is ready to be merged (as far as I'm concerned).

[sashahilton00]

Thanks for this @kaymes. Merged.