Repository navigation
Add method to reset ad group to be played again. #9615
Description
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!
Activity
tonihei commentedon Nov 15, 2021
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
AdPlaybackStatethemselves unless they have a customizedAdsLoder.Using
withNewAdGroupadds a new ad group to the list, so this is by design.chladto1 commentedon Nov 18, 2021
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 calladPlaybackState.withNewAdGroup(i, timeUs).*that increase propertyadGroupCountand causes a crash on newly added line 285 in AdsMediaSource. Could you please just add a method to setAdGroupat index?tonihei commentedon Nov 18, 2021
AdPlaybackStateis 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 inAdPlaybackState, for examplewithAdCount,withAdUri,withPlayedAdetc. 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 commentedon Nov 18, 2021
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 commentedon Nov 18, 2021
Issue #8810 is about marking an ad as skipped, which can be achieved with
AdPlaybackState.withSkippedAd.chladto1 commentedon Nov 18, 2021
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. CalladPlaybackState = adPlaybackState.withNewAdGroup(i, adTimeUs)causes crash on line 285 in AdsMediaSource.tonihei commentedon Nov 18, 2021
I'm not sure I understand which part is missing or not working.
withNewAdGroupadds 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/withSkippedAdetc, which you are using already per your last comment.To mark an ad as available, you need to provide an URI via
withAdUri.chladto1 commentedon Nov 18, 2021
URI of an ad is not available at the moment. We need to set
AdPlaybackState.AdGroup()at index witch probably corresponds towithNewAdGroup. There is no any migration guide, so I am just guessing.tonihei commentedon Nov 18, 2021
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 emptyAdGroupafter 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 commentedon Nov 18, 2021
URIs of all
AdGroupsare not available until they are loaded. We want to make exactly that state.AdGroupis available to load. We markAdGroupas skipped and later we markAdGroupas "ready to load".This was available in previous versions of ExoPlayer with standard API. There was not needed reflection until version 2.15.1.
tonihei commentedon Nov 19, 2021
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?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 commentedon Nov 19, 2021
Yes, method like
AdPlaybackState.withResetAdGroup(int adGroupIndex)that set everything to the same state as constructor would be great!tonihei commentedon Nov 19, 2021
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.
[-]Crash in onAdPlaybackState[/-][+]Add method to reset ad group to be played again.[/+]chladto1 commentedon Nov 19, 2021
If I remember correctly, there was some Exo/IMA code that requires exactly that state. Maybe empty
statesorurisarray orcountasC.LENGTH_UNSET. For that reason, we are settingAdPlaybackState.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 commentedon Nov 19, 2021
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 commentedon Nov 19, 2021
Glad to hear it, thanks.
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, ...
Add method to reset ad group from final states to be playable again.
tonihei commentedon Nov 19, 2021
Added the method in the commit above. Closing this issue accordingly.
Add method to reset ad group from final states to be playable again.