Fix position reporting with fetch errors

On receiving a fetch error for an ad that would otherwise play based on an
initial/seek position, the pending content position wasn't cleared which meant
that position reporting was broken after a fetch error. Fix this by always
clearing the pending position (if there was a pending position that will have
triggered the fetch error).

Also deduplicate the code for handling empty ad groups (fetch errors)
and ad group load errors.

Issue: #7956
PiperOrigin-RevId: 334113131

RELEASENOTES.md

@@ -28,6 +28,9 @@
     *   Add the option to sort tracks by `Format` in `TrackSelectionView` and
     *   Add the option to sort tracks by `Format` in `TrackSelectionView` and
         `TrackSelectionDialogBuilder`
         `TrackSelectionDialogBuilder`
         ([#7709](https://github.com/google/ExoPlayer/issues/7709)).
         ([#7709](https://github.com/google/ExoPlayer/issues/7709)).
+*   IMA extension:
+    *   Fix position reporting after fetch errors
+        ([#7956](https://github.com/google/ExoPlayer/issues/7956)).
 
 
 ### 2.12.0 (2020-09-11) ###
 ### 2.12.0 (2020-09-11) ###
 
 

extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java

@@ -1077,7 +1077,7 @@ public final class ImaAdsLoader
             adGroupTimeSeconds == -1.0
             adGroupTimeSeconds == -1.0
                 ? adPlaybackState.adGroupCount - 1
                 ? adPlaybackState.adGroupCount - 1
                 : getAdGroupIndexForCuePointTimeSeconds(adGroupTimeSeconds);
                 : getAdGroupIndexForCuePointTimeSeconds(adGroupTimeSeconds);
-        handleAdGroupFetchError(adGroupIndex);
+        markAdGroupInErrorStateAndClearPendingContentPosition(adGroupIndex);
         break;
         break;
       case CONTENT_PAUSE_REQUESTED:
       case CONTENT_PAUSE_REQUESTED:
         // After CONTENT_PAUSE_REQUESTED, IMA will playAd/pauseAd/stopAd to show one or more ads
         // After CONTENT_PAUSE_REQUESTED, IMA will playAd/pauseAd/stopAd to show one or more ads
@@ -1364,35 +1364,20 @@ public final class ImaAdsLoader
     }
     }
   }
   }
 
 
-  private void handleAdGroupFetchError(int adGroupIndex) {
-    AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adGroupIndex];
-    if (adGroup.count == C.LENGTH_UNSET) {
-      adPlaybackState = adPlaybackState.withAdCount(adGroupIndex, max(1, adGroup.states.length));
-      adGroup = adPlaybackState.adGroups[adGroupIndex];
-    }
-    for (int i = 0; i < adGroup.count; i++) {
-      if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) {
-        if (DEBUG) {
-          Log.d(TAG, "Removing ad " + i + " in ad group " + adGroupIndex);
-        }
-        adPlaybackState = adPlaybackState.withAdLoadError(adGroupIndex, i);
-      }
-    }
-    updateAdPlaybackState();
-  }
-
   private void handleAdGroupLoadError(Exception error) {
   private void handleAdGroupLoadError(Exception error) {
-    if (player == null) {
-      return;
-    }
-
-    // TODO: Once IMA signals which ad group failed to load, remove this call.
     int adGroupIndex = getLoadingAdGroupIndex();
     int adGroupIndex = getLoadingAdGroupIndex();
     if (adGroupIndex == C.INDEX_UNSET) {
     if (adGroupIndex == C.INDEX_UNSET) {
       Log.w(TAG, "Unable to determine ad group index for ad group load error", error);
       Log.w(TAG, "Unable to determine ad group index for ad group load error", error);
       return;
       return;
     }
     }
+    markAdGroupInErrorStateAndClearPendingContentPosition(adGroupIndex);
+    if (pendingAdLoadError == null) {
+      pendingAdLoadError = AdLoadException.createForAdGroup(error, adGroupIndex);
+    }
+  }
 
 
+  private void markAdGroupInErrorStateAndClearPendingContentPosition(int adGroupIndex) {
+    // Update the ad playback state so all ads in the ad group are in the error state.
     AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adGroupIndex];
     AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adGroupIndex];
     if (adGroup.count == C.LENGTH_UNSET) {
     if (adGroup.count == C.LENGTH_UNSET) {
       adPlaybackState = adPlaybackState.withAdCount(adGroupIndex, max(1, adGroup.states.length));
       adPlaybackState = adPlaybackState.withAdCount(adGroupIndex, max(1, adGroup.states.length));
@@ -1407,9 +1392,7 @@ public final class ImaAdsLoader
       }
       }
     }
     }
     updateAdPlaybackState();
     updateAdPlaybackState();
