GNOME Bugzilla – Bug 752374
dashdemux: gst_dash_demux_get_live_seek_range should not return negative values
Last modified: 2016-02-10 22:27:04 UTC
If availabilityStartTime is larger than current time (stream not yet available), the gst_dash_demux_get_live_seek_range function will set negative values in start and stop parameters and return TRUE. It is not very sure if the caller will validate the fact that start and stop are positive numbers. I believe the function should return FALSE if availabilityStartTime is bigger than current time and start and stop would be negative.
I'm not sure about that. I think gst_dash_demux_get_live_seek_range() function should only return FALSE if it fails to calculate the range. The fact that the range is empty is not an error in the gst_dash_demux_get_live_seek_range() function, it is simply that there is no available fragments.
the range will contain negative values. For example start=-10 stop=-5. Is this a valid range? I think not. And I doubt that callers of this function will check that start and stop are >0. And if later start and stop are used in unsigned comparisons, we will have a big problem. We could set start=0 and stop=0 if you want to return true. But I don't think it is normal to return true and set start=-10 stop=-5
Anything to be done here still?
The main question is: is it allowed to have negative values in start and stop parameters for a seek range? For example, is start=-10 stop=-5 a valid range for seek? Negative values means the position will be before the stream is available. Usually seek range is between 0 and now - availabilityStartTime. Both of them are positive.
No, negative start/stop values are not supported for seek events.
The function returns an incorrect start value if timeShiftBufferDepth is not defined (-1). The function returns as start the stop + 1ms, which is obviously wrong. In this case, according to the standard the buffer should be infinite so start should be 0. I'll correct the function to return only positive values for start and stop.
Created attachment 315989 [details] [review] proposed patch
Merged, did a little amend to consider the lack of availabilityStartTime a runtime error not an assertion as it is read from the input. commit 88e21e6089bade0e2ddc5557c4ee41dce8b3ce24 Author: Florin Apostol <florin.apostol@oregan.net> Date: Fri Nov 20 19:38:03 2015 +0000 dashdemux: gst_dash_demux_get_live_seek_range returns positive values https://bugzilla.gnome.org/show_bug.cgi?id=752374