Cannot control behaviour of ads in AdsMediaSource

richjhart commented 6 years ago

Issue description

It should be possible to control whether an ad plays at the point of seeking. Currently, ANY seek results in a previous ad playing. We only want ads to play if the time hits the ad start time.

Reproduction steps

I have a set of ads, defined manually using:

            ArrayList<Serializable[]> ads = ...
            long[] times = new long[ads.size()];
            for (int i = 0; i < ads.size(); i++) {
                times[i] = (long) ads.get(i)[Movie.ADS_TIME_INDEX] * 1000000;
            }
            mAdPlaybackState = new AdPlaybackState(times);
            for (int i = 0; i < ads.size(); i++) {
                mAdPlaybackState.adGroups[i] = mAdPlaybackState.adGroups[i].withAdCount(1);
                mAdPlaybackState.adGroups[i] = mAdPlaybackState.adGroups[i].withAdUri(Uri.parse((String) ads.get(i)[Movie.ADS_URL]), 0);
            }
            eventListener.onAdPlaybackState(mAdPlaybackState);

Which works fine if you just let it play though. But as soon as you start seeking, it always plays a previous ad (if there is one). We should have more control over this.

I have tried a number of work arounds, such as changing the AdPlaybackState, but this seems to muck up the timeline as the number of periods is wrong.

I have also tried just changing the media source provided so that it passes a 0-length source if it shouldn't be playing ads. But this doesn't work because it remembers the AdMediaSources once they are created.

Link to test content

This behaviour happens with any HLS stream as the main stream and the ads we use are MP4.

Version of ExoPlayer being used

2.7.1

Device(s) and version(s) of Android being used

OnePlus 5, Android 8.0

A full bug report captured from the device

N/A

andrewlewis commented 6 years ago

The code that determines what ad to play when seeking to a position currently assumes that an ad group gates access to the following piece of content, which matches the behavior of the IMA SDK. See MediaPeriodQueue, which delegates to Timeline and AdPlaybackState.

I've marked this as an enhancement to track adding a way for the application to customize whether to play a preceding ad group when seeking, but, at the moment, I don't think we'll get round to it soon. I guess the option should be stored in the AdPlaybackState, so any ads loader can use this if it wants, then Timeline and MediaPeriodQueue would need to be updated to support this.

Finally, it's a bit hacky, but you could try intercepting the seek request and marking the preceding ad group as played immediately. I think you'd need to recreate the media source on seeking before the preceding ad group as you can't mark a played ad group as unplayed, so this may not be a viable option.

richjhart commented 6 years ago

Thanks for the reply. It would certainly be handy to be able to override the decision at runtime but understand that may take time.

However, while I can understand the gate part, there is a part that doesn't feel right to me. If you seek WITHIN a section, it still plays the preceding ad. I would have thought that it would only gate when switching sections. I have created a small project which highlights this at: https://bitbucket.org/richjhart/exoplayertest/overview (first commit on master).

There are ads at 2 min, 4 min, 6 min and 8 min. If you jump to anywhere in the first 2 minutes, it plays no ad (no matter where you jump from). If you jump to anywhere else, it plays the most recent ad - even jumping from 3:00 to 3;30 will play the ad from 2:00.

So while the gating is OK (even though it doesn't actually match our current design), ads playing on any seek is a problem. So I'm going to try a couple more "hacky" options to see if I can get anything going in the mean time.

andrewlewis commented 6 years ago

I'm a bit confused about part of your comment "If you seek WITHIN a section, it still plays the preceding ad. I would have thought that it would only gate when switching sections.". How can you seek within a section without having played its immediately preceding ad already? Is the distinction between playback advancing from one section to the next without a seek, versus the user seeking to the next section? I think the rationale is that the user could skip over an ad the publisher wanted to play just by seeking past an ad group immediately before the ad group would have played.

Just want to make sure we are on the same page about how this works, in case there's a bug somewhere. Thanks!

richjhart commented 6 years ago

I should go into a bit more detail about what I'm looking for. There are two different but related scenarios I'm thinking of really:

1)

Video automatically jumps to a point some time into the video when the user loads it (e.g. due to having previously watched part of it). Because of business requirements, no ad should play at this point.
But the only way to do this is to set all ads prior to that point to skipped
So if the user then seeks back to before the ad, the ads can't be reset to unplayed to then play next time they reach that section

2)

Video plays an ad guarding a section (due to either advancing or seeking)
At this point, the user should be able to move around within that section freely. To do this, the ad (group) must be set as played
But again, if the user skips back to a previous section, it doesn't seem unreasonable to want to "re-arm" the ad

It occurs to me that it shouldn't be too difficult to allow this decision. When the user seeks between sections, or within a section, a decision must be being made somewhere to alter the timeline, adding a period in for the ad (because without the seek, there is no ad to be played) - so that decision just needs to be intercepted before any changes are made. (I'm not suggesting that means I would expect it quickly - just wondering aloud about modifying the library for my own purposes)

andrewlewis commented 6 years ago

It looks like you're not using ImaAdsLoader (where IMA dictates exactly what ad should play when) but instead implementing your own AdsLoader, so:

(1) should be possible. Although ImaAdsLoader marks ads before a resume position as played, custom implementations don't need to do that, and I think it would work just to leave them unplayed. You could just mark the one ad group before the initial seek position as played. If you're implementing your own system you can even persist what ads were played in the previous playback session and restore this information when you resume! Please let me know if this doesn't work and I'll take a look.
If I understand correctly, (2) isn't possible at the moment because an ad group can't be marked as unplayed once it's been played (or is in the error state). Ad groups before the one preceding a non-initial seek destination shouldn't be marked as played automatically, however.

google / ExoPlayer