Skip to content

Commit

Permalink
Proposed improved DASH EOS detection
Browse files Browse the repository at this point in the history
  • Loading branch information
@ojw28
ojw28 committed Oct 15, 2018
1 parent 6fb8f66 commit f70cc75
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 68 deletions.
24 changes: 6 additions & 18 deletions library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,6 @@ public int[] getSupportedTypes() {

private int staleManifestReloadAttempt;
private long expiredManifestPublishTimeUs;
private boolean dynamicMediaPresentationEnded;

private int firstPeriodId;

Expand Down Expand Up @@ -679,7 +678,6 @@ public void releaseSourceInternal() {
elapsedRealtimeOffsetMs = 0;
staleManifestReloadAttempt = 0;
expiredManifestPublishTimeUs = C.TIME_UNSET;
dynamicMediaPresentationEnded = false;
firstPeriodId = 0;
periodsById.clear();
}
Expand All @@ -691,10 +689,6 @@ public void releaseSourceInternal() {
startLoadingManifest();
}

/* package */ void onDashLiveMediaPresentationEndSignalEncountered() {
this.dynamicMediaPresentationEnded = true;
}

/* package */ void onDashManifestPublishTimeExpired(long expiredManifestPublishTimeUs) {
if (this.expiredManifestPublishTimeUs == C.TIME_UNSET
|| this.expiredManifestPublishTimeUs < expiredManifestPublishTimeUs) {
Expand Down Expand Up @@ -734,9 +728,8 @@ public void releaseSourceInternal() {
// behind.
Log.w(TAG, "Loaded out of sync manifest");
isManifestStale = true;
} else if (dynamicMediaPresentationEnded
|| (expiredManifestPublishTimeUs != C.TIME_UNSET
&& newManifest.publishTimeMs * 1000 <= expiredManifestPublishTimeUs)) {
} else if (expiredManifestPublishTimeUs != C.TIME_UNSET
&& newManifest.publishTimeMs * 1000 <= expiredManifestPublishTimeUs) {
// If we receive a dynamic manifest that's older than expected (i.e. its publish time has
// expired, or it's dynamic and we know the presentation has ended), then this manifest is
// stale.
Expand All @@ -745,8 +738,6 @@ public void releaseSourceInternal() {
"Loaded stale dynamic manifest: "
+ newManifest.publishTimeMs
+ ", "
+ dynamicMediaPresentationEnded
+ ", "
+ expiredManifestPublishTimeUs);
isManifestStale = true;
}
Expand All @@ -763,7 +754,6 @@ public void releaseSourceInternal() {
staleManifestReloadAttempt = 0;
}


manifest = newManifest;
manifestLoadPending &= manifest.dynamic;
manifestLoadStartTimestampMs = elapsedRealtimeMs - loadDurationMs;
Expand Down Expand Up @@ -1175,7 +1165,10 @@ public Window getWindow(
presentationStartTimeMs,
windowStartTimeMs,
/* isSeekable= */ true,
manifest.dynamic,
// TODO: Split so that we can disambiguate between dynamic cases where media may be
// appended, and those where media can only be removed. The former should not prevent
// transition to the ended state.
manifest.dynamic && manifest.minUpdatePeriodMs != C.TIME_UNSET,

This comment has been minimized.

Sign in to view

Copy link
@ojw28

ojw28 Oct 15, 2018

Author Contributor

Edit to manifest.dynamic && manifest.durationMs == C.TIME_UNSET for testing.

In a final version, I'd propose adding a new Timeline.Window.isEnded field. isDynamic will keep its existing meaning (and be set as in this change). isEnded will be set to !manifest.dynamic || manifest.durationMs != C.TIME_UNSET, and will mean that no further media will be appended. If isEnded and isDynamic are both true, that indicates a window that may be getting smaller, but to which nothing will be appended (and so transitioning to STATE_ENDED should be allowed).

This comment has been minimized.

Sign in to view

Copy link
@peddisri

peddisri Oct 25, 2018 •
edited
Loading

Contributor

Is there a real need for another variable isEnded? Can' we change the nature of window to static when we encounter this scenario? IMO, it will be a bit confusing - just from the Timeline/Window concept perspective

This comment has been minimized.

Sign in to view

Copy link
@ojw28

ojw28 Nov 13, 2018 •
edited
Loading

Author Contributor

There is a need to split the variable. Without splitting there's no way to describe a window where:

  • It's known that nothing will be added to the end, so the player should be able to transition to the ended state.
  • Content may be removed from the beginning

Which is exactly the case we need to describe for DASH manifests that are marked as dynamic, have a set value for mininimumUpdatePeriod, and have a set duration. To guarantee that nothing can be removed from the beginning the server would need to tell us that by removing minimumUpdatePeriod.
windowDefaultStartPositionUs,
windowDurationUs,
/* firstPeriodIndex= */ 0,
Expand Down Expand Up @@ -1253,11 +1246,6 @@ public void onDashManifestRefreshRequested() {
public void onDashManifestPublishTimeExpired(long expiredManifestPublishTimeUs) {
DashMediaSource.this.onDashManifestPublishTimeExpired(expiredManifestPublishTimeUs);
}

@Override
public void onDashLiveMediaPresentationEndSignalEncountered() {
DashMediaSource.this.onDashLiveMediaPresentationEndSignalEncountered();
}
}

private final class ManifestCallback implements Loader.Callback<ParsingLoadable<DashManifest>> {
Expand Down
15 changes: 8 additions & 7 deletions .../dash/src/main/java/com/google/android/exoplayer2/source/dash/DefaultDashChunkSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,12 @@ public void getNextChunk(
}
}

long periodDurationUs = representationHolder.periodDurationUs;
boolean periodEnded = periodDurationUs != C.TIME_UNSET;

if (representationHolder.getSegmentCount() == 0) {
// The index doesn't define any segments.
out.endOfStream = !manifest.dynamic || (periodIndex < manifest.getPeriodCount() - 1);
out.endOfStream = periodEnded;
return;
}

Expand All @@ -343,17 +346,15 @@ public void getNextChunk(
fatalError = new BehindLiveWindowException();
return;
}

if (segmentNum > lastAvailableSegmentNum
|| (missingLastSegment && segmentNum >= lastAvailableSegmentNum)) {
// The segment is beyond the end of the period. We know the period will not be extended if the
// manifest is static, or if there's a period after this one.
out.endOfStream = !manifest.dynamic || (periodIndex < manifest.getPeriodCount() - 1);
// The segment is beyond the end of the period.
out.endOfStream = periodEnded;
return;
}

long periodDurationUs = representationHolder.periodDurationUs;
if (periodDurationUs != C.TIME_UNSET
&& representationHolder.getSegmentStartTimeUs(segmentNum) >= periodDurationUs) {
if (periodEnded && representationHolder.getSegmentStartTimeUs(segmentNum) >= periodDurationUs) {
// The period duration clips the period to a position before the segment.
out.endOfStream = true;
return;
Expand Down
54 changes: 11 additions & 43 deletions library/dash/src/main/java/com/google/android/exoplayer2/source/dash/PlayerEmsgHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@
*/
public final class PlayerEmsgHandler implements Handler.Callback {

private static final int EMSG_MEDIA_PRESENTATION_ENDED = 1;
private static final int EMSG_MANIFEST_EXPIRED = 2;
private static final int EMSG_MANIFEST_EXPIRED = 1;

/** Callbacks for player emsg events encountered during DASH live stream. */
public interface PlayerEmsgCallback {
Expand All @@ -75,9 +74,6 @@ public interface PlayerEmsgCallback {
* @param expiredManifestPublishTimeUs The manifest publish time that has been expired.
*/
void onDashManifestPublishTimeExpired(long expiredManifestPublishTimeUs);

/** Called when a media presentation end signal is encountered during live stream. * */
void onDashLiveMediaPresentationEndSignalEncountered();
}

private final Allocator allocator;
Expand All @@ -88,7 +84,6 @@ public interface PlayerEmsgCallback {

private DashManifest manifest;

private boolean dynamicMediaPresentationEnded;
private long expiredManifestPublishTimeUs;
private long lastLoadedChunkEndTimeUs;
private long lastLoadedChunkEndTimeBeforeRefreshUs;
Expand Down Expand Up @@ -134,21 +129,15 @@ public void updateManifest(DashManifest newManifest) {
return true;
}
boolean manifestRefreshNeeded = false;
if (dynamicMediaPresentationEnded) {
// The manifest we have is dynamic, but we know a non-dynamic one representing the final state
// should be available.
manifestRefreshNeeded = true;
} else {
// Find the smallest publishTime (greater than or equal to the current manifest's publish
// time) that has a corresponding expiry time.
Map.Entry<Long, Long> expiredEntry = ceilingExpiryEntryForPublishTime(manifest.publishTimeMs);
if (expiredEntry != null) {
long expiredPointUs = expiredEntry.getValue();
if (expiredPointUs < presentationPositionUs) {
expiredManifestPublishTimeUs = expiredEntry.getKey();
notifyManifestPublishTimeExpired();
manifestRefreshNeeded = true;
}
// Find the smallest publishTime (greater than or equal to the current manifest's publish
// time) that has a corresponding expiry time.
Map.Entry<Long, Long> expiredEntry = ceilingExpiryEntryForPublishTime(manifest.publishTimeMs);
if (expiredEntry != null) {
long expiredPointUs = expiredEntry.getValue();
if (expiredPointUs < presentationPositionUs) {
expiredManifestPublishTimeUs = expiredEntry.getKey();
notifyManifestPublishTimeExpired();
manifestRefreshNeeded = true;
}
}
if (manifestRefreshNeeded) {
Expand Down Expand Up @@ -221,9 +210,6 @@ public boolean handleMessage(Message message) {
return true;
}
switch (message.what) {
case (EMSG_MEDIA_PRESENTATION_ENDED):
handleMediaPresentationEndedMessageEncountered();
return true;
case (EMSG_MANIFEST_EXPIRED):
ManifestExpiryEventInfo messageObj = (ManifestExpiryEventInfo) message.obj;
handleManifestExpiredMessage(
Expand All @@ -248,11 +234,6 @@ private void handleManifestExpiredMessage(long eventTimeUs, long manifestPublish
}
}

private void handleMediaPresentationEndedMessageEncountered() {
dynamicMediaPresentationEnded = true;
notifySourceMediaPresentationEnded();
}

private @Nullable Map.Entry<Long, Long> ceilingExpiryEntryForPublishTime(long publishTimeMs) {
return manifestPublishTimeToExpiryTimeUs.ceilingEntry(publishTimeMs);
}
Expand All @@ -273,10 +254,6 @@ private void notifyManifestPublishTimeExpired() {
playerEmsgCallback.onDashManifestPublishTimeExpired(expiredManifestPublishTimeUs);
}

private void notifySourceMediaPresentationEnded() {
playerEmsgCallback.onDashLiveMediaPresentationEndSignalEncountered();
}

/** Requests DASH media manifest to be refreshed if necessary. */
private void maybeNotifyDashManifestRefreshNeeded() {
if (lastLoadedChunkEndTimeBeforeRefreshUs != C.TIME_UNSET
Expand Down Expand Up @@ -413,16 +390,7 @@ private void parsePlayerEmsgEvent(long eventTimeUs, EventMessage eventMessage) {
if (manifestPublishTimeMsInEmsg == C.TIME_UNSET) {
return;
}

if (isMessageSignalingMediaPresentationEnded(eventMessage)) {
onMediaPresentationEndedMessageEncountered();
} else {
onManifestExpiredMessageEncountered(eventTimeUs, manifestPublishTimeMsInEmsg);
}
}

private void onMediaPresentationEndedMessageEncountered() {
handler.sendMessage(handler.obtainMessage(EMSG_MEDIA_PRESENTATION_ENDED));
onManifestExpiredMessageEncountered(eventTimeUs, manifestPublishTimeMsInEmsg);
}

private void onManifestExpiredMessageEncountered(
Expand Down

1 comment on commit f70cc75

@peddisri
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's quite a lot of change. Let me go through it and test. Thanks for the patch.

Please sign in to comment.