Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DASH] Possible improvement to detecting end of live events #4780

Closed
juechemparathy opened this issue Sep 5, 2018 · 29 comments
Closed

[DASH] Possible improvement to detecting end of live events #4780

juechemparathy opened this issue Sep 5, 2018 · 29 comments
Assignees
Labels

Comments

@juechemparathy
Copy link

Any pointers to Exoplayer support for detecting when a DASH Live stream has finished.Thanks.

@tonihei
Copy link
Collaborator

tonihei commented Sep 6, 2018

If the stream is actually finished, I'd assume that you receive an onPlayerStateChanged event with the playback state Player.STATE_ENDED. That depends on the manifest however. Do you have an example for a manifest with an ended live stream?

@tonihei tonihei self-assigned this Sep 6, 2018
@juechemparathy
Copy link
Author

juechemparathy commented Sep 6, 2018

@tonihei - Here is the test url - https://content.downlynk.com/channel/29e9c85c070f413fb71a3b2a39eed208.mpd?xtest=end30&prettydash=1
I don't see the playback state as Player.STATE_ENDED on end.

@juechemparathy
Copy link
Author

juechemparathy commented Sep 6, 2018

@tonihei @ojw28
There are really 2 things to pay attention in the manifest -

The lack of a minimumUpdatePeriod on a dynamic manifest, and the addition of MPD presentation duration.
so when you see those things the live event has been finalized at a final length. The addition of the presentation duration is the final length and the removal of the minUpdatePeriod signals that no further manifest queries are necessary.

Please suggest if this doesn't make sense or there is some other standard way of handling this.Thanks.

@ojw28
Copy link
Contributor

ojw28 commented Sep 6, 2018

I think you should be turning type="dynamic" to type="static" when the manifest stops updating.

@tonihei
Copy link
Collaborator

tonihei commented Sep 7, 2018

Yes, that is what I was looking for. We only use the type tag to determine whether we should expect further dynamic updates (and not end the playback) or whether the playback can be safely ended. Please change this tag if possible.

@tonihei tonihei closed this as completed Sep 7, 2018
@juechemparathy
Copy link
Author

Thanks @ojw28 @tonihei for the quick response. Having type=static for the last mpd triggered Player.STATE_ENDED correctly.

Any suggestions on detecting live streaming in this case. Currently we use boolean isLive = player.isCurrentWindowDynamic() || player.getDuration() == C.TIME_UNSET to dectect if its live streaming.Thanks.

@tonihei
Copy link
Collaborator

tonihei commented Sep 10, 2018

It's probably better just to check for player.isCurrentWindowDynamic(). When playing progressive streams (like Mp4 etc.) you'll always get a unset duration in the beginning until it becomes known - and you wouldn't want to classify such a case as live stream.

For the ended live stream case, it's slightly unclear how they differ from non-live streams from a player point of view. Both have a non-changing manifest. If your live streams usually only have a part of the period available, you could check for a non-zero offset in timeline.getWindow(0, new Window()).positionInFirstPeriodUs. But that depends on the type of content.

@peddisri
Copy link
Contributor

peddisri commented Sep 25, 2018

I think I found one issue. I have one URL where the way a presentation ends is by signalling inband via emsg box in fmp4 . I see that the player is detecting the end of presentation, but is trying to either retry, or after max retry is over, throw a DashManifestStaleException.
https://github.com/google/ExoPlayer/blob/release-v2/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java#L682-L689

IMO, if dynamicMediaPresentationEnded is true, there is no need to retry, and instead it should take the player to ENDED state. So this check needs to be re-looked.

https://github.com/google/ExoPlayer/blob/release-v2/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java#L665

@ojw28
Copy link
Contributor

ojw28 commented Sep 25, 2018

If the presentation end has been signalled in-band, then shouldn't any attempt to request the manifest result in a static manifest being received? In which case you'd never hit the code paths you reference. It sounds more likely that the server is not updating the manifest to be static, which I think it should do.

@peddisri
Copy link
Contributor

peddisri commented Oct 1, 2018

I don't think so. I'm trying to find if the spec mentions that explicitly or not. My high level understanding from the spec is that, a static MPD is primarily used for for a VIdeo-on-Demand, whereas Dynamic MPD is used for Live streaming, which can end, but not necessarily be streamed as VOD later. I've sent the sample content URL to the email id.

Section 4.5.2.1 says
"the event duration expresses the remaining duration of Media Presentation from the event time. If the event duration is 0, Media Presentation ends at the event time. If 0xFFFF, the media presentation duration is unknown. In the case in which both presentation_time_delta and event_duration are zero, then the Media Presentation is ended."

@ojw28
Copy link
Contributor

