-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[interactive_media_ads] Adds support for mid-roll ads #7407
Conversation
@@ -79,13 +80,18 @@ base class AndroidAdsLoader extends PlatformAdsLoader { | |||
} | |||
|
|||
@override | |||
Future<void> requestAds(AdsRequest request) async { | |||
Future<void> requestAds(PlatformAdsRequest request) async { |
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.
The platform implementations haven't been exposed yet, so this shouldn't be a breaking change.
|
||
if (_adsManager != null) { | ||
_contentProgressTimer = Timer.periodic( | ||
const Duration(milliseconds: 500), |
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.
@harold1208 @stuartmorgan What are your thoughts on the interval to be used here in the example? This works for me on Android and iOS, but this is a Flutter specific workaround to handle the SDK running on a separate thread.
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.
For context, this timer updates the SDK of the progress of the content video with this being the interval between updates.
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.
Per our offline discussion with the IMA team, a 200ms interval would be the ideal value.
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.
Per our offline discussion with the IMA team, a 200ms interval would be the ideal value to poll content progress.
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.
LGTM once the 500->200 change is made.
flutter/packages@4f2b9cd...c7406b3 2024-09-17 [email protected] [interactive_media_ads] Adds support for mid-roll ads (flutter/packages#7407) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Adds
ContentProgressProvider
and addsAdsRequest.contentProgressProvider
field.This changes the platform interface
AdsRequest
toPlatformAdsRequest
, so thePlatformContentProgressProvider
can be passed to it. And the app-facingAdsRequest
can take aContentProgressProver
.Fixes flutter/flutter#154261
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.