Thumbnails: Cache pending thumbnail request #1718
Merged
Conversation
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
peaBerberian
commented
Jul 24, 2025
| // load thumbnail after a timer to avoid doing too many requests when the | ||
| // user quickly moves its pointer or whatever is calling this | ||
| loadThumbnailTimeout = window.setTimeout(() => { | ||
| loadThumbnailTimeout = null; |
There was a problem hiding this comment.
Here I just moved the setTimeout so it only applies to when the thumbnail is loaded through the VideoThumbnailLoader
peaBerberian
commented
Jul 24, 2025
| cmcdPayload: undefined, | ||
| }; | ||
|
|
||
| log.debug("TF: Beginning thumbnail request", thumbnail.time); |
There was a problem hiding this comment.
Everything here moved to the new doFetch inner function
peaBerberian
force-pushed
the
misc/store-pending-thumbnail-request
branch
2 times, most recently
from
8831149 to
394b54b
Compare
peaBerberian
force-pushed
the
misc/store-pending-thumbnail-request
branch
from
394b54b to
68e8549
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
peaBerberian
force-pushed
the
misc/store-pending-thumbnail-request
branch
from
68e8549 to
3468357
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
When I demoed how DASH thumbnail worked in the RxPlayer demo, I was unhappy to realize that there was a noticeable delay when quickly moving the mouse over the seekbar. This is both because we have a timer in our demo of a few milliseconds before doing the actual thumbnail request, and even much more importantly because the RxPlayer cancel the last thumbnail request when the application decides to fetch one from a new position instead. Yet positions that are sufficiently close (which is frequent when moving the mouse over a seekbar) are often going to lead to the same thumbnail request. Here cancelling the last one is unfortunate: we should reuse the same request. --- We already have a thumbnail cache but sadly enough it is only populated once the request is done. If the request is still pending and an application ask a thumbnail for a new position, we're going to run a new request even if it leads to the same thumbnail. Our "last loaded" cache was in main thread, which does not even know the relation between the wanted time and the corresponding thumbnail, so this new logic cannot be added there. Instead I added another level of "cache" in our `ThumbnailFetcher` (in `core/fetchers`) for the pending request only. The result is much more satisfying.
peaBerberian
force-pushed
the
misc/store-pending-thumbnail-request
branch
from
3468357 to
5c5b194
Compare
Florent-Bouisset
approved these changes
Jul 31, 2025
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When I demoed how DASH thumbnails worked in the RxPlayer demo, I was unhappy to realize that there was a noticeable delay when quickly moving the mouse over the seekbar.
thumb-before.mp4
Video: how thumbnails displayed before in our demo page. It's slow and, especially for
tile5, we can see that it's requested multiple times.This is both because we have a timer in our demo of a few milliseconds before doing the actual thumbnail request, and even much more importantly because the RxPlayer cancel the last thumbnail request when the application decides to fetch one from a new position instead.
Yet positions that are sufficiently close (which is frequent when moving the mouse over a seekbar) are often going to lead to the same thumbnail request. Here cancelling the last one is unfortunate: we should reuse the same request.
We already have a thumbnail cache but sadly enough it is only populated once the request is done. If the request is still pending and an application ask a thumbnail for a new position, we're going to run a new request even if it leads to the same thumbnail.
Our "last loaded" cache was in main thread, which does not even know the relation between the wanted time and the corresponding thumbnail, so this new logic cannot be added there.
Instead I added another level of "cache" in our
ThumbnailFetcher(incore/fetchers) for the pending request only. The result is much more satisfying:after.mp4