Multiple dimensions for video requests

[marcelomalcher]

Hi @opinali,

While using the DoubleClickOpenRtbMapper and testing it with Video requests, I notice that the mapper is not creating Video impressions.

I believe the reason is the following code:

    if (dcSlot.getWidthCount() == 1) {
      video.setW(dcSlot.getWidth(0));
      video.setH(dcSlot.getHeight(0));
    } else if (dcSlot.getWidthCount() > 1) {
      if (interstitial) {
        video.setW(dcSlot.getWidth(0));
        video.setH(dcSlot.getHeight(0));
      } else {
        logger.debug("Invalid Video, non-interstitial with multiple sizes");
        return null;
      }
    }

In that code above, the mapper is returning null when it is an interstitial request and has multiple-dimensions.

After enabling debug level in the mapper class, I could see several the messages:

...
[DEBUG] 2015-11-04 00:33:31,987 DoubleClickOpenRtbMapper - Invalid Video, non-interstitial with multiple sizes
 [DEBUG] 2015-11-04 00:33:31,988 DoubleClickOpenRtbMapper - Request has no impressions
 [DEBUG] 2015-11-04 00:33:31,988 DoubleClickOpenRtbMapper - Request has no impressions
 [DEBUG] 2015-11-04 00:33:34,561 DoubleClickOpenRtbMapper - Invalid Video, non-interstitial with multiple sizes
 [DEBUG] 2015-11-04 00:33:34,561 DoubleClickOpenRtbMapper - Invalid Video, non-interstitial with multiple sizes
 [DEBUG] 2015-11-04 00:33:34,561 DoubleClickOpenRtbMapper - Request has no impressions
 [DEBUG] 2015-11-04 00:33:34,561 DoubleClickOpenRtbMapper - Request has no impressions
...

So, for my surprise, it seems that AdX is only sending video requests to me that are both interstitial requests and has multiple dimensions - thus the mapper is always returning null and finishing the translation to OpenRtb with no impressions objects.

Wondering if this is the right approach I went to DoubleClick spec, where it can be read:

... // For mobile interstitial ads (including ones where video ads are eligible) // the first width height pair is the screen size (this is also the video // player size for VAST video ads). Subsequent pairs are recommended // interstitial ad sizes that satisfy the interstitial size restrictions, // i.e. no bigger than the screen size and no smaller than 50% of width and // 40% height. repeated int32 width = 2; repeated int32 height = 3;

So, from my understanding, it is possible to have video interstitial requests with multiple dimensions. In that case, we should get the first width and height as they should be the video player size.

Therefore, I would propose the following change:

    if (dcSlot.getWidthCount() > 0) {
      video.setW(dcSlot.getWidth(0));
      video.setH(dcSlot.getHeight(0));
    } else {
      logger.debug("Invalid Video, dimension of the video player not defined");
      return null;
    }

And no need for interstitial argument in the buildVideo method...

What do you think? Am I missing something?

PS: I did not open a PR as I read in the CONTRIBUTING file "...It's generally best to start by opening a new issue describing the bug or feature you're intending to fix..."

[opinali]

You are right, the problem is that OpenRTB doesn't support multiple sizes for video (not even with min/max ranges like Banner), but we should have just picked the size [0] for non-interstitials. I'm fixing this right now.

[marcelomalcher]

That's great! Thanks so much! :+1:

[opinali]

Just commit the fix, please test, close this bug if it's OK :)

[opinali]

(fix on master here, no Maven-central release yet).

[marcelomalcher]

I believe I made a mistake while explaining the issue. In fact, we are receiving only requests that are NOT interstitial and has multiple dimensions.

In the code before your commit, this condition is returning null, which is the issue we are facing.

However, while giving a look to the change you made ...

    if (dcSlot.getWidthCount() != 0) {
      if (dcSlot.getWidthCount() > 1 && interstitial) {
        logger.debug("Invalid Video, non-interstitial with multiple sizes");
        return null;
      } else {
        video.setW(dcSlot.getWidth(0));
        video.setH(dcSlot.getHeight(0));
      }
    }

... I notice two things:

• Are you sure we cannot have video requests that are interstitial and have multiple dimensions? Why not just picking the first width and height? :)
• If you are right about interstitial requests having only one dimension defined, the debug message should be changed to "Invalid Video, interstitial with multiple sizes".

Well, like I said before, I would really just pick the first width and height from the dimension list, not caring if there are multiple ones.

What do you think?

[opinali]

First, it was a bad idea to rush this code to you before running functional tests (the unit tests in this project are just the tip of the iceberg... can't add the full tests for AdX integration here because that requires a whole bidder). I had inverted the interstitial condition, which also made the debug text look wrong. Fixed now.

So the rules is that only interstitials can be multisize, with size 0 being the player size. BTW you may find some AdX callouts that violate that rule, but this was also a bug, fixed last week but should only hit production next week or so; anyway I prefer to have this double-check here.

[marcelomalcher]

Got it! So, the current version is the correct one.

Like I said to you, every AdX video request that we are receiving is coming as non-interstitial and with multiple dimensions, thus, violating the rule you said.

Well, thanks so much for the help! I will wait this fix to continue my tests with video requests.

Closing the issue!

[opinali]

On a second thought, handling this as a fatal error was always heavy-handed. I made a final change so if multiple sizes are found when not expected, the mapper will only log but not reject.