ojw28 commented Oct 1, 2018

Are you removing @minimumUpdatePeriod and finalizing the duration, as described by @juechemparathy further up? I can see the argument for that being sufficient to transition to the ended state (although given you're having to update the manifest to do this, it feels like it's easy to also change it to static).

It's definitely not sufficient to only signal with an in-band message IMO. Reopening primarily to consider the @minimumUpdatePeriod + finalized duration case.

@ojw28 ojw28 reopened this Oct 1, 2018
@ojw28 ojw28 assigned ojw28 and unassigned tonihei Oct 1, 2018
@ojw28 ojw28 changed the title [DASH] Detect when LIVE event has ended [DASH] Possible improvement to detecting end of live events Oct 1, 2018
@peddisri
Copy link
Contributor

peddisri commented Oct 3, 2018

The manifest finalizes the duration by adding mediaPresenationDuration and the duration of the period. However, mininumUpdatePeriod is not removed.

The thing is the content does not belong to us. So we don't have control on how it should behave.

I'm referring to section 4.5 of https://dashif.org/docs/DASH-IF-IOP-v4.2-clean.htm and it does not specify that that the type of mpd should be changed from dynamic to static for the client to detect end of live event.

I've sent the URL to the dev email id

@ojw28
Copy link
Contributor

ojw28 commented Oct 3, 2018

We'll look into removing the assumption that the type should change from dynamic to static. It looks like removal of @minimumUpdatePeriod is required, so it's fairly likely that will remain a requirement for ExoPlayer.

It's the responsibility of the content provider to ensure producing compliant content.

@ojw28 ojw28 added bug and removed question labels Oct 3, 2018
@juechemparathy
Copy link
Author

@ojw28 - Much better. It wasn't a big deal for us as we have control over the content.
@peddisri - Thanks for bringing this up. 👍

@peddisri
Copy link
Contributor

peddisri commented Oct 4, 2018

Unfortunately, as per the interopability guidelines , service of type 4.5 " MPD and Segment-based Live Service Offering" - there is no such requirement of removing "minimumUpdatePeriod". On the contrary, it mandates it to be present.

@peddisri
Copy link
Contributor

peddisri commented Oct 4, 2018

I did minor changes and looks like we are not able to handle end of live stream. Changes attached. (not sending pull request, since I'm not fully confident of the changes). This is just for discussion.
patch.zip

@ojw28
Copy link
Contributor

ojw28 commented Oct 4, 2018

We can't rely on the in-band message to determine whether the presentation has ended. You must be able to tell only from the manifest. Suppose the client hasn't requested any segments yet. If it can't tell that the presentation has ended from the manifest, it will likely try and request segments that don't exist.

If we inferred the presentation has ended only based on the existence of mediaPresentationDuration, would that work for you? On thinking it through a bit more, perhaps that makes sense. I can envisage a case where it's known that nothing will be added to a manifest, but segments are still being removed from the beginning. In which case minimumUpdatePeriod would still be present and the type would still be dynamic, but the player should be able to transition to the ended state if it reaches the end. Looking for a set mediaPresentationDuration is the only way I can think of for the player to determine that.

Does that make sense? And would it work for your case?

@peddisri
Copy link
Contributor

peddisri commented Oct 4, 2018

Yeah and that's what I've tried to do in DefaultDashChunkSource i.e depend on mediaPresentationDuration to decide if the playback has ended or not. But for that I had to allow getNextChunk proceed to the part where it sets endofstream flag on the chunk by not forcing a manifest refresh when dynamicMediaPresentationEnded is set to true in PlayerEmsgHandler.

Although, to decide if this is the right way to do, we should revisit the spec and make sure that presence of mediaPresentationDuration really means that no more data will be added at the end. (I mean It could also mean that this is the current known duration of the presentation)

Finally, to get the player to really go to ENDED state, I had to change the Timeline to static instead of dynamic, and for that I used the in-band message. Perhaps I could use the presence of mediaPresentationDuration itself to set the timeline to static so that there is no dependency on in-band messages. Would that be okay?

@ojw28
Copy link
Contributor

ojw28 commented Oct 15, 2018

This makes sense. I think more broadly we should probably:

  1. Get rid of EMSG_MEDIA_PRESENTATION_ENDED and related methods/logic completely, and just handle those events using the regular EMSG_MANIFEST_EXPIRED. This simplifies your patch a little (e.g. dynamicMediaPresentationEnded goes away completely).
  2. Have DefaultDashChunkSource set endOfStream based on whether the period duration is known. It shouldn't be necessary to look at manifest.dynamic, manifest.durationMs or whether it's the last period in the manifest. I think just looking at the period duration being known covers all of these cases.
  3. Update DashTimeline so it sets the window's isDynamic flag to false if the manifest duration is known. You're currently using dynamicMediaPresentationEnded for this, but I think you should just update the line in the window.set call to use manifest.dynamic && manifest.durationMs != C.TIME_UNSET.

Note step (3) isn't quite correct, because isDynamic is currently used to indicate possible changes to both ends of the window, where-as the suggested condition only implies there wont be changes to the end of the window (the start may still change if the live window is shrinking toward the end of the content). We probably need to split isDynamic into two fields to fix this properly.

Does the above sound right to you?

@ojw28
Copy link
Contributor

ojw28 commented Oct 15, 2018

A proposed change that implements the above is here: f70cc75

Could you give that a try with the streams you have, and/or review it for sanity? For (3) I've not used manifest.durationMs (there's a TODO there to split the field), but you can edit just that line for testing purposes.

