Skip to content
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

Rubicon video support #950

Merged

Conversation

snapwich
Copy link
Collaborator

Type of change

  • Feature

Description of change

Adds video support for the Rubicon adapter. An example adUnit for displaying a video ad should look something like this:

        var videoAdUnit = {
            code: 'example_video_1',
            sizes: [[640,320]],
            mediaType: 'video',
            bids: [
                {
                    bidder: 'rubicon',
                    params: {
                        accountId: "7780",
                        siteId: "87184",
                        zoneId: "412394",
                        userId: "12346",
                        keywords: ["nhl","hockey","goalie"],
                        inventory: {rating:["4-star"], prodtype:["sports"]},
                        visitor: {ucat: ["new"], lastsearch:["hockey"]},
                        position: "atf",
                        video: {

                          'language': 'en',

                          'playerHeight': 320,

                          'playerWidth': 640,

                          'size_id': 201,
                          'aeParams': {"p_aso.video.ext.skip": "1","p_aso.video.ext.skipdelay": "15"}

                        }
                    }
                }
            ]
        };

@snapwich
Copy link
Collaborator Author

I added a small commit to also include the versioning for our adapter requests.

@jaiminpanchal27
Copy link
Collaborator

@snapwich currently i am getting swf mediafile in vast response. Can you guys configure it to send any mp4 mediafile.
Actually the test page which i have is only configured for html5.

@dbridges12
Copy link

@jaiminpanchal27 can you send me one of the VASTUrls you are receiving as well as your test page which has the adunit you've defined for the test and I'll work through it with you. We can take this offline if you want.

@jaiminpanchal27
Copy link
Collaborator

jaiminpanchal27 commented Jan 31, 2017

@dbridges12 reach me on jpanchal@appnexus.com

@jaiminpanchal27
Copy link
Collaborator

jaiminpanchal27 commented Feb 2, 2017

@dbridges12 Please update with the fix for description_url

@dbridges12
Copy link

dbridges12 commented Feb 2, 2017 via email

@mkendall07
Copy link
Member

@snapwich @dbridges12
Thanks for making updates to the code. Is the method with the redirect in the DFP URL acceptable for now? Ideally this wouldn't exist, but we implemented that way to due to limitations in DFP.

@dbridges12
Copy link

@mkendall07 - we worked around the redirect by passing our impression id in the description_url field and using a macro to insert it into our main site - that way dfp returns a wrapper with our site directly embedded.

@mkendall07
Copy link
Member

mkendall07 commented Feb 2, 2017

Ah right. Ok I would like to propose a change.
Since adding impression_id to bid.vastUrl feels kind of ugly, I would like to instead use a new field such as bid.descriptionUrl that can be populated with a VAST URL or an impression ID or anything (up to the Bidder's implementation). This field would then be used in the pbjs.buildMasterVideoTagFromAdserverTag to construct the macro.

Thoughts?
@snapwich
@dbridges12
@jaiminpanchal27

Edit: we'd need to update appnexusAst if we make this change.

@dbridges12
Copy link

@mkendall07 - I'm all for it. Let me know when your ready.

@jaiminpanchal27
Copy link
Collaborator

@dbridges12 one of the test is failing for your spec. Check travis failed build.

@dbridges12
Copy link

We caught the failed test - waiting to make the change matt suggested before resubmitting

@mkendall07
Copy link
Member

Cool. @jaiminpanchal27 can you put up a PR with those changes for core + appnexus?

@mkendall07
Copy link
Member

Can rebase off master now with the change.

@dbridges12
Copy link

Latest commit incorporates the bid.descriptionUrl field in the rubicon adaptor.

Also fixes a small bug in the adserver.js file where the correlator was not being passed in the arguments to DFP.

@jaiminpanchal27
Copy link
Collaborator

Looks good. Merging.

@jaiminpanchal27 jaiminpanchal27 merged commit 8dc5a88 into prebid:master Feb 6, 2017
@snapwich snapwich deleted the rubicon-video-support branch February 6, 2017 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants