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 757655 - dashdemux: cannot get segment availability start time if segment duration is not present
dashdemux: cannot get segment availability start time if segment duration is ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.7.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-06 00:03 UTC by Florin Apostol
Modified: 2016-02-09 20:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
1/3 unit test to reproduce the problem (2.09 KB, patch)
2015-11-06 00:07 UTC, Florin Apostol
none Details | Review
2/3 provided fix (2.50 KB, patch)
2015-11-06 00:07 UTC, Florin Apostol
reviewed Details | Review
3/3 renamed gst_mpd_client_get_next_segment_availability_end_time (6.29 KB, patch)
2015-11-06 00:09 UTC, Florin Apostol
none Details | Review
patch 1/3 unit test to reproduce the problem (2.10 KB, patch)
2015-11-10 22:03 UTC, Florin Apostol
none Details | Review
patch 2/3 provided fix (2.77 KB, patch)
2015-11-10 22:05 UTC, Florin Apostol
none Details | Review
patch 3/3 renamed gst_mpd_client_get_next_segment_availability_end_time (6.34 KB, patch)
2015-11-10 22:06 UTC, Florin Apostol
none Details | Review
patch 1/3 unit test to reproduce the problem (4.33 KB, patch)
2015-11-19 15:40 UTC, Florin Apostol
committed Details | Review
patch 2/3 provided fix (3.21 KB, patch)
2015-11-19 15:43 UTC, Florin Apostol
committed Details | Review
patch 3/3 renamed gst_mpd_client_get_next_segment_availability_end_time (6.34 KB, patch)
2015-11-19 15:45 UTC, Florin Apostol
committed Details | Review

Description Florin Apostol 2015-11-06 00:03:47 UTC
The function to get the segment availability (gst_mpd_client_get_next_segment_availability_end_time) relies on segment duration to compute the segment end time. But duration is not present if the segment list is defined using segment timeline. In this case, the function is unable to return the segment availability time.
Comment 1 Florin Apostol 2015-11-06 00:07:23 UTC
Created attachment 314948 [details] [review]
1/3 unit test to reproduce the problem
Comment 2 Florin Apostol 2015-11-06 00:07:49 UTC
Created attachment 314949 [details] [review]
2/3 provided fix
Comment 3 Florin Apostol 2015-11-06 00:09:25 UTC
Created attachment 314950 [details] [review]
3/3 renamed gst_mpd_client_get_next_segment_availability_end_time
Comment 4 Florin Apostol 2015-11-10 22:03:31 UTC
Created attachment 315217 [details] [review]
patch 1/3 unit test to reproduce the problem

updated commit message
Comment 5 Florin Apostol 2015-11-10 22:05:42 UTC
Created attachment 315218 [details] [review]
patch 2/3 provided fix

corrected the fix for situations where there is no segment list (e.g. when segment templates are used)
Comment 6 Florin Apostol 2015-11-10 22:06:45 UTC
Created attachment 315219 [details] [review]
patch 3/3 renamed gst_mpd_client_get_next_segment_availability_end_time

improved commit message
Comment 7 Thiago Sousa Santos 2015-11-18 18:08:30 UTC
Review of attachment 314949 [details] [review]:

::: ext/dash/gstmpdparser.c
@@ +5679,3 @@
   seg_idx = stream->segment_index;
+
+  segment = g_ptr_array_index (stream->segments, seg_idx);

I guess this would fail if segments==NULL in case a template is used. Right?
Comment 8 Thiago Sousa Santos 2015-11-18 18:08:33 UTC
Review of attachment 314949 [details] [review]:

::: ext/dash/gstmpdparser.c
@@ +5679,3 @@
   seg_idx = stream->segment_index;
+
+  segment = g_ptr_array_index (stream->segments, seg_idx);

I guess this would fail if segments==NULL in case a template is used. Right?
Comment 9 Florin Apostol 2015-11-19 11:09:34 UTC
(In reply to Thiago Sousa Santos from comment #8)
> Review of attachment 314949 [details] [review] [review]:
> 
> ::: ext/dash/gstmpdparser.c
> @@ +5679,3 @@
>    seg_idx = stream->segment_index;
> +
> +  segment = g_ptr_array_index (stream->segments, seg_idx);
> 
> I guess this would fail if segments==NULL in case a template is used. Right?

yes, but that patch is obsolete. The new patch does not have this problem. Please review the new patch.
Comment 10 Florin Apostol 2015-11-19 15:40:50 UTC
Created attachment 315894 [details] [review]
patch 1/3 unit test to reproduce the problem

previous proposed patch had a problem, so improved the unit tests to detect it
Comment 11 Florin Apostol 2015-11-19 15:43:45 UTC
Created attachment 315896 [details] [review]
patch 2/3 provided fix

gst_mpdparser_get_segment_end_time returned an incorrect end time in case segment repeat is not 0, so implemented a different solution that does not rely on gst_mpdparser_get_segment_end_time
Comment 12 Florin Apostol 2015-11-19 15:45:22 UTC
Created attachment 315897 [details] [review]
patch 3/3 renamed gst_mpd_client_get_next_segment_availability_end_time

no changes here, but resubmited to avoid git confusion due to changes in previous patches from the set
Comment 13 Thiago Sousa Santos 2016-02-09 20:48:12 UTC
commit 9ccd541980420f9fa8c22e8dd05b1916fecfd68e
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Tue Nov 10 22:01:38 2015 +0000

    mpdparser: renamed gst_mpd_client_get_next_segment_availability_end_time to gst_mpd_client_get_next_segment_availability_start_time
    
    The function actually returns the segment availability start time (as defined by the standard).
    That is at the end of the segment, but it is called availability start time.
    
    Availability end time is something else (the time when the segment is no longer
    available on the server). The function name was misleading.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757655

commit b96ea72b4f49a33fecf93b7681a509317c3f1140
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Tue Nov 10 22:00:58 2015 +0000

    mpdparser: corrected getting segment availability
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757655

commit 25c89b12d6decc9599d1a90b4a9a59685b41b7cd
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Thu Nov 19 15:30:34 2015 +0000

    mpdparser: tests: added unit test for getting segment availability when segment timeline is used
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757655