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

Android banner video request does not send width and height #768

Closed
shinwan2 opened this issue Jun 14, 2024 · 3 comments · Fixed by #775
Closed

Android banner video request does not send width and height #768

shinwan2 opened this issue Jun 14, 2024 · 3 comments · Fixed by #775
Assignees

Comments

@shinwan2
Copy link
Contributor

shinwan2 commented Jun 14, 2024

Describe the bug

In Android, with the following code

val adSize = AdSize.BANNER
val bannerAdUnit = BannerAdUnit(
    /* configId = */ requestId,
    /* width = */ adSize.width,
    /* height = */ adSize.height,
    /* adUnitFormats = */ EnumSet.of(
        AdUnitFormat.BANNER,
        AdUnitFormat.VIDEO
    )
)
bannerAdUnit.videoParameters = VideoParameters(…).apply {
    api = …
    // omitted for brevity
    // does not explicitly set videoParameters adSize
}

Below JSON request is sent for the imp.video

"video": {
  … // no width and height info.
  "api": [
    1,
    2
  ]
},

While in iOS

let bannerAdUnit = BannerAdUnit(configId: unitId, size: CGSize(width: 300, height: 250))
bannerAdUnit.adFormats = [.banner, .video]
bannerAdUnit.videoParameters = {
    let videoParameters = VideoParameters(mimes: …)
    videoParameters.api = …
    // does not set videoParameters.adSize
    return videoParameters
}()

the imp.video sends w and h

"video": {
  "api": [
    1,
    2
  ],
  …
  "h": 250,
  "w": 300
}

Expected behavior
I think iOS is correct and w and h should be sent in imp.video.

Smartphone (please complete the following information):

  • Device: iPhone 13 Mini, Samsung S21
  • OS: iOS 17.5.1, Android 13
  • Browser: stock browser.
@jsligh
Copy link
Collaborator

jsligh commented Jun 20, 2024

@shinwan2 what is AdSize.BANNER? This doesn't exist in codebase of the Android SDK. If you'll notice in your iOS example, you set the width and height. You did not instantiate AdSize in your Android example which takes width and height as parameters. Width and height will be null if it is not instantiated. If you look at this class in the Android repo, you'll see that it does add width and height in lines 87-88. If you also look at the code below from another class (BasicParameterBuilder.java), you will see how the width and height is set.

        if (videoParams != null) {
            AdSize adSize = videoParams.getAdSize();
            if (adSize != null) {
                video.w = adSize.getWidth();
                video.h = adSize.getHeight();
            }
        } else if (!adConfiguration.getSizes().isEmpty()) {
            for (AdSize size : adConfiguration.getSizes()) {
                video.w = size.getWidth();
                video.h = size.getHeight();
                break;
            }
        } else if (resources != null) {
            Configuration deviceConfiguration = resources.getConfiguration();
            video.w = deviceConfiguration.screenWidthDp;
            video.h = deviceConfiguration.screenHeightDp;
        }

@shinwan2
Copy link
Contributor Author

shinwan2 commented Jun 21, 2024

@jsligh AdSize.BANNER is GAM constant defined as

String var0 = "320x50_mb";
BANNER = new AdSize(320, 50, var0);

Sorry I meant AdSize.MEDIUM_RECTANGLE with size 300x250 defined in iOS below.

/// Medium Rectangle size for the iPad (especially in a UISplitView's left pane). Typically 300x250.
FOUNDATION_EXPORT GADAdSize const GADAdSizeMediumRectangle;

I did set the size for both Android and iOS. Let's inline the size

AndroidiOS
val bannerAdUnit = BannerAdUnit(
    /* configId = */ requestId,
    /* width = */ 300,
    /* height = */ 250,
    /* adUnitFormats = */ EnumSet.of(
        AdUnitFormat.BANNER,
        AdUnitFormat.VIDEO
    )
)
let bannerAdUnit = BannerAdUnit(
    configId: unitId, 
    size: CGSize(width: 300, height: 250)
)

Yes in iOS, if I'm correct, the request width and height is set by these lines.

if (formats.count) {
    PBMORTBFormat * const primarySize = (PBMORTBFormat *)formats[0];
    nextVideo.w = primarySize.w;
    nextVideo.h = primarySize.h;
}

Which is only equivalent to 1 else if block below from your snippet while the if (videoParams != null) in iOS does not exist -> causing the discrepancy.

} else if (!adConfiguration.getSizes().isEmpty()) {
    for (AdSize size : adConfiguration.getSizes()) {
        video.w = size.getWidth();
        video.h = size.getHeight();
        break;
    }

Proposed solution

Sorry it's in Kotlin for conciseness.

func getAdSize(): AdSize? {
    return videoParams?.getAdSize() ?: adConfiguration.getSizes().firstOrNull() 
}

val adSize = getAdSize()
if (adSize != null) {
    video.w = adSize.getWidth()
    video.h = adSize.getHeight()
} else if (resources != null) { … }

@jsligh
Copy link
Collaborator

jsligh commented Jun 21, 2024

@shinwan2 Thank you so much for your clarification and proposed solution. Apologies, I did not remember that AdSize.BANNER was a GAM constant. I will test and implement the proposed changes / fixes. Again, thank you so much for your comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment