From e0110a5776f1a4acc76c48c99b03b59d8b026f1c Mon Sep 17 00:00:00 2001 From: Gal Orlanczyk Date: Mon, 24 Apr 2017 11:41:38 +0300 Subject: [PATCH] FEM-1261 (#144) * #FEM-1260 * Added timeout handling for `IMAPlugin`. * Added state machine for `IMAPlugin` for better handling of state changes and readability. * Renamed `AdsConfig` to `IMAConfig` * Added `enableDebugMode` for `IMAConfig` for better debug handling in IMA. * * Fixed issue with IMAPlugin * Renamed 3 properties in `PKAdInfo` according to their naming in IMA SDK to be aligned. * #FEM-1261 * Added delayed prepare for ads player decorator. * Added discard checks for IMAPlugin ads playing to know when to discard ads. --- Classes/PKStateMachine.swift | 9 ++- Plugins/IMA/AdsEnabledPlayerController.swift | 63 ++++++++++++++++++-- Plugins/IMA/AdsPlugin.swift | 8 +++ Plugins/IMA/IMAConfig.swift | 16 +++-- Plugins/IMA/IMAPlugin.swift | 42 +++++++++---- 5 files changed, 110 insertions(+), 28 deletions(-) diff --git a/Classes/PKStateMachine.swift b/Classes/PKStateMachine.swift index 3f4962bc..4cf67645 100644 --- a/Classes/PKStateMachine.swift +++ b/Classes/PKStateMachine.swift @@ -53,9 +53,12 @@ class BasicStateMachine { PKLog.error("\(String(describing: type(of: self))) was set to initial state, this is not allowed") return } - self.state = state - DispatchQueue.main.async { - self.onStateChange?(state) + // only set state when changed + if self.state != state { + self.state = state + DispatchQueue.main.async { + self.onStateChange?(state) + } } } } diff --git a/Plugins/IMA/AdsEnabledPlayerController.swift b/Plugins/IMA/AdsEnabledPlayerController.swift index fab39870..cc8db43e 100644 --- a/Plugins/IMA/AdsEnabledPlayerController.swift +++ b/Plugins/IMA/AdsEnabledPlayerController.swift @@ -11,14 +11,36 @@ import UIKit import AVFoundation import AVKit +/// `AdsPlayerState` represents `AdsEnabledPlayerController` state machine states. +enum AdsPlayerState: Int, StateProtocol { + /// initial state. + case start = 0 + /// when prepare was requested for the first time and it is stalled until ad started (preroll) / faliure or content resume + case waitingForPrepare + /// a moment before we called prepare until prepare() was finished (the sychornos code only not async tasks) + case preparing + /// Indicates when prepare() was finished (the sychornos code only not async tasks) + case prepared +} + class AdsEnabledPlayerController : PlayerDecoratorBase, AdsPluginDelegate, AdsPluginDataSource { enum PlayType { case play, resume } + /// The ads player state machine. + private var stateMachine = BasicStateMachine(initialState: AdsPlayerState.start, allowTransitionToInitialState: true) + + /// The media config to prepare the player with. + /// Uses @NSCopying in order to make a copy whenever set with new value. + @NSCopying private var prepareMediaConfig: MediaConfig! + /// indicates if play was used, if `play()` or `resume()` was called we set this to true. - var isPlayEnabled = false + private var isPlayEnabled = false + + /// a semaphore to make sure prepare calling will not be reached from 2 threads by mistake. + private let prepareSemaphore = DispatchSemaphore(value: 1) /// when playing post roll google sends content resume when finished. /// In our case we need to prevent sending play/resume to the player because the content already ended. @@ -48,10 +70,10 @@ class AdsEnabledPlayerController : PlayerDecoratorBase, AdsPluginDelegate, AdsPl } } - // TODO:: finilize prepare override func prepare(_ config: MediaConfig) { - super.prepare(config) - + self.stop() + self.stateMachine.set(state: .waitingForPrepare) + self.prepareMediaConfig = config self.adsPlugin.requestAds() } @@ -75,8 +97,9 @@ class AdsEnabledPlayerController : PlayerDecoratorBase, AdsPluginDelegate, AdsPl } override func stop() { - self.adsPlugin.destroyManager() + self.stateMachine.set(state: .start) super.stop() + self.adsPlugin.destroyManager() self.isPlayEnabled = false self.shouldPreventContentResume = false } @@ -95,18 +118,24 @@ class AdsEnabledPlayerController : PlayerDecoratorBase, AdsPluginDelegate, AdsPl return self.delegate!.playerShouldPlayAd(self) } + var adsPluginStartTime: TimeInterval { + return self.prepareMediaConfig?.startTime ?? 0 + } + /************************************************************/ // MARK: - AdsPluginDelegate /************************************************************/ func adsPlugin(_ adsPlugin: AdsPlugin, loaderFailedWith error: String) { if self.isPlayEnabled { + self.preparePlayerIfNeeded() super.play() self.adsPlugin.didPlay() } } func adsPlugin(_ adsPlugin: AdsPlugin, managerFailedWith error: String) { + self.preparePlayerIfNeeded() super.play() self.adsPlugin.didPlay() } @@ -117,9 +146,15 @@ class AdsEnabledPlayerController : PlayerDecoratorBase, AdsPluginDelegate, AdsPl super.pause() case let e where type(of: e) == AdEvent.adDidRequestResume: if !self.shouldPreventContentResume { + self.preparePlayerIfNeeded() super.resume() } case let e where type(of: e) == AdEvent.adResumed: self.isPlayEnabled = true + case let e where type(of: e) == AdEvent.adStarted: + // when starting to play pre roll start preparing the player. + if event.adInfo?.positionType == .preRoll { + self.preparePlayerIfNeeded() + } case let e where type(of: e) == AdEvent.adLoaded || type(of: e) == AdEvent.adBreakReady: if self.shouldPreventContentResume == true { return } // no need to handle twice if already true if event.adInfo?.positionType == .postRoll { @@ -132,12 +167,30 @@ class AdsEnabledPlayerController : PlayerDecoratorBase, AdsPluginDelegate, AdsPl func adsRequestTimedOut(shouldPlay: Bool) { if shouldPlay { + self.preparePlayerIfNeeded() self.play() } } func play(_ playType: PlayType) { + self.preparePlayerIfNeeded() playType == .play ? super.play() : super.resume() self.adsPlugin.didPlay() } + + /************************************************************/ + // MARK: - Private + /************************************************************/ + + /// prepare the player only if wasn't prepared yet. + private func preparePlayerIfNeeded() { + self.prepareSemaphore.wait() // use semaphore to make sure will not be called from more than one thread by mistake. + if self.stateMachine.getState() == .waitingForPrepare { + self.stateMachine.set(state: .preparing) + PKLog.debug("will prepare player") + super.prepare(self.prepareMediaConfig) + self.stateMachine.set(state: .prepared) + } + self.prepareSemaphore.signal() + } } diff --git a/Plugins/IMA/AdsPlugin.swift b/Plugins/IMA/AdsPlugin.swift index 074ac836..3643bee2 100644 --- a/Plugins/IMA/AdsPlugin.swift +++ b/Plugins/IMA/AdsPlugin.swift @@ -11,6 +11,8 @@ import AVKit protocol AdsPluginDataSource : class { func adsPluginShouldPlayAd(_ adsPlugin: AdsPlugin) -> Bool + /// the player's media config start time. + var adsPluginStartTime: TimeInterval { get } } protocol AdsPluginDelegate : class { @@ -27,12 +29,18 @@ protocol AdsPlugin: PKPlugin, AVPictureInPictureControllerDelegate { weak var dataSource: AdsPluginDataSource? { get set } weak var delegate: AdsPluginDelegate? { get set } var pipDelegate: AVPictureInPictureControllerDelegate? { get set } + /// is ad playing currently. var isAdPlaying: Bool { get } + /// request ads from the server. func requestAds() + /// resume ad func resume() + /// pause ad func pause() + /// ad content complete func contentComplete() + /// destroy the ads manager func destroyManager() /// called after player called `super.play()` func didPlay() diff --git a/Plugins/IMA/IMAConfig.swift b/Plugins/IMA/IMAConfig.swift index ed428c0f..d90886ee 100644 --- a/Plugins/IMA/IMAConfig.swift +++ b/Plugins/IMA/IMAConfig.swift @@ -11,22 +11,20 @@ import GoogleInteractiveMediaAds @objc public class IMAConfig: NSObject { - @objc public var language: String = "en" - @objc public var enableBackgroundPlayback: Bool { - return true - } + @objc public let enableBackgroundPlayback = true // defaulted to false, because otherwise ad breaks events will not happen. // we need to have control on whether ad break will start playing or not using `Loaded` event is not enough. - // (will also need more safety checks because loaded will happen more than once). - @objc public var autoPlayAdBreaks: Bool { - return false - } + // (will also need more safety checks for loaded because loaded will happen more than once). + @objc public let autoPlayAdBreaks = false + @objc public var language: String = "en" + @objc public var videoBitrate = kIMAAutodetectBitrate @objc public var videoMimeTypes: [Any]? @objc public var adTagUrl: String = "" @objc public var companionView: UIView? @objc public var webOpenerPresentingController: UIViewController? - @objc public var requestTimeoutInterval: TimeInterval = 5 + /// ads request timeout interval, when ads request will take more then this time will resume content. + @objc public var requestTimeoutInterval: TimeInterval = IMAPlugin.defaultTimeoutInterval /// enables debug mode on IMA SDK which will output detailed log information to the console. /// The default value is false. @objc public var enableDebugMode: Bool = false diff --git a/Plugins/IMA/IMAPlugin.swift b/Plugins/IMA/IMAPlugin.swift index befffea5..8d7eb6e5 100644 --- a/Plugins/IMA/IMAPlugin.swift +++ b/Plugins/IMA/IMAPlugin.swift @@ -32,6 +32,9 @@ enum IMAState: Int, StateProtocol { @objc public class IMAPlugin: BasePlugin, PKPluginWarmUp, PlayerDecoratorProvider, AdsPlugin, IMAAdsLoaderDelegate, IMAAdsManagerDelegate, IMAWebOpenerDelegate, IMAContentPlayhead { + /// the default timeout interval for ads request. + static let defaultTimeoutInterval: TimeInterval = 5 + weak var dataSource: AdsPluginDataSource? { didSet { PKLog.debug("data source set") @@ -43,20 +46,19 @@ enum IMAState: Int, StateProtocol { /// The IMA plugin state machine private var stateMachine = BasicStateMachine(initialState: IMAState.start, allowTransitionToInitialState: false) + private static var loader: IMAAdsLoader! private var adsManager: IMAAdsManager? private var renderingSettings: IMAAdsRenderingSettings! = IMAAdsRenderingSettings() - private static var loader: IMAAdsLoader! - private var pictureInPictureProxy: IMAPictureInPictureProxy? private var loadingView: UIView? + // we must have config error will be thrown otherwise private var config: IMAConfig! - private var timer: Timer? /// timer for checking IMA requests timeout. private var requestTimeoutTimer: Timer? /// the request timeout interval - private var requestTimeoutInterval: TimeInterval = 5 + private var requestTimeoutInterval: TimeInterval = IMAPlugin.defaultTimeoutInterval /************************************************************/ // MARK: - IMAContentPlayhead @@ -91,6 +93,7 @@ enum IMAState: Int, StateProtocol { try super.init(player: player, pluginConfig: pluginConfig, messageBus: messageBus) if let adsConfig = pluginConfig as? IMAConfig { self.config = adsConfig + self.requestTimeoutInterval = adsConfig.requestTimeoutInterval if IMAPlugin.loader == nil { self.setupLoader(with: adsConfig) } @@ -118,7 +121,6 @@ enum IMAState: Int, StateProtocol { // TODO:: finilize update config & updateMedia logic public override func onUpdateMedia(mediaConfig: MediaConfig) { - PKLog.debug("mediaConfig: " + String(describing: mediaConfig)) super.onUpdateMedia(mediaConfig: mediaConfig) } @@ -265,16 +267,20 @@ enum IMAState: Int, StateProtocol { // Ad break, will be called before each scheduled ad break. Ad breaks may contain more than 1 ad. // `event.ad` is not available at this point do not use it here. case .AD_BREAK_READY: - self.notify(event: AdEvent.AdBreakReady()) - guard canPlayAd(forState: currentState) else { return } - self.start(adsManager: adsManager) + if shouldDiscardAd() { + PKLog.debug("discard Ad Break") + } else { + self.notify(event: AdEvent.AdBreakReady()) + guard canPlayAd(forState: currentState) else { return } + self.start(adsManager: adsManager) + } + // single ad only fires `LOADED` without `AD_BREAK_READY`. case .LOADED: if shouldDiscard(ad: event.ad, currentState: currentState) { - adsManager.discardAdBreak() + self.discardAdBreak(adsManager: adsManager) } else { let adEvent = event.ad != nil ? AdEvent.AdLoaded(adInfo: PKAdInfo(ad: event.ad)) : AdEvent.AdLoaded() self.notify(event: adEvent) - // single ad only fires `LOADED` without `AD_BREAK_READY`. // if we have more than one ad don't start the manager, it will be handled in `AD_BREAK_READY` guard adsManager.adCuePoints.count == 0 else { return } guard canPlayAd(forState: currentState) else { return } @@ -447,15 +453,29 @@ enum IMAState: Int, StateProtocol { return false } + private func shouldDiscardAd() -> Bool { + if currentTime < self.dataSource?.adsPluginStartTime ?? 0 { + return true + } + return false + } + private func shouldDiscard(ad: IMAAd, currentState: IMAState) -> Bool { let adInfo = PKAdInfo(ad: ad) + let isStartTimeInvalid = adInfo.positionType != .postRoll && adInfo.timeOffset < self.dataSource?.adsPluginStartTime ?? 0 let isPreRollInvalid = adInfo.positionType == .preRoll && (currentState == .adsRequestTimedOut || currentState == .contentPlaying) - if isPreRollInvalid { + if isStartTimeInvalid || isPreRollInvalid { return true } return false } + private func discardAdBreak(adsManager: IMAAdsManager) { + PKLog.debug("discard Ad Break") + adsManager.discardAdBreak() + self.adsManagerDidRequestContentResume(adsManager) + } + /************************************************************/ // MARK: - AVPictureInPictureControllerDelegate /************************************************************/