Add method to reset ad group to be played again.

[chladto1]

We are updating ExoPlayer in our project from version 2.13.2 to 2.15.1. We are using modified IMA extension because of feature described in issue 8810. We had to change adPlaybackState.adGroups[i] = AdPlaybackState.AdGroup().withAdCount(1).withAdState(AdPlaybackState.AD_STATE_PLAYED, 0) to adPlaybackState = adPlaybackState.withNewAdGroup(i, timeUs).withAdCount(i, 1).withPlayedAd(i, 0) and adPlaybackState.adGroups[i] = AdPlaybackState.AdGroup() to adPlaybackState = adPlaybackState.withNewAdGroup(i, adTimeUs). But it looks like there are some internal changes and our application is crashing in newly added check on line 285 in AdsMediaSource that was not in 2.13.2 version.

Could you please help us how to set AdGroup directly into AdPlaybackState.adGroups and avoid increasing the AdPlaybackState.adGroupCount? Our code is in Kotlin language and we created extension AdPlaybackState.setAdGroup(index: Int, adGroup: AdPlaybackState.AdGroup) that is using reflection to set an AdGroup into AdPlaybackState.adGroups. But using reflection is not the best solution. Thanks!

[tonihei]

I'm not aware of any removed methods that no longer work, so I think whatever you are planning to do in the old code should still work the same in the new code. A general note in that regard: It's unclear what you are trying to achieve exactly and usually we don't expect apps to update AdPlaybackState themselves unless they have a customized AdsLoder.

void increasing the AdPlaybackState.adGroupCount

Using withNewAdGroup adds a new ad group to the list, so this is by design.

[chladto1]

There is no ability to set AdGroup at specific index in the new version. In 2.13.2 we can call adPlaybackState.adGroups[i] = *. In 2.15.1 we must call adPlaybackState.withNewAdGroup(i, timeUs).* that increase property adGroupCount and causes a crash on newly added line 285 in AdsMediaSource. Could you please just add a method to set AdGroup at index?

[tonihei]

AdPlaybackState is supposed to be an immutable class and changing the existing array members was never a valid usage. You should be able to modify an existing ad with the various helper methods in AdPlaybackState, for example withAdCount, withAdUri, withPlayedAd etc. If there is something specific you need to do that isn't possible via these methods, please describe your use case in more detail and we might be able to add additional helper methods.

[chladto1]

Use case is described in issue 8810 and I shared you our code. I can share you our current code that we are using in 2.15.1. Is that what would help?

[tonihei]

Issue #8810 is about marking an ad as skipped, which can be achieved with AdPlaybackState.withSkippedAd.

[chladto1]

Yes, we are doing that, but to mark them available, we were call adPlaybackState.adGroups[i] = AdPlaybackState.AdGroup() which is not possible in the new version. Call adPlaybackState = adPlaybackState.withNewAdGroup(i, adTimeUs) causes crash on line 285 in AdsMediaSource.

[tonihei]

I'm not sure I understand which part is missing or not working.

• withNewAdGroup adds a new group, and this is not what you want to do if I understand correctly. Instead you want to change an existing ad group.

• Changing an existing ad group can be done by calling AdPlaybackState.withAdCount/withPlayedAd/withSkippedAd etc, which you are using already per your last comment.

• but to mark them available, we were call adPlaybackState.adGroups[i] = AdPlaybackState.AdGroup()

  To mark an ad as available, you need to provide an URI via withAdUri.

[chladto1]

URI of an ad is not available at the moment. We need to set AdPlaybackState.AdGroup() at index witch probably corresponds to withNewAdGroup. There is no any migration guide, so I am just guessing.

[tonihei]

If the URI of the ad is not yet available, then you can't mark the ad as available because it would violate state assumption we have about AdGroup. Assigning an empty AdGroup after it has previously been non-empty is also not supported because the player assumes the number of ads can only grow, not shrink.

Other modifications are allowed: You can add new ads in the group, you can set the ad durations, you can set the state to played/skipped/error, or provide an URI to set the state back to available.
So I still don't see which method is missing exactly - which state of the existing ad group do you need to modify that can't be modified at the moment?

[chladto1]

If the URI of the ad is not yet available, then you can't mark the ad as available because it would violate state assumption we have about AdGroup.

URIs of all AdGroups are not available until they are loaded. We want to make exactly that state. AdGroup is available to load. We mark AdGroup as skipped and later we mark AdGroup as "ready to load".

Assigning an empty AdGroup after it has previously been non-empty is also not supported because the player assumes the number of ads can only grow, not shrink.

This was available in previous versions of ExoPlayer with standard API. There was not needed reflection until version 2.15.1.

[tonihei]

URIs of all AdGroups are not available until they are loaded. We want to make exactly that state. AdGroup is available to load. We mark AdGroup as skipped and later we mark AdGroup as "ready to load".

There is no "ready to load" state I think because you'd need to either reset it to UNAVAILABLE or AVAILABLE, depending on whether the ad URI has already been set.

So to clarify, you need a method to reset the final ad states (SKIPPED, ERROR, PLAYED) back to (UNAVAILABLE or AVAILABLE), allowing the player to pick up the ad again for playback the next time it plays at this position. The method would look something like AdPlaybackState.withResetAdGroup(int adGroupIndex). Would that help to solve your problem?

This was available in previous versions of ExoPlayer with standard API.

That was never a valid API we intended to be used. It's more of an oversight that let you modify array elements that should have unmodifiable.

[chladto1]

Yes, method like AdPlaybackState.withResetAdGroup(int adGroupIndex) that set everything to the same state as constructor would be great!

[tonihei]

that set everything to the same state as constructor

It wouldn't :). The method would keep ad count, ad URIs, the ad group time and known ad durations. But if I understand the requirements correctly, this is also what you need to reset the skipped state.

[chladto1]

If I remember correctly, there was some Exo/IMA code that requires exactly that state. Maybe empty states or uris array or count as C.LENGTH_UNSET. For that reason, we are setting AdPlaybackState.AdGroup(adTimeUs) at index. Other solutions causes an errors. I can send you our actual code and you can try it. Maybe you will find a better solution :)

[tonihei]

I tested setting all ads to SKIPPED initially (to prevent playing them although they are loaded) and then after a while I reset just their state back to AVAILABLE (keeping all other values). After this point they started playing again as expected, so I think such a method would solve the general problem of resetting ads.

Depending on your customization of the ads loader, it could be that you need to update the states in more places or make sure IMA receives the expected positions etc., but that's down to your app customization and we can't provide detailed advise on how to customize the components correctly. The enhancement tracked by #8810 is also still not solved, so you might see short rebufferings when you change the ad state while the previous content piece is already playing.

[chladto1]

After this point they started playing again as expected, so I think such a method would solve the general problem of resetting ads.

Glad to hear it, thanks.

you need to update the states in more places or make sure IMA receives the expected positions etc

Yes, I know. We are keeping track with the original IMA extension, but our version has a lot of modifications to "solve" save instance state, our smart seek feature, delayed responses, UI customizations, ...

[tonihei]

Added the method in the commit above. Closing this issue accordingly.