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

restrict outstream w/o renderer to PBS #3881

Merged
merged 5 commits into from
Jul 12, 2019
Merged

Conversation

sumit116
Copy link
Contributor

@sumit116 sumit116 commented Jun 4, 2019

Type of change

  • Bugfix
  • Feature
  • New 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

  • test parameters for validating bids
[{
      'code': 'div-gpt-ad-1460505748561-0',
      'sizes': [640, 480],
      'mediaTypes': {
        'video': {
          playerSize: [[ 640, 480 ]],
          context: 'outstream',
          mimes: ['video/mp4']
        },
        banner: { sizes: [[300, 250]] }
      },
      'transactionId': '4ef956ad-fd83-406d-bd35-e4bb786ab86c',
      'bids': [
        {
          'bid_id': '123',
          'bidder': 'appnexus',
          'params': { 'placementId': '12349520' }
        }
      ]
    },
    {
      code: 'video1',
      mediaTypes: {
        video: {
          playerSize: [640, 480],
          context: 'outstream',
          mimes: ['video/mp4']
        }
      },
      bids: [
          {
              bidder: 'appnexus',
              params: {
                  placementId: 13232385,
                  video: {
                      skippable: true,
                      playback_method: ['auto_play_sound_off']
                  }
              }
          }
      ],
      renderer: {
        url: 'http://cdn.adnxs.com/renderer/video/ANOutstreamVideo.js',
        render: function (bid) {
            ANOutstreamVideo.renderAd({
                targetId: bid.adUnitCode,
                adResponse: bid.adResponse,
            });
        }
      }
    }
  ];

Be sure to test the integration with your adserver using the Hello World sample page.

Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I setup an outstream video bid and sent it to PBS, the ORTB request still went out and returned with a 400 Bad Request response because of the following error:

Invalid request: request.imp[0] must contain at least one of "banner", "video", "audio", or "native"

If there are no other valid bids to make, then the request shouldn't go out and prebid should return some type of error message indicating that the request for that adunit was rejected.

@stale
Copy link

stale bot commented Jun 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 24, 2019
@stale stale bot closed this Jul 1, 2019
@sumit116 sumit116 reopened this Jul 4, 2019
@stale stale bot removed the stale label Jul 4, 2019
@sumit116
Copy link
Contributor Author

sumit116 commented Jul 5, 2019

@jsnellbaker ,
The below adResponse object is required to pass in the renderer.render() to display an outstream video add through Prebid Server.

bid.adResponse = {
	"ad": {
		"video": {
			content: bid.vastXml,
                        player_height: bid.playerHeight,
                        player_width: bid.playerWidth
		}
	}
}

So, now the renderer object would look like:

renderer: {
                 url: 'http://cdn.adnxs.com/renderer/video/ANOutstreamVideo.js',
                 render: function (bid) {
                    adResponse = {
                        ad: {
                            video: {
                                content: bid.vastXml,
                                player_height: bid.playerHeight,
                                player_width: bid.playerWidth
                            }
                        }
                    }
                    bid.renderer.push(() => {
                        ANOutstreamVideo.renderAd({
                            targetId: bid.adUnitCode, // target div id to render video
                            adResponse: adResponse
                        });
                    });
                  }
              }

Can you please review this PR now?

Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sumit116 I think these updates look good.

Can you put together a docs PR for the updated sample adUnit render code (for when serving from PBS)? The additions would be helpful to note down.

@jsnellbaker jsnellbaker added needs 2nd review Core module updates require two approvals from the core team needs docs and removed needs review labels Jul 10, 2019
@sumit116
Copy link
Contributor Author

@sumit116 I think these updates look good.

Can you put together a docs PR for the updated sample adUnit render code (for when serving from PBS)? The additions would be helpful to note down.

Thanks @jsnellbaker . Docs PR added: prebid/prebid.github.io#1385.

@jaiminpanchal27 jaiminpanchal27 added LGTM and removed needs 2nd review Core module updates require two approvals from the core team labels Jul 11, 2019
@sumit116 sumit116 merged commit d17e437 into master Jul 12, 2019
@spormeon
Copy link

how do you get this "renderer" thing to work with multi boxes? the docs are non existant and the examples dont show how to do it?

@sumit116
Copy link
Contributor Author

boxes

@jsnellbaker , do you have any idea about this?

@jsnellbaker
Copy link
Collaborator

@spormeon Can you please clarify a little further on what you mean?

@spormeon
Copy link

how do you get a renderer to work with "multi boxes" it just doesnt seem to work?

@jsnellbaker
Copy link
Collaborator

@spormeon Do you have a page setup where you're trying to get this to work that I can see? I don't understand the term 'multi boxes' in this context. Do you mean just having multiple outstream video placements serving together?

@spormeon
Copy link

spormeon commented Jul 25, 2019

prebid "multi-format" my test pages are behind login, so unless i can Pm you its hard to give you it
http://prebid.org/dev-docs/show-multi-format-ads.html

no mention of a 'renderer" anywhere or how to add one and throw in some bidders being S2S and got a mess, with no docs or info on how to do it?

leonardlabat pushed a commit to criteo-forks/Prebid.js that referenced this pull request Jul 30, 2019
* restrict outstream w/o renderer to PBS

* reject Request to Prebid Server for invalid media types

* add player height and width to response
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 1, 2019
* restrict outstream w/o renderer to PBS

* reject Request to Prebid Server for invalid media types

* add player height and width to response
sa1omon pushed a commit to gamoshi/Prebid.js that referenced this pull request Nov 28, 2019
* restrict outstream w/o renderer to PBS

* reject Request to Prebid Server for invalid media types

* add player height and width to response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants