-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Outstream Video Support #1082
Outstream Video Support #1082
Conversation
aad0409
to
367245b
Compare
@mkendall07 steel thread version for review |
3056648
to
fc14fec
Compare
src/prebid.js
Outdated
}; | ||
|
||
function performRenderViaRenderer(doc, adObject) { | ||
window.apntag = { debug: true }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic is specific to appnexus, we should move it into the adapter or into a renderer interface type.
src/prebid.js
Outdated
// use renderer defined by creative or default to ANOutstreamVideo.js | ||
loadScript( | ||
adObject.rendererUrl || | ||
'http://cdn.adnxs.com/renderer/video/ANOutstreamVideo.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make protocoless to support https
bidder: 'appnexusAst', | ||
params: { | ||
placementId: '5768085', | ||
video: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could move these params up to the adunit level?
src/prebid.js
Outdated
var url = adObject.adUrl; | ||
var ad = adObject.ad; | ||
const { height, width, ad, mediaType } = adObject; | ||
const url = adObject.adUrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be included in the destructuring above
src/prebid.js
Outdated
utils.logError(`Error trying to write ad. Ad render call ad id ${id} was prevented from writing to the main document.`); | ||
} else if (mediaType === 'video' || mediaType === 'video-outstream') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to render a pure video
response yet, since that requires a video player
727df64
to
b24f061
Compare
@@ -0,0 +1,180 @@ | |||
<!doctype html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop this file.
src/Renderer.js
Outdated
@@ -0,0 +1,37 @@ | |||
import { loadScript } from 'src/adloader'; | |||
const utils = require('src/utils'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why not import?
src/adapters/appnexusAst.js
Outdated
tagId: bid.adResponse.tag_id, | ||
sizes: [bid.getSize().split('x')], | ||
targetId: bid.adUnitCode, // target div id to render video | ||
uuid: bid.adResponse.uuid, // is this the correct UUID ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should work fine.
src/adapters/appnexusAst.js
Outdated
uuid: bid.adResponse.uuid, // is this the correct UUID ? | ||
adResponse: bid.adResponse, | ||
rendererOptions: { | ||
adText: 'Prebid Outstream Video Ad' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might want to default this to empty text.
src/adapters/appnexusAst.js
Outdated
rendererOptions: { | ||
adText: 'Prebid Outstream Video Ad' | ||
} | ||
}, (id, eventName) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is id
here uuid
? If so, maybe we should make this adUnit.code
instead.
src/adapters/appnexusAst.js
Outdated
}); | ||
}); | ||
bid.renderer.setEventHandlers({ | ||
impression: () => utils.logMessage('AppNexus outstream video impression event'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who's firing these events?
Looks great! Just a few minor changes/suggestions |
@protonate looks like there is a conflict. |
initial appnexus outstream renderer adapter
RAD-1439
cache loadScript for renderer Renderer.install returns existing instance of renderer, if available
a7364b4
to
2d7c7db
Compare
LGTM. |
@@ -225,6 +227,26 @@ function AppnexusAstAdapter() { | |||
return tag && tag.ads && tag.ads.length && tag.ads.find(ad => ad.rtb); | |||
} | |||
|
|||
function outstreamRender(bid) { | |||
window.ANOutstreamVideo.renderAd({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exception would be thrown here if window.ANOutstreamVideo
isn't defined but should be caught in renderAd
so I guess it's alright for now.
LGTM |
Hey @mkendall07 quick question: If you want to render outstream without DFP just straight by calling and rendering on the page, I took the example from postbid, modifying it to video, then getting bids returned, but don't get the ad to render. Not sure if this works for outstream in the same fashion as for display using the iFrame. Appreciate a short note.
|
* add renderer adapter type initial appnexus outstream renderer adapter * render outstream response from appnexus * trim back to steel thread * turn off istanbul * fix adUrl, move mediaType logical test * clean up * load renderer in renderAd * clean up * fix loadScript in render RAD-1439 * collapse divs on render * add Renderer class, remove render functions from prebid.js file * wip * add API to Renderer to set render function and event handlers cache loadScript for renderer Renderer.install returns existing instance of renderer, if available * add support for multiple video units, renderer options can be set from bid level data * example page edits * decruftification * import utils with es syntax * add tests * add tests * use .call instead of .bind, reduce auction time limit on example * use mock server for tests on Renderer.install * fix test * appease Travis * appease Travis some more * appease Travis some more again * outstream example create utility functions for readability * test link to use public endpoint * remove prebid key from example ad server targeting * reorder code in bidder * review notes * fix test
is there a full list of renderer options anywhere, i cant find it in an docs only the adText one: renderer: { options: { adText: 'Advertisement', skippable: true } }, |
Type of change
Description of change
As media formats proliferate a Renderer type is proposed. This provides a set of functions to specify, load, configure and utilize a custom 3rd party renderer for a given bid's media format.
An initial use case for Renderer is support for outstream video as shown in this change. Here we are assuming the
renderer_url
is a value returned on a Bid response object, and if present the Bid response is assumed to be an outstream video ad in need of a custom renderer.A renderer source could also be specified at the adapter or publisher level, which can be done by setting the
rendererInstance.url
value appropriately.To implement in a bidder adapter, import Renderer module:
A
renderer
can be installed on a bid response in a bidder adapter by using the staticRenderer.install
method. This takes an object with the following properties:An example looks like:
Once a renderer is installed on a bid, the renderer can set it's render function using
setRender()
:The render function will receive the bid response object and the renderer can be called with it:
For the AppNexus Outstream Video renderer, the second argument takes a function that is invoked on events. We use this to call the Renderer's
handleVideoEvent
method:A
renderer
can register event handlers using a hash with keys mapped to the renderers event names: