From 552add042802d71235e6b2f67bb06162d8629ed4 Mon Sep 17 00:00:00 2001 From: Mansi Pandya Date: Tue, 18 Nov 2025 14:30:50 -0500 Subject: [PATCH 1/4] fix: implement changes requested in code review for previous PR --- media/src/main/java/com/mparticle/media/MediaSession.kt | 8 ++++---- media/src/test/java/com/mparticle/MediaSessionTest.kt | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/media/src/main/java/com/mparticle/media/MediaSession.kt b/media/src/main/java/com/mparticle/media/MediaSession.kt index e129918..bed76b0 100644 --- a/media/src/main/java/com/mparticle/media/MediaSession.kt +++ b/media/src/main/java/com/mparticle/media/MediaSession.kt @@ -575,7 +575,7 @@ class MediaSession protected constructor(builder: Builder) { } private fun pauseContentTimeIfAdBreakExclusionEnabled() { - if (!excludeAdBreaksFromContentTime) { + if (excludeAdBreaksFromContentTime) { currentPlaybackStartTimestamp?.let { storedPlaybackTime += ((System.currentTimeMillis() - it) / 1000) currentPlaybackStartTimestamp = null @@ -584,7 +584,7 @@ class MediaSession protected constructor(builder: Builder) { } private fun resumeContentTimeIfAdBreakExclusionEnabled() { - if (!excludeAdBreaksFromContentTime) { + if (excludeAdBreaksFromContentTime) { if (currentPlaybackStartTimestamp != null) { currentPlaybackStartTimestamp = System.currentTimeMillis() } @@ -777,10 +777,10 @@ class MediaSession protected constructor(builder: Builder) { } /** - * When enabled, automatically pauses content time tracking during ad breaks and resumes after. + * When enabled, ad break time is excluded from content time tracking. * When disabled (default), ad break time is included in content time spent. */ - fun pauseContentDuringAdBreaks(shouldPause: Boolean): Builder { + fun excludeAdBreaksFromContentTime(shouldPause: Boolean): Builder { this.excludeAdBreaksFromContentTime = shouldPause return this } diff --git a/media/src/test/java/com/mparticle/MediaSessionTest.kt b/media/src/test/java/com/mparticle/MediaSessionTest.kt index bad6e54..98b3ca5 100644 --- a/media/src/test/java/com/mparticle/MediaSessionTest.kt +++ b/media/src/test/java/com/mparticle/MediaSessionTest.kt @@ -695,13 +695,13 @@ class MediaSessionTest { mediaSession.logAdBreakStart { id = "break-1" } - Thread.sleep(1000) // should NOT count toward content time + Thread.sleep(1000) mediaSession.logPause() mediaSession.logAdBreakEnd() val contentTime = mediaSession.mediaContentTimeSpent - assertEquals(1.0, contentTime) + assertEquals(2.0, contentTime, 0.2) val playCount = events.count { it.eventName == MediaEventName.PLAY } val pauseCount = events.count { it.eventName == MediaEventName.PAUSE } @@ -729,7 +729,7 @@ class MediaSessionTest { mediaSession.logPause() val contentTime = mediaSession.mediaContentTimeSpent - assertEquals(2.0, contentTime) + assertEquals(1.0, contentTime, 0.2) } } From e286e0d3083ee21297fa6a3edb28ca9b2f32d326 Mon Sep 17 00:00:00 2001 From: Mansi Pandya Date: Wed, 19 Nov 2025 15:27:41 -0500 Subject: [PATCH 2/4] added logic to handle edge cases to match iOS behavior --- media/src/main/java/com/mparticle/media/MediaSession.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/media/src/main/java/com/mparticle/media/MediaSession.kt b/media/src/main/java/com/mparticle/media/MediaSession.kt index bed76b0..9806bc6 100644 --- a/media/src/main/java/com/mparticle/media/MediaSession.kt +++ b/media/src/main/java/com/mparticle/media/MediaSession.kt @@ -119,6 +119,7 @@ class MediaSession protected constructor(builder: Builder) { var storedPlaybackTime: Double = 0.0 //On Pause calculate playback time and clear currentPlaybackTime private set private var sessionSummarySent = false // Ensures we only send summary event once + private var pausedByAdBreak: Boolean = false // Tracks if content was paused by an ad break (for resume logic) private var testing = false // Enabled for test cases @@ -579,15 +580,21 @@ class MediaSession protected constructor(builder: Builder) { currentPlaybackStartTimestamp?.let { storedPlaybackTime += ((System.currentTimeMillis() - it) / 1000) currentPlaybackStartTimestamp = null + pausedByAdBreak = true + } ?: run { + // Content was already paused, don't mark as paused by ad break + pausedByAdBreak = false } } } private fun resumeContentTimeIfAdBreakExclusionEnabled() { if (excludeAdBreaksFromContentTime) { - if (currentPlaybackStartTimestamp != null) { + // Only resume if content was paused by the ad break, not if it was already paused + if (pausedByAdBreak && currentPlaybackStartTimestamp == null) { currentPlaybackStartTimestamp = System.currentTimeMillis() } + pausedByAdBreak = false } } From 3bb9e9fa22fe447b54aa434d14cba3284897f54f Mon Sep 17 00:00:00 2001 From: Mansi Pandya Date: Fri, 5 Dec 2025 15:38:53 -0500 Subject: [PATCH 3/4] align ad playback tracking logic with iOS implementation --- .../java/com/mparticle/media/MediaSession.kt | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/media/src/main/java/com/mparticle/media/MediaSession.kt b/media/src/main/java/com/mparticle/media/MediaSession.kt index 9806bc6..b25610f 100644 --- a/media/src/main/java/com/mparticle/media/MediaSession.kt +++ b/media/src/main/java/com/mparticle/media/MediaSession.kt @@ -91,7 +91,7 @@ class MediaSession protected constructor(builder: Builder) { val mediaContentTimeSpent: Double get() { //total seconds spent playing content return currentPlaybackStartTimestamp?.let { - this.storedPlaybackTime + (System.currentTimeMillis().minus(it) / 1000).toDouble() + this.storedPlaybackTime + (System.currentTimeMillis() - it) / 1000.0 } ?: this.storedPlaybackTime } var mediaContentCompleteLimit: Int = 100 @@ -119,7 +119,7 @@ class MediaSession protected constructor(builder: Builder) { var storedPlaybackTime: Double = 0.0 //On Pause calculate playback time and clear currentPlaybackTime private set private var sessionSummarySent = false // Ensures we only send summary event once - private var pausedByAdBreak: Boolean = false // Tracks if content was paused by an ad break (for resume logic) + private var playbackState: PlaybackState = PlaybackState.PAUSED_BY_USER // Tracks whether playback was playing, paused, or paused by ad break private var testing = false // Enabled for test cases @@ -192,6 +192,8 @@ class MediaSession protected constructor(builder: Builder) { if (currentPlaybackStartTimestamp == null) { currentPlaybackStartTimestamp = System.currentTimeMillis() } + + playbackState = PlaybackState.PLAYING val playEvent = MediaEvent(this, MediaEventName.PLAY, options = options) logEvent(playEvent) } @@ -204,6 +206,8 @@ class MediaSession protected constructor(builder: Builder) { storedPlaybackTime += ((System.currentTimeMillis() - it) / 1000) currentPlaybackStartTimestamp = null; } + + playbackState = PlaybackState.PAUSED_BY_USER val pauseEvent = MediaEvent(this, MediaEventName.PAUSE, options = options) logEvent(pauseEvent) } @@ -576,34 +580,30 @@ class MediaSession protected constructor(builder: Builder) { } private fun pauseContentTimeIfAdBreakExclusionEnabled() { - if (excludeAdBreaksFromContentTime) { - currentPlaybackStartTimestamp?.let { - storedPlaybackTime += ((System.currentTimeMillis() - it) / 1000) - currentPlaybackStartTimestamp = null - pausedByAdBreak = true - } ?: run { - // Content was already paused, don't mark as paused by ad break - pausedByAdBreak = false - } + if (!excludeAdBreaksFromContentTime || playbackState != PlaybackState.PLAYING) return + + currentPlaybackStartTimestamp?.let { + storedPlaybackTime += (System.currentTimeMillis() - it) / 1000.0 + currentPlaybackStartTimestamp = null + playbackState = PlaybackState.PAUSED_BY_AD_BREAK } } private fun resumeContentTimeIfAdBreakExclusionEnabled() { - if (excludeAdBreaksFromContentTime) { - // Only resume if content was paused by the ad break, not if it was already paused - if (pausedByAdBreak && currentPlaybackStartTimestamp == null) { - currentPlaybackStartTimestamp = System.currentTimeMillis() - } - pausedByAdBreak = false - } + if (!excludeAdBreaksFromContentTime || playbackState != PlaybackState.PAUSED_BY_AD_BREAK) return + + currentPlaybackStartTimestamp = System.currentTimeMillis() + playbackState = PlaybackState.PLAYING } private fun logAdSummary(content: MediaAd?) { content?.let { ad -> ad.adStartTimestamp?.let { startTime -> - val endTime = System.currentTimeMillis() - ad.adEndTimestamp = endTime - mediaTotalAdTimeSpent += ((endTime - startTime) / 1000).toDouble() + val endTime = ad.adEndTimestamp ?: System.currentTimeMillis() + if (ad.adEndTimestamp == null) { + ad.adEndTimestamp = endTime + mediaTotalAdTimeSpent += ((endTime - startTime) / 1000).toDouble() + } } val customAttributes: MutableMap = mutableMapOf() @@ -802,6 +802,12 @@ class MediaSession protected constructor(builder: Builder) { } +private enum class PlaybackState { + PLAYING, + PAUSED_BY_USER, + PAUSED_BY_AD_BREAK +} + private fun String?.require(variableName: String): String { if (this == null) { Logger.error("\"$variableName\" should not be null") From 9f84584fc52be598068443bebdc693a01c94c68f Mon Sep 17 00:00:00 2001 From: Mansi Pandya Date: Mon, 8 Dec 2025 11:46:44 -0500 Subject: [PATCH 4/4] Update tests to include ad-break pause scenario and address review comments --- .../java/com/mparticle/MediaSessionTest.kt | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/media/src/test/java/com/mparticle/MediaSessionTest.kt b/media/src/test/java/com/mparticle/MediaSessionTest.kt index 98b3ca5..a9c205f 100644 --- a/media/src/test/java/com/mparticle/MediaSessionTest.kt +++ b/media/src/test/java/com/mparticle/MediaSessionTest.kt @@ -679,7 +679,7 @@ class MediaSessionTest { } @Test - fun testContentTimeExcludesAdBreak_When_Flag_Disable() { + fun testExcludeAdBreaksFromContentTime_Default_Disabled_IncludesAdTime() { val mparticle = MockMParticle() val events = mutableListOf() val mediaSession = MediaSession.builder(mparticle) { @@ -710,7 +710,7 @@ class MediaSessionTest { } @Test - fun testDefaultBehavior_Unchanged_When_Flag_Enable() { + fun testExcludeAdBreaksFromContentTime_Enabled_ExcludesAdTime() { val mparticle = MockMParticle() val mediaSession = MediaSession.builder(mparticle) { title = "hello" @@ -724,13 +724,39 @@ class MediaSessionTest { mediaSession.logAdBreakStart { id = "break-2" } - Thread.sleep(1000) // should count toward content time when flag disabled + Thread.sleep(1000) mediaSession.logAdBreakEnd() mediaSession.logPause() val contentTime = mediaSession.mediaContentTimeSpent assertEquals(1.0, contentTime, 0.2) } + + @Test + fun testExcludeAdBreaksFromContentTime_UserPausesMidAd_ThenAdBreakEnds() { + val mparticle = MockMParticle() + val mediaSession = MediaSession.builder(mparticle) { + title = "hello" + mediaContentId ="123" + duration =1000 + excludeAdBreaksFromContentTime = true + } + + mediaSession.logPlay() + Thread.sleep(1000) + + mediaSession.logAdBreakStart { + id = "break-3" + } + Thread.sleep(1000) + + mediaSession.logPause() + + mediaSession.logAdBreakEnd() + + val contentTime = mediaSession.mediaContentTimeSpent + assertEquals(1.0, contentTime, 0.2) + } } fun T.assertTrue(assertion: (T) -> Boolean) {