After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 780397 - dashdemux: Fix SEEK of multi-period On-Demand profile
dashdemux: Fix SEEK of multi-period On-Demand profile
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.11.91
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-22 13:27 UTC by Seungha Yang
Modified: 2017-04-09 07:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
dashdemux: Fix SEEK of multi-period On-Demand profile (1.36 KB, patch)
2017-03-22 13:28 UTC, Seungha Yang
none Details | Review
dashdemux: Fix SEEK of multi-period On-Demand profile (1.45 KB, patch)
2017-03-23 23:19 UTC, Seungha Yang
committed Details | Review

Description Seungha Yang 2017-03-22 13:27:48 UTC
For each period, media presentation time is the relative to the
period-start time. So SIDX seek position should be target seek
position minus period-start. Also, if presentationTimeOffset
is defined, the value should be compensated
Comment 1 Seungha Yang 2017-03-22 13:28:32 UTC
Created attachment 348483 [details] [review]
dashdemux: Fix SEEK of multi-period On-Demand profile
Comment 2 Sebastian Dröge (slomo) 2017-03-22 19:51:38 UTC
Do you have a example stream for this?
Comment 3 Sebastian Dröge (slomo) 2017-03-23 10:54:00 UTC
Review of attachment 348483 [details] [review]:

::: ext/dash/gstdashdemux.c
@@ +1345,3 @@
+        gst_mpd_parser_get_period_start_time (dashdemux->client);
+    if (ts > period_start)
+      ts -= period_start;

What if ts <= period_start? Especially the == case seems useful to handle here. The < case is probably an error and we should just error out?

Would it make sense to first add the presentation offset, and then do the subtraction?
Comment 4 Seungha Yang 2017-03-23 23:19:23 UTC
Created attachment 348626 [details] [review]
dashdemux: Fix SEEK of multi-period On-Demand profile
Comment 5 Seungha Yang 2017-03-23 23:36:51 UTC
(In reply to Sebastian Dröge (slomo) from comment #3)
> Review of attachment 348483 [details] [review] [review]:

Sorry, I couldn't share it since it's private stream.
But I can show how the MPD looks like (it's on-demand profile)

<Period duration="PT8S" id="0">
... 
<SegmentTemplate duration="90000" media="audio$Number%08d$.m4a" presentationTimeOffset="1231" startNumber="0" timescale="1000">
</SegmentTemplate>
..
</Period>

<Period duration="PT10S" id="1">
<SegmentTemplate duration="90000" media="audio$Number%08d$.m4a" presentationTimeOffset="22599" startNumber="1" timescale="1000">
</SegmentTemplate>
...
</Period>
....

> ::: ext/dash/gstdashdemux.c
> @@ +1345,3 @@
> +        gst_mpd_parser_get_period_start_time (dashdemux->client);
> +    if (ts > period_start)
> +      ts -= period_start;
> 
> What if ts <= period_start? Especially the == case seems useful to handle
> here. The < case is probably an error and we should just error out?

Obviously, equal condition need to be handled. I fixed it on attachment 348626 [details] [review].
The "<" case seems to be an error case (it's outside of period!!), which should not be happen but seems to be probable case for example Bug 776559 case.
I'm not sure whether return ERROR is correct or not, because whenever stream_seek() has returned FLOW_OK until now. 

> Would it make sense to first add the presentation offset, and then do the
> subtraction?

In my opinion, add presentation offset first or later seems same if the target seek position was inside of the period :)
Comment 6 Sebastian Dröge (slomo) 2017-04-09 07:54:43 UTC
Attachment 348626 [details] pushed as 41996ad - dashdemux: Fix SEEK of multi-period On-Demand profile