Skip to content

Conversation

@kaymes
Copy link
Contributor

@kaymes kaymes commented Nov 19, 2019

I noticed some ocational buffer under-runs as a consequence of PR #393 for which I made a fix here.

The issue is, that the pre-fetch algorithm can potentially fire of lots of small requests to keep the amount of pending bytes at the desired level. If bandwidth limits are hit, this leads to responses being interleaved - all requests are served simultaneously, which means the total data rate is great but each single request experiences a low data rate. A consequence can be, that the first request isn't responded to in time and we get a buffer under-run.

I solved this by limiting the amount of open requests when pre-fetching: a pre-fetch request is only sent when less than 4 requests are open. This leads to less requests which are larger. Thus, individual requests should still get decent download rates.

I experimented with different values. Using a limit of 3 leads to significantly lower overall download rates. Thus, I chose 4.

I also increased the amount of data that is requested ahead of the current read positions.

So far I haven't had any more buffer under-runs with these changes. Fingers crossed.

@kaymes
Copy link
Contributor Author

kaymes commented Dec 2, 2019

This PR has been sitting here for two weeks without getting merged or commented on. I'm just wondering whether there's anything I need to do to get this processed. Or is everyone on vacation already? :-)

@sashahilton00 sashahilton00 merged commit f67a38d into librespot-org:dev Dec 2, 2019
@sashahilton00
Copy link
Member

I've been pretty busy for the past few weeks, I guess the other maintainers have as well. Anyway, just skimmed over it, did a quick test, seems fine, merged.

@kaymes
Copy link
Contributor Author

kaymes commented Dec 2, 2019

Thank you.

ashthespy pushed a commit to ashthespy/librespot that referenced this pull request Mar 30, 2020
paulfariello pushed a commit to paulfariello/librespot that referenced this pull request Sep 23, 2025
Limit number of requests for pre-fetching
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants