GNOME Bugzilla – Bug 700491
dashdemux: Handle cases where minimumUpdatePeriod sets the period length
Last modified: 2018-05-06 12:57:55 UTC
Created attachment 244472 [details] [review] Initial patch On a live ts stream generator I ran into this case; unfortunately it is not publicly available for reproduction. Per A.3.2 of ISO/IEC 23009, concerning Representations using SegmentTemplates: "if the MPD@mediaPresentationDuration attribute is not present, then PeriodEndTime is defined as FetchTime + MPD@minimumUpdatePeriod." However, the code fails out in this case and refuses to even start playback, since the PeriodEnd value in gst_mpd_client_setup_representation() sets PeriodEnd to GST_CLOCK_TIME_NONE in such a case because no Duration is provided. I have attached a patch to fix this issue. It also provides a comment for future debugging of similar cases.
You patch seems to assume that PeriodStart == FetchTime.. is that really always true ?
There is a MPD file that can in theory reproduce this in attachment #244365 [details]
You make a good point about PeriodStart. I'll create an updated patch. As for your MPD, where did it come from? It does not include a full URL, so if I run with, say "gst-launch-1.0 playbin uri=file:///home/art/Downloads/infinite-loop.mpd" dashdemux attempts to find fragments in file:///home/art/Downloads/big_buck_bunny_480p_h264.mov/150000/fragment-0.ts -- which is not where they are.
Created attachment 244537 [details] [review] A revised patch. This handles the case where PeriodStart is nonzero; the case I ran into did have a zero PeriodStart.
Is it really from PeriodStart? I understand it as if you have minimumUpdatePeriod=30 and segment duration=10, then every 30 secs this should be called again so it should yield PeriodEnd=30, but the next time it shoudl be 60 even though the MPD file hasn't changed... ? My MPD comes from an application.. just put any 10s .ts files there..
You make a good point, it should be the fetch time. I'm working on figuring out the clean way to get the fetch time over to that point in the code; it's in the element, but the MPD parser code never deals directly with the element. I will say that I'm able to see my fix somewhat working (don't have 10s .ts files handy, so it's jumpy due to the short length of my clips) with that MPD.
I'm also not sure what the relation between PeriodStart and FetchTime is exactly. Are they on the same clock? Are they related to availabilityStartTime ? My understanding is that one could have a never-updated MPD file with one single Period, a non-0 minimumUpdatePeriod and a <SegmentTemplate ..\>. And at every re-load it should yield a new set of segment files to load? But how does it know what their indexes are supposed to be ? I don't know.
I don't think minimumUpdatePeriod has any real relationship with the duration of the Period. According to ISO 23009-1_2012, Section 5.4: "If the attribute MPD@miniumUpdatePeriod is present updates to the MPD are expected and restricted in a sense that at the location where the MPD is available at a certain time, the MPD is also valid for the duration of the value of the MPD@minimumUpdatePeriod attribute." Basically, all this says is that the current MPD shall remain valid for entire duration encompassed by minimumUpdatePeriod (from the time that the MPD was retrieved). In the case of our "live" dynamic MPDs, this means that the client can continue to download segments according to the current template until the client reads a new MPD and sees a change. Since the Period has no duration (which would be the case in a "live" service), the client just continues to download segments. The MPD@minimumUpdatePeriod could be 10 minutes. If there is no real change to the structure of the MPD at that time, the MPD@availabilityStartTime and the new SegmentTemplate@startNumber should align very closely with what the client is already doing and the client just keeps on going.
(In reply to comment #8) > Since the Period has no duration (which would be the case in a "live" service), > the client just continues to download segments. The MPD@minimumUpdatePeriod > could be 10 minutes. If there is no real change to the structure of the MPD at > that time, the MPD@availabilityStartTime and the new > SegmentTemplate@startNumber should align very closely with what the client is > already doing and the client just keeps on going. I want to clarify this a little bit. I don't mean to say that availabilityStartTime is always a clock reference for the segments in this MPD. It simply indicates when the segments in the MPD shall be available for download by the client. However, section 5.4 does indicate: "NOTE 3: The second condition above ensures that sufficient media is contained in each Representation to present the Media Presentation up to Texp(i) for a client that begins playing each Segment at the earliest possible time (its availability start time)." The text in Section 5.4 of ISO-23001-9_2012 clarifies the updating of MPDs in a way that should allow clients to determine that an MPD update is "consistent" with the previous MPD and that the client can continue to download segments of increasing numbers, queuing them behind the last segment downloaded from the previous MPD rules, and presenting them.
I realized that availabilityStartTime just assumes that both the client and the server has synchronized clocks (NTP style). And then it should be relatively easy to do.
Created attachment 245579 [details] [review] 3rd revision of patch This patch handles passing in the fetch time per what I highlighted from the spec and putting in the duration from the minimumUpdatePeriod in a manner that will ensure they are available for all needs. Also, Olivier, it works much better with your test case. Let us know what you see.
Even with your patch, it still loops on this: 0:00:02.847825528 21956 0x7fffd80b70f0 LOG dashdemux gstdashdemux.c:1394:gst_dash_demux_download_loop:<dashdemux0> Starting download loop 0:00:02.847831269 21956 0x7fffd80b70f0 DEBUG dashdemux gstdashdemux.c:1409:gst_dash_demux_download_loop:<dashdemux0> Next update: 79:12:26.151649481 now: 79:11:48.679422794 0:00:02.847842010 21956 0x7fffd80b70f0 DEBUG dashdemux gstdashdemux.c:1534:gst_dash_demux_download_loop:<dashdemux0> download loop 0 0:00:02.847848827 21956 0x7fffd80b70f0 INFO dashdemux gstdashdemux.c:1867:gst_dash_demux_get_next_fragment:<dashdemux0> This Period doesn't contain more fragments for stream 0 0:00:02.847855857 21956 0x7fffd80b70f0 INFO dashdemux gstdashdemux.c:1572:gst_dash_demux_download_loop:<dashdemux0> Internal buffering : 0 s I'm not sure how the current code is supposed to be stopped in my use-case, there is one period that is not time bound..
My loop is bug #701404
Took me a bit to realize what you are seeing. I am seeing that playback works, and I thought you were not. The download_loop does spin; however that appears to be because of the use of a GstTask for the download_loop, see bug #701404, comment 5. My patch is intended to address a different aspect, the use of implicit period lengths in a dynamic clip. Also with this patch, your infinite-loop.mpd auto-restarts rather than sitting forever.
I've created bug 703136 to track the problem with the download_loop. Do we think that the current patch properly addresses the original issue? If so, can we get this issue closed?
Review of attachment 245579 [details] [review]: Having a simple MPD to reproduce this issue would make it easier to see what you are getting. No media is required as you claim the failure is at the MPD parsing stage. Are you using the start time or duration somehow? Or is this only to make the parsing finish correctly? ::: ext/dash/gstmpdparser.c @@ +3036,3 @@ + if (client->mpd_node->minimumUpdatePeriod != -1) { + duration = client->mpd_node->minimumUpdatePeriod; + } This still doesn't seem like a good idea to me. The spec only says this is true for "Template based generation of segment list".
Review of attachment 245579 [details] [review]: I agree with Thiago, that this might not be quite correct. But I think its close. The spec says that this is true only for templates because templates are the only "open-ended" segment specification. With segment list and timeline, the parser can easily calculate the duration of the period by summing the duration of the segments (since every segment is described explicitly). While with segment templates, each segment has a duration, but the total number of segments may be unknown. We need a new patch that ensures that Period duration is properly calculated in all 3 cases (this current patch only addresses one of them). ::: ext/dash/gstmpdparser.c @@ +3035,3 @@ + /* might be a live file, ignore unspecified duration if no updates */ + if (client->mpd_node->minimumUpdatePeriod != -1) { + duration = client->mpd_node->minimumUpdatePeriod; This calculation of duration is incorrect because it doesn't factor in the proper units. minimumUpdatePeriod is in msecs, while duration is a GstClockTime in nsecs.
Created attachment 248490 [details] [review] My updated fix for this issue. This patch would supersede the previous patch as it encapsulates those fixes (with some modifications and additional fixes).
Thiago, what do you think about this patch?
Sorry for forgetting about this issue, can you remind me of the issue this is causing? What is the use case here? Is that that without knowing the period-duration dashdemux is trying to fetch fragments that were still not created? Is it still happening with git master or one of the 1.5.* releases? "My patch is intended to address a different aspect, the use of implicit period lengths in a dynamic clip. Also with this patch, your infinite-loop.mpd auto-restarts rather than sitting forever."
Closing this bug report as no further information has been provided. Please feel free to reopen this bug report if you can provide the information that was asked for in a previous comment. Thanks!