-    if (pendingAdLoadError == null) {
-      pendingAdLoadError = AdLoadException.createForAdGroup(error, adGroupIndex);
-    }
+    // Clear any pending content position that triggered attempting to load the ad group.
     pendingContentPositionMs = C.TIME_UNSET;
     pendingContentPositionMs = C.TIME_UNSET;
     fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
     fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
   }
   }
@@ -1522,8 +1505,10 @@ public final class ImaAdsLoader
    * no such ad group.
    * no such ad group.
    */
    */
   private int getLoadingAdGroupIndex() {
   private int getLoadingAdGroupIndex() {
-    long playerPositionUs =
-        C.msToUs(getContentPeriodPositionMs(checkNotNull(player), timeline, period));
+    if (player == null) {
+      return C.INDEX_UNSET;
+    }
+    long playerPositionUs = C.msToUs(getContentPeriodPositionMs(player, timeline, period));
     int adGroupIndex =
     int adGroupIndex =
         adPlaybackState.getAdGroupIndexForPositionUs(playerPositionUs, C.msToUs(contentDurationMs));
         adPlaybackState.getAdGroupIndexForPositionUs(playerPositionUs, C.msToUs(contentDurationMs));
     if (adGroupIndex == C.INDEX_UNSET) {
     if (adGroupIndex == C.INDEX_UNSET) {

extensions/ima/src/test/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoaderTest.java

@@ -48,6 +48,7 @@ import com.google.ads.interactivemedia.v3.api.ImaSdkSettings;
 import com.google.ads.interactivemedia.v3.api.player.AdMediaInfo;
 import com.google.ads.interactivemedia.v3.api.player.AdMediaInfo;
 import com.google.ads.interactivemedia.v3.api.player.ContentProgressProvider;
 import com.google.ads.interactivemedia.v3.api.player.ContentProgressProvider;
 import com.google.ads.interactivemedia.v3.api.player.VideoAdPlayer;
 import com.google.ads.interactivemedia.v3.api.player.VideoAdPlayer;
+import com.google.ads.interactivemedia.v3.api.player.VideoProgressUpdate;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.ExoPlaybackException;
 import com.google.android.exoplayer2.ExoPlaybackException;
 import com.google.android.exoplayer2.MediaItem;
 import com.google.android.exoplayer2.MediaItem;
@@ -312,6 +313,31 @@ public final class ImaAdsLoaderTest {
   }
   }
 
 
   @Test
   @Test
+  public void playback_withMidrollFetchError_updatesContentProgress() {
+    AdEvent mockMidrollFetchErrorAdEvent = mock(AdEvent.class);
+    when(mockMidrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR);
+    when(mockMidrollFetchErrorAdEvent.getAdData())
+        .thenReturn(ImmutableMap.of("adBreakTime", "5.5"));
+    setupPlayback(CONTENT_TIMELINE, ImmutableList.of(5.5f));
+
+    // Simulate loading an empty midroll ad and advancing the player position.
+    imaAdsLoader.start(adsLoaderListener, adViewProvider);
+    adEventListener.onAdEvent(mockMidrollFetchErrorAdEvent);
+    long playerPositionUs = CONTENT_DURATION_US - C.MICROS_PER_SECOND;
+    long playerPositionInPeriodUs =
+        playerPositionUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US;
+    long periodDurationUs =
+        CONTENT_TIMELINE.getPeriod(/* periodIndex= */ 0, new Period()).durationUs;
+    fakeExoPlayer.setPlayingContentPosition(C.usToMs(playerPositionUs));
+
+    // Verify the content progress is updated to reflect the new player position.
+    assertThat(contentProgressProvider.getContentProgress())
+        .isEqualTo(
+            new VideoProgressUpdate(
+                C.usToMs(playerPositionInPeriodUs), C.usToMs(periodDurationUs)));
+  }
+
+  @Test
   public void playback_withPostrollFetchError_marksAdAsInErrorState() {
   public void playback_withPostrollFetchError_marksAdAsInErrorState() {
     AdEvent mockPostrollFetchErrorAdEvent = mock(AdEvent.class);
     AdEvent mockPostrollFetchErrorAdEvent = mock(AdEvent.class);
     when(mockPostrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR);
     when(mockPostrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR);