Skip to content

Commit

Permalink
Remove flakiness from DefaultAnalyticsCollectorTest
Browse files Browse the repository at this point in the history 
Our FakeClock generally makes sure that playback tests are fully
deterministic. However, this fails if the test uses blocking waits
with clock.onThreadBlocked and where relevant Handlers are created
without using the clock.

To fix the flakiness, we can make the following adjustments:
 - Use TestExoPlayerBuilder instead of legacy ExoPlayerTestRunner
   to avoid onThreadBlocked calls. This also makes the tests more
   readable.
 - Use clock to create Handler for FakeVideoRenderer and
   FakeAudioRenderer. Ideally, this should be passed through
   RenderersFactory, but it's too disruptive given this is a
   public API.
 - Use clock for MediaSourceList and MediaPeriodQueue update
   handler.

PiperOrigin-RevId: 490907495
  • Loading branch information
@tonihei @rohitjoins
tonihei authored and rohitjoins committed Nov 29, 2022
1 parent 7fffe65 commit 7d62943
Show file tree
Hide file tree
Showing 11 changed files with 570 additions and 425 deletions.
2 changes: 1 addition & 1 deletion library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public ExoPlayerImplInternal(

deliverPendingMessageAtStartPositionRequired = true;

Handler eventHandler = new Handler(applicationLooper);
HandlerWrapper eventHandler = clock.createHandler(applicationLooper, /* callback= */ null);
queue = new MediaPeriodQueue(analyticsCollector, eventHandler);
mediaSourceList =
new MediaSourceList(/* listener= */ this, analyticsCollector, eventHandler, playerId);
Expand Down
5 changes: 3 additions & 2 deletions library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
import com.google.android.exoplayer2.upstream.Allocator;
import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.HandlerWrapper;
import com.google.common.collect.ImmutableList;

/**
Expand Down Expand Up @@ -69,7 +70,7 @@
private final Timeline.Period period;
private final Timeline.Window window;
private final AnalyticsCollector analyticsCollector;
private final Handler analyticsCollectorHandler;
private final HandlerWrapper analyticsCollectorHandler;

private long nextWindowSequenceNumber;
private @RepeatMode int repeatMode;
Expand All @@ -89,7 +90,7 @@
* on.
*/
public MediaPeriodQueue(
AnalyticsCollector analyticsCollector, Handler analyticsCollectorHandler) {
AnalyticsCollector analyticsCollector, HandlerWrapper analyticsCollectorHandler) {
this.analyticsCollector = analyticsCollector;
this.analyticsCollectorHandler = analyticsCollectorHandler;
period = new Timeline.Period();
Expand Down
167 changes: 111 additions & 56 deletions library/core/src/main/java/com/google/android/exoplayer2/MediaSourceList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
*/
package com.google.android.exoplayer2;

import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
import static java.lang.Math.max;
import static java.lang.Math.min;

import android.os.Handler;
import android.util.Pair;
import androidx.annotation.Nullable;
import com.google.android.exoplayer2.analytics.AnalyticsCollector;
import com.google.android.exoplayer2.analytics.PlayerId;
Expand All @@ -36,6 +38,7 @@
import com.google.android.exoplayer2.upstream.Allocator;
import com.google.android.exoplayer2.upstream.TransferListener;
import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.HandlerWrapper;
import com.google.android.exoplayer2.util.Log;
import com.google.android.exoplayer2.util.Util;
import java.io.IOException;
Expand All @@ -47,6 +50,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.checkerframework.checker.nullness.compatqual.NullableType;

/**
* Concatenates multiple {@link MediaSource}s. The list of {@link MediaSource}s can be modified
Expand Down Expand Up @@ -76,11 +80,10 @@ public interface MediaSourceListInfoRefreshListener {
private final IdentityHashMap<MediaPeriod, MediaSourceHolder> mediaSourceByMediaPeriod;
private final Map<Object, MediaSourceHolder> mediaSourceByUid;
private final MediaSourceListInfoRefreshListener mediaSourceListInfoListener;
private final MediaSourceEventListener.EventDispatcher mediaSourceEventDispatcher;
private final DrmSessionEventListener.EventDispatcher drmEventDispatcher;
private final HashMap<MediaSourceList.MediaSourceHolder, MediaSourceAndListener> childSources;
private final Set<MediaSourceHolder> enabledMediaSourceHolders;

private final AnalyticsCollector eventListener;
private final HandlerWrapper eventHandler;
private ShuffleOrder shuffleOrder;
private boolean isPrepared;

Expand All @@ -100,20 +103,18 @@ public interface MediaSourceListInfoRefreshListener {
public MediaSourceList(
MediaSourceListInfoRefreshListener listener,
AnalyticsCollector analyticsCollector,
Handler analyticsCollectorHandler,
HandlerWrapper analyticsCollectorHandler,
PlayerId playerId) {
this.playerId = playerId;
mediaSourceListInfoListener = listener;
shuffleOrder = new DefaultShuffleOrder(0);
mediaSourceByMediaPeriod = new IdentityHashMap<>();
mediaSourceByUid = new HashMap<>();
mediaSourceHolders = new ArrayList<>();
mediaSourceEventDispatcher = new MediaSourceEventListener.EventDispatcher();
drmEventDispatcher = new DrmSessionEventListener.EventDispatcher();
eventListener = analyticsCollector;
eventHandler = analyticsCollectorHandler;
childSources = new HashMap<>();
enabledMediaSourceHolders = new HashSet<>();
mediaSourceEventDispatcher.addEventListener(analyticsCollectorHandler, analyticsCollector);
drmEventDispatcher.addEventListener(analyticsCollectorHandler, analyticsCollector);
}

/**
Expand Down Expand Up @@ -307,7 +308,7 @@ public MediaPeriod createPeriod(
Object mediaSourceHolderUid = getMediaSourceHolderUid(id.periodUid);
MediaSource.MediaPeriodId childMediaPeriodId =
id.copyWithPeriodUid(getChildPeriodUid(id.periodUid));
MediaSourceHolder holder = Assertions.checkNotNull(mediaSourceByUid.get(mediaSourceHolderUid));
MediaSourceHolder holder = checkNotNull(mediaSourceByUid.get(mediaSourceHolderUid));
enableMediaSource(holder);
holder.activeMediaPeriodIds.add(childMediaPeriodId);
MediaPeriod mediaPeriod =
Expand All @@ -323,8 +324,7 @@ public MediaPeriod createPeriod(
* @param mediaPeriod The period to release.
*/
public void releasePeriod(MediaPeriod mediaPeriod) {
MediaSourceHolder holder =
Assertions.checkNotNull(mediaSourceByMediaPeriod.remove(mediaPeriod));
MediaSourceHolder holder = checkNotNull(mediaSourceByMediaPeriod.remove(mediaPeriod));
holder.mediaSource.releasePeriod(mediaPeriod);
holder.activeMediaPeriodIds.remove(((MaskingMediaPeriod) mediaPeriod).id);
if (!mediaSourceByMediaPeriod.isEmpty()) {
Expand Down Expand Up @@ -449,8 +449,7 @@ private void prepareChildSource(MediaSourceHolder holder) {
private void maybeReleaseChildSource(MediaSourceHolder mediaSourceHolder) {
// Release if the source has been removed from the playlist and no periods are still active.
if (mediaSourceHolder.isRemoved && mediaSourceHolder.activeMediaPeriodIds.isEmpty()) {
MediaSourceAndListener removedChild =
Assertions.checkNotNull(childSources.remove(mediaSourceHolder));
MediaSourceAndListener removedChild = checkNotNull(childSources.remove(mediaSourceHolder));
removedChild.mediaSource.releaseSource(removedChild.caller);
removedChild.mediaSource.removeEventListener(removedChild.eventListener);
removedChild.mediaSource.removeDrmEventListener(removedChild.eventListener);
Expand Down Expand Up @@ -525,12 +524,8 @@ private final class ForwardingEventListener
implements MediaSourceEventListener, DrmSessionEventListener {

private final MediaSourceList.MediaSourceHolder id;
private MediaSourceEventListener.EventDispatcher mediaSourceEventDispatcher;
private DrmSessionEventListener.EventDispatcher drmEventDispatcher;

public ForwardingEventListener(MediaSourceList.MediaSourceHolder id) {
mediaSourceEventDispatcher = MediaSourceList.this.mediaSourceEventDispatcher;
drmEventDispatcher = MediaSourceList.this.drmEventDispatcher;
this.id = id;
}

Expand All @@ -542,8 +537,14 @@ public void onLoadStarted(
@Nullable MediaSource.MediaPeriodId mediaPeriodId,
LoadEventInfo loadEventData,
MediaLoadData mediaLoadData) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
mediaSourceEventDispatcher.loadStarted(loadEventData, mediaLoadData);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onLoadStarted(
eventParameters.first, eventParameters.second, loadEventData, mediaLoadData));
}
}

Expand All @@ -553,8 +554,14 @@ public void onLoadCompleted(
@Nullable MediaSource.MediaPeriodId mediaPeriodId,
LoadEventInfo loadEventData,
MediaLoadData mediaLoadData) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
mediaSourceEventDispatcher.loadCompleted(loadEventData, mediaLoadData);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onLoadCompleted(
eventParameters.first, eventParameters.second, loadEventData, mediaLoadData));
}
}

Expand All @@ -564,8 +571,14 @@ public void onLoadCanceled(
@Nullable MediaSource.MediaPeriodId mediaPeriodId,
LoadEventInfo loadEventData,
MediaLoadData mediaLoadData) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
mediaSourceEventDispatcher.loadCanceled(loadEventData, mediaLoadData);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onLoadCanceled(
eventParameters.first, eventParameters.second, loadEventData, mediaLoadData));
}
}

Expand All @@ -577,8 +590,19 @@ public void onLoadError(
MediaLoadData mediaLoadData,
IOException error,
boolean wasCanceled) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
mediaSourceEventDispatcher.loadError(loadEventData, mediaLoadData, error, wasCanceled);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onLoadError(
eventParameters.first,
eventParameters.second,
loadEventData,
mediaLoadData,
error,
wasCanceled));
}
}

Expand All @@ -587,8 +611,14 @@ public void onUpstreamDiscarded(
int windowIndex,
@Nullable MediaSource.MediaPeriodId mediaPeriodId,
MediaLoadData mediaLoadData) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
mediaSourceEventDispatcher.upstreamDiscarded(mediaLoadData);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onUpstreamDiscarded(
eventParameters.first, checkNotNull(eventParameters.second), mediaLoadData));
}
}

Expand All @@ -597,8 +627,14 @@ public void onDownstreamFormatChanged(
int windowIndex,
@Nullable MediaSource.MediaPeriodId mediaPeriodId,
MediaLoadData mediaLoadData) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
mediaSourceEventDispatcher.downstreamFormatChanged(mediaLoadData);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onDownstreamFormatChanged(
eventParameters.first, eventParameters.second, mediaLoadData));
}
}

Expand All @@ -609,75 +645,94 @@ public void onDrmSessionAcquired(
int windowIndex,
@Nullable MediaSource.MediaPeriodId mediaPeriodId,
@DrmSession.State int state) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
drmEventDispatcher.drmSessionAcquired(state);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onDrmSessionAcquired(
eventParameters.first, eventParameters.second, state));
}
}

