GNOME Bugzilla – Bug 757655
dashdemux: cannot get segment availability start time if segment duration is not present
Last modified: 2016-02-09 20:49:21 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.
Created attachment 314948 [details] [review] 1/3 unit test to reproduce the problem
Created attachment 314949 [details] [review] 2/3 provided fix
Created attachment 314950 [details] [review] 3/3 renamed gst_mpd_client_get_next_segment_availability_end_time
Created attachment 315217 [details] [review] patch 1/3 unit test to reproduce the problem updated commit message
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)
Created attachment 315219 [details] [review] patch 3/3 renamed gst_mpd_client_get_next_segment_availability_end_time improved commit message
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?
(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.
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
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
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
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