@ojw28
Copy link
Contributor

ojw28 commented Oct 15, 2018

[Related internal refs: b/69703223, b/31426754, clipped live streams]

@peddisri
Copy link
Contributor

The proposed change doesn't work. It goes into buffering at the end. The suggestions I gave earlier works. i.e

  1. Don't retry manifest refresh if dynamicMediaPresentationEnded is set to true
    https://github.com/google/ExoPlayer/blob/release-v2/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java#L737

else if (!dynamicMediaPresentationEnded
&& (expiredManifestPublishTimeUs != C.TIME_UNSET
&& newManifest.publishTimeMs * 1000 <= expiredManifestPublishTimeUs)) {

  1. Change Timeline to static if durationMs is set in manifest.
    https://github.com/google/ExoPlayer/blob/release-v2/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java#L1178
    manifest.durationMs != C.TIME_UNSET ? false : manifest.dynamic,

  2. Detect EOS

https://github.com/google/ExoPlayer/blob/release-v2/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DefaultDashChunkSource.java#L350

out.endOfStream = !manifest.dynamic || (periodIndex < manifest.getPeriodCount() - 1) ||

  •    (manifest.durationMs != C.TIME_UNSET && loadPositionUs >= (manifest.durationMs * 1000));
    
  1. Skip manifest refresh if dynamicMediaPresentationEnded is true
    https://github.com/google/ExoPlayer/blob/release-v2/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/PlayerEmsgHandler.java#L140

manifestRefreshNeeded = false;

@ojw28
Copy link
Contributor

ojw28 commented Oct 30, 2018

As per this comment and in particular the one that follows it, you need to edit the line for (3) to use manifest.durationMs to see correct behavior. Did you try that, or just patch the change in as it is?

@peddisri
Copy link
Contributor

Oh I forgot about that change. I'll do that today and retest. Sorry about that.

@peddisri
Copy link
Contributor

Yes this works.

@ojw28
Copy link
Contributor

ojw28 commented Oct 31, 2018

Thanks for checking! I'll take a look at splitting the isDynamic flag as described above, so we can land a final version of this change.

@ojw28
Copy link
Contributor

ojw28 commented Nov 13, 2018

Unfortunately, as per the interopability guidelines , service of type 4.5 " MPD and Segment-based Live Service Offering" - there is no such requirement of removing "minimumUpdatePeriod". On the contrary, it mandates it to be present.

It's interesting that section 4.4.3.1 describes minimumUpdatePeriod being removed when a live stream is terminated. I wonder if it's an omission that section 4.5 doesn't contain a similar clause. I'm still confused about how a client is supposed to know that the manifest isn't going to be changed (specifically the removal of content from the start of the window), if minimumUpdatePeriod has not been removed.

@ojw28
Copy link
Contributor

ojw28 commented Nov 13, 2018

I've filed https://github.com/Dash-Industry-Forum/DASH-IF-IOP/issues/213 to discuss whether minimumUpdatePeriod should be removed in this case.

ojw28 added a commit that referenced this issue Nov 14, 2018
The remaining work is to split Window.isDynamic so that it's
possible to represent a window that wont be appended to, but
may still be removed from.

Issue: #4780

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=221439220
@ojw28
Copy link
Contributor

ojw28 commented Nov 16, 2018

This should be fixed in dev-v2. We opted not to split isDynamic for now, but for DASH we've set it to the value that means end detection should work correctly. I would be grateful if someone could give the dev-v2 branch a try and verify the fix. Thanks!

@ojw28 ojw28 closed this as completed Nov 16, 2018
ojw28 added a commit that referenced this issue Nov 28, 2018
The remaining work is to split Window.isDynamic so that it's
possible to represent a window that wont be appended to, but
may still be removed from.

Issue: #4780

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=221439220
@google google locked and limited conversation to collaborators May 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants