Video Module: Local cache for video bids #12502
Conversation
|
@mkomorski how do the blobs go away over time? Is there a risk of running out of storage? |
@patmmccann They just get cleaned up when document scope is gone, so they will live till the user close the browser/tab. I don't think there is a limited memory pool dedicated for blobs so it's most likely matter of browser memory. But it can actually be a bit more efficient to also explicitly revoke not needed ones (lost bids) when auction ends up. good call |
|
Maybe when the ttl expires instead? They might enter the cache |
| video:{"context":"outstream"} | ||
| video:{ | ||
| "context":"outstream", | ||
| playerSize: [640, 360] |
There was a problem hiding this comment.
playerSize shouldn't be required; the Video Module should be populating this, not the publisher. The Video Module is the source of truth.
There was a problem hiding this comment.
@mkomorski why was playerSize in all the demos, this should be populated by the Video Module already
There was a problem hiding this comment.
@karimMourra so it seems that it's not populated for some reason, at least in all the demos. without the publisher explicitly setting playerSize to adUnit, none of the examples works. I'm not very familiar with the details of the video module, but maybe this means there is some issue? Here's the example that I was basing on - Removing playerSize from this example's code breaks it as well
There was a problem hiding this comment.
@mkomorski in the case of the JW Player examples, the auction happens before the player is fully instantiated. So the player's exact size isn't available. In this case, for JW Player I will add logic to calculate the size based on values in the config. Please remove the changes to the JW Player examples.
Are you a maintainer of Ad Player Pro ?
| const { url, useLocal } = config.getConfig('cache') || {}; | ||
| if ((!url && !useLocal) && bid.vastXml && !bid.vastUrl) { |
There was a problem hiding this comment.
is there ever a case where url would be defined and useLocal would be set to true ? Do we need a safeguard against this ?
| if (!config.getConfig('cache.useLocal')) { | ||
| url.searchParams.append(UUID_MARKER, uuid); | ||
| } |
There was a problem hiding this comment.
why wouldn't we append the UUID Marker when the url is local ? The idea is to know that the ad from the bid actually rendered. This is most importantly for when the bid competes in the ad server auction, but without a marker we'll never know if the ad from the bid actually rendered.
There was a problem hiding this comment.
I don't think that local Blob URLs can be modified in any way, adding query parameters to it will make it no longer point to the current blob and return 404. We might think of some alternative for handling this tracking in case of using local cache
Co-authored-by: Karim Mourra <[email protected]>
Co-authored-by: Karim Mourra <[email protected]>
|
Tread carefully! This PR adds 3 linter errors (possibly disabled through directives):
|
1 similar comment
|
Tread carefully! This PR adds 3 linter errors (possibly disabled through directives):
|
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
Other information
#11393