@Override
public void onDrmKeysLoaded(
int windowIndex, @Nullable MediaSource.MediaPeriodId mediaPeriodId) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
drmEventDispatcher.drmKeysLoaded();
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() -> eventListener.onDrmKeysLoaded(eventParameters.first, eventParameters.second));
}
}

@Override
public void onDrmSessionManagerError(
int windowIndex, @Nullable MediaSource.MediaPeriodId mediaPeriodId, Exception error) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
drmEventDispatcher.drmSessionManagerError(error);
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onDrmSessionManagerError(
eventParameters.first, eventParameters.second, error));
}
}

@Override
public void onDrmKeysRestored(
int windowIndex, @Nullable MediaSource.MediaPeriodId mediaPeriodId) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
drmEventDispatcher.drmKeysRestored();
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() -> eventListener.onDrmKeysRestored(eventParameters.first, eventParameters.second));
}
}

@Override
public void onDrmKeysRemoved(
int windowIndex, @Nullable MediaSource.MediaPeriodId mediaPeriodId) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
drmEventDispatcher.drmKeysRemoved();
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() -> eventListener.onDrmKeysRemoved(eventParameters.first, eventParameters.second));
}
}

@Override
public void onDrmSessionReleased(
int windowIndex, @Nullable MediaSource.MediaPeriodId mediaPeriodId) {
if (maybeUpdateEventDispatcher(windowIndex, mediaPeriodId)) {
drmEventDispatcher.drmSessionReleased();
@Nullable
Pair<Integer, MediaSource.@NullableType MediaPeriodId> eventParameters =
getEventParameters(windowIndex, mediaPeriodId);
if (eventParameters != null) {
eventHandler.post(
() ->
eventListener.onDrmSessionReleased(eventParameters.first, eventParameters.second));
}
}

/** Updates the event dispatcher and returns whether the event should be dispatched. */
private boolean maybeUpdateEventDispatcher(
/** Updates the event parameters and returns whether the event should be dispatched. */
@Nullable
private Pair<Integer, MediaSource.@NullableType MediaPeriodId> getEventParameters(
int childWindowIndex, @Nullable MediaSource.MediaPeriodId childMediaPeriodId) {
@Nullable MediaSource.MediaPeriodId mediaPeriodId = null;
if (childMediaPeriodId != null) {
mediaPeriodId = getMediaPeriodIdForChildMediaPeriodId(id, childMediaPeriodId);
if (mediaPeriodId == null) {
// Media period not found. Ignore event.
return false;
return null;
}
}
int windowIndex = getWindowIndexForChildWindowIndex(id, childWindowIndex);
if (mediaSourceEventDispatcher.windowIndex != windowIndex
|| !Util.areEqual(mediaSourceEventDispatcher.mediaPeriodId, mediaPeriodId)) {
mediaSourceEventDispatcher =
MediaSourceList.this.mediaSourceEventDispatcher.withParameters(
windowIndex, mediaPeriodId, /* mediaTimeOffsetMs= */ 0L);
}
if (drmEventDispatcher.windowIndex != windowIndex
|| !Util.areEqual(drmEventDispatcher.mediaPeriodId, mediaPeriodId)) {
drmEventDispatcher =
MediaSourceList.this.drmEventDispatcher.withParameters(windowIndex, mediaPeriodId);
}
return true;
return Pair.create(windowIndex, mediaPeriodId);
}
}
}
Loading

0 comments on commit 7d62943

Please sign in to comment.