Merge pull request #9 from bitmovin/atomic-ad-playback-tracker

Atomic ad playback tracker state updates

Author: zigavehovec

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+
+- Race condition that could cause `IndexOutOfBoundsException` when `MediaTailorAdBreak`s got removed during playback
+
 ## [0.1.1] - 2025-07-11
 
 ### Fixed

mediatailor/src/main/java/com/bitmovin/player/integration/mediatailor/AdPlaybackTracker.kt

@@ -11,6 +11,7 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.merge
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
@@ -47,33 +48,36 @@ internal class DefaultAdPlaybackTracker(
 
     init {
         scope.launch {
-            player.eventFlow<PlayerEvent.TimeChanged>().collect {
-                trackAdBreaks()
-            }
-        }
-        scope.launch {
-            merge(
-                // When seeking or time shifting, the calculated ad breaks are most likely outdated
-                player.eventFlow<PlayerEvent.Seeked>(),
-                player.eventFlow<PlayerEvent.TimeShifted>(),
-                // In case ad breaks are updated before the tail the indices might point to an invalid ad break
-                mediaTailorSession.adBreaks,
-            ).collect {
+            stateShouldBeInvalidatedFlow().collectLatest {
                 currentAdBreakIndex = 0
                 currentAdIndex = 0
-                trackAdBreaks()
+                val adBreaks = mediaTailorSession.adBreaks.value
+
+                player.eventFlow<PlayerEvent.TimeChanged>().collect {
+                    trackAdBreaks(adBreaks)
+                }
             }
         }
     }
 
+    private fun stateShouldBeInvalidatedFlow() = merge(
+        // When seeking or time shifting, the calculated ad breaks are most likely outdated
+        player.eventFlow<PlayerEvent.Seeked>(),
+        player.eventFlow<PlayerEvent.TimeShifted>(),
+        // In case ad breaks are updated before the tail the indices might point to an invalid ad break
+        mediaTailorSession.adBreaks,
+    )
+
     /**
      * This function uses [currentAdBreakIndex] and [currentAdIndex] to track the current position
      * of the [playingAdBreak] and [nextAdBreak] to avoid unnecessary iterations over all ad breaks.
      *
      * It assumes that ad breaks are sorted by schedule time.
      */
-    private fun trackAdBreaks() {
-        val adBreaks = mediaTailorSession.adBreaks.value.takeIf { it.isNotEmpty() } ?: return
+    private fun trackAdBreaks(adBreaks: List<MediaTailorAdBreak>) {
+        if (adBreaks.isEmpty()) {
+            return
+        }
 
         while (currentAdBreakIndex < adBreaks.lastIndex &&
             player.currentTime >= adBreaks[currentAdBreakIndex].endTime

mediatailor/src/test/java/com/bitmovin/player/integration/mediatailor/AdPlaybackTrackerSpec.kt

@@ -88,8 +88,11 @@ class AdPlaybackTrackerSpec : DescribeSpec({
 
         describe("with an ad break in the present") {
             lateinit var adBreak: MediaTailorAdBreak
+            lateinit var timeChangedFlow: MutableSharedFlow<PlayerEvent.TimeChanged>
+
             beforeEach {
-                createPlayer(currentTime = 5.0)
+                timeChangedFlow = MutableSharedFlow()
+                createPlayer(currentTime = 5.0, timeChangedFlow)
                 adBreak = MediaTailorAdBreak(
                     id = "adBreak1",
                     ads = listOf(
@@ -107,6 +110,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
                     adMarkerDuration = "5",
                 )
                 createAdPlaybackTracker(listOf(adBreak))
+                timeChangedFlow.emit(PlayerEvent.TimeChanged(5.0))
             }
 
             it("currentAdBreak is the correct adBreak") {
@@ -116,6 +120,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
 
         describe("with an ad break in the past") {
             lateinit var adBreak: MediaTailorAdBreak
+
             beforeEach {
                 createPlayer(currentTime = 5.0)
                 adBreak = MediaTailorAdBreak(
@@ -136,8 +141,11 @@ class AdPlaybackTrackerSpec : DescribeSpec({
 
         describe("with an ad break in the future") {
             lateinit var adBreak: MediaTailorAdBreak
+            lateinit var timeChangedFlow: MutableSharedFlow<PlayerEvent.TimeChanged>
+
             beforeEach {
-                createPlayer(currentTime = 5.0)
+                timeChangedFlow = MutableSharedFlow()
+                createPlayer(currentTime = 5.0, timeChangedFlow)
                 adBreak = MediaTailorAdBreak(
                     id = "adBreak1",
                     ads = listOf(),
@@ -147,6 +155,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
                     adMarkerDuration = "5",
                 )
                 createAdPlaybackTracker(listOf(adBreak))
+                timeChangedFlow.emit(PlayerEvent.TimeChanged(5.0))
             }
 
             it("currentAdBreak is null") {
@@ -161,8 +170,11 @@ class AdPlaybackTrackerSpec : DescribeSpec({
         describe("with ad break in the past future and current ad break") {
             lateinit var currentAdBreak: MediaTailorAdBreak
             lateinit var nextAdBreak: MediaTailorAdBreak
+            lateinit var timeChangedFlow: MutableSharedFlow<PlayerEvent.TimeChanged>
+
             beforeEach {
-                createPlayer(currentTime = 5.0)
+                timeChangedFlow = MutableSharedFlow()
+                createPlayer(currentTime = 5.0, timeChangedFlow)
                 currentAdBreak = MediaTailorAdBreak(
                     id = "adBreak1",
                     ads = listOf(
@@ -196,6 +208,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
                     adMarkerDuration = "5",
                 )
                 createAdPlaybackTracker(listOf(currentAdBreak, nextAdBreak))
+                timeChangedFlow.emit(PlayerEvent.TimeChanged(5.0))
             }
 
             it("currentAdBreak is the correct adBreak") {
@@ -239,9 +252,13 @@ class AdPlaybackTrackerSpec : DescribeSpec({
             )
 
             describe("when the player time is at first ad") {
+                lateinit var timeChangedFlow: MutableSharedFlow<PlayerEvent.TimeChanged>
+
                 beforeEach {
-                    createPlayer(currentTime = 5.0)
+                    timeChangedFlow = MutableSharedFlow()
+                    createPlayer(currentTime = 5.0, timeChangedFlow)
                     createAdPlaybackTracker(listOf(adBreak))
+                    timeChangedFlow.emit(PlayerEvent.TimeChanged(5.0))
                 }
 
                 it("current ad is the first ad") {
@@ -254,9 +271,13 @@ class AdPlaybackTrackerSpec : DescribeSpec({
             }
 
             describe("when the player time is at second ad") {
+                lateinit var timeChangedFlow: MutableSharedFlow<PlayerEvent.TimeChanged>
+
                 beforeEach {
-                    createPlayer(currentTime = 10.0)
+                    timeChangedFlow = MutableSharedFlow()
+                    createPlayer(currentTime = 10.0, timeChangedFlow)
                     createAdPlaybackTracker(listOf(adBreak))
+                    timeChangedFlow.emit(PlayerEvent.TimeChanged(10.0))
                 }
 
                 it("current ad is the second ad") {
@@ -269,9 +290,13 @@ class AdPlaybackTrackerSpec : DescribeSpec({
             }
 
             describe("when the player time is at third ad") {
+                lateinit var timeChangedFlow: MutableSharedFlow<PlayerEvent.TimeChanged>
+
                 beforeEach {
-                    createPlayer(currentTime = 15.0)
+                    timeChangedFlow = MutableSharedFlow()
+                    createPlayer(currentTime = 15.0, timeChangedFlow)
                     createAdPlaybackTracker(listOf(adBreak))
+                    timeChangedFlow.emit(PlayerEvent.TimeChanged(15.0))
                 }
 
                 it("current ad is the third ad") {
@@ -287,6 +312,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
 
     describe("when player receives seeking and time shifting events") {
         val timeShiftedFlow = MutableSharedFlow<PlayerEvent.TimeShifted>()
+        val timeChangedFlow = MutableSharedFlow<PlayerEvent.TimeChanged>()
         val seekedFlow = MutableSharedFlow<PlayerEvent.Seeked>()
         lateinit var firstAdBreak: MediaTailorAdBreak
         lateinit var secondAdBreak: MediaTailorAdBreak
@@ -296,6 +322,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
                 currentTime = 5.0,
                 timeShiftedFlow = timeShiftedFlow,
                 seekedFlow = seekedFlow,
+                timeChangedFlow = timeChangedFlow,
             )
             firstAdBreak = MediaTailorAdBreak(
                 id = "adBreak1",
@@ -330,6 +357,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
                 adMarkerDuration = "5",
             )
             createAdPlaybackTracker(listOf(firstAdBreak, secondAdBreak))
+            timeChangedFlow.emit(PlayerEvent.TimeChanged(5.0))
         }
 
         describe("before the time shift or seek") {
@@ -346,6 +374,7 @@ class AdPlaybackTrackerSpec : DescribeSpec({
             beforeEach {
                 // Time has to change otherwise the same ad breaks will be calculated again
                 every { player.currentTime } returns 50.0
+                timeChangedFlow.emit(PlayerEvent.TimeChanged(50.0))
             }
 
             it("resets the state when time shifted") {
@@ -503,6 +532,8 @@ class AdPlaybackTrackerSpec : DescribeSpec({
 
         describe("when the player starts in the second ad in the first ad break") {
             it("selects the correct ad index initially") {
+                timeChangedFlow.emit(PlayerEvent.TimeChanged(1.5))
+
                 expectThat(adPlaybackTracker.playingAdBreak.value?.adBreak).isEqualTo(firstAdBreak)
                 expectThat(adPlaybackTracker.playingAdBreak.value?.adIndex).isEqualTo(1)
             }