GNOME Bugzilla – Bug 661049
matroskademux: support seek with start_type NONE
Last modified: 2011-10-28 10:41:16 UTC
Created attachment 198407 [details] [review] support seek with start_type GST_SEEK_TYPE_NONE We want to do a seek on a matroskademuxer that updates the stop position of the current segment, but does not affect the start or the current playback. We seek like this: gst_event_new_seek (1.0, GST_FORMAT_TIME, GST_SEEK_FLAG_ACCURATE, GST_SEEK_TYPE_NONE, -1, GST_SEEK_TYPE_SET, new_stop_time) The current effect when we try this is that the demuxer restarts playback from the start of the current segment, so buffers that have already been played are played again. A newsegment event for closing the currently running segment is sent out with start as the current start, and stop as the current last_stop (as we expected). But then a newsegment is sent out with start as the current start of the segment, and stop as the new end of the segment. We had expected the start of the second newsegment event to be the same as the end of the first newsegment event. The attached proposed patch handles the following when start_type is GST_SEEK_TYPE_NONE: let playback continue, and use the end of the first newsegment event as the start of the second newsegment event. When start_type is GST_SEEK_TYPE_NONE, gst_matroska_demux_move_to_entry is not called. Furthermore, gst_message_new_segment_start is not called on segment seeks. From the documentation we understood this as the correct behaviour.
While I can see this to be an intention and making sense, I am not sure if this is the semantics that goes with such a _NONE seek. In particular, gst_segment_set_seek specifically does not alter the segment.start in case of _NONE seek, and as such this patch unfortunately has to tweak/override that in (each?) demuxer. So, trying to sort this out, what documentation suggests this as correct behaviour?
The semantics are that when NONE is used in the seek for start/stop, the start/stop positions are not updated. If the start is not updated in rate > 0 (and stop not updated when rate < 0), no seek happens and playback continues where it was (possibly with a new rate or new stop/start positions). The fact that a seek should happen because the start/stop changed is reflected in the update field when calling gst_segment_set_seek()
Yes, but that does not say what to do with all the downstream sending of closing segment and stuff (assuming that "a seek should happen" means a new last_stop is at hand). For example, if the update field is false (basically meaning last_stop does not give a new position), we could forego sending any closing segments and so on, but that would not convey any new stop position to downstream. If we do still want to sent stuff there, it would have to act differently depending on update or not (and any of this would presumably apply to all demuxer stuff out there, not only matroskademux).
(In reply to comment #1) > While I can see this to be an intention and making sense, I am not sure if this > is the semantics that goes with such a _NONE seek. In particular, > gst_segment_set_seek specifically does not alter the segment.start in case of > _NONE seek, and as such this patch unfortunately has to tweak/override that in > (each?) demuxer. So, trying to sort this out, what documentation suggests this > as correct behaviour? Sorry, it was unclear what I was referring to there. I meant that in the documentation of gst_message_new_segment_start, it says "posted by elements that start playback of a segment as a result of a segment seek". I concluded it should not be posted when start_type is GST_SEEK_TYPE_NONE.
(In reply to comment #3) > Yes, but that does not say what to do with all the downstream sending of > closing segment and stuff (assuming that "a seek should happen" means a new > last_stop is at hand). > > For example, if the update field is false (basically meaning last_stop does not > give a new position), we could forego sending any closing segments and so on, > but that would not convey any new stop position to downstream. If we do still > want to sent stuff there, it would have to act differently depending on update > or not (and any of this would presumably apply to all demuxer stuff out there, > not only matroskademux). You would have to send a segment update in all cases IMO. If start (and thus also last_stop) was left unchanged because of _NONE, you would send the segment update anyway because maybe the stop or the rate changed. If the user sent a seek event that didn't update anything (we could also disallow that..) a segment update will be sent that does nothing. So I guess the seek handling would check the update flag, if updated, do regular seek logic, otherwise send segment update. I'm sure not many demuxers do this right currently..
Created attachment 198424 [details] [review] matroskademux: tune non-update seek handling So minding the update would then make it possibly something along these lines ?
To me, this seems like a good approach.
So let's go for that one then commit 4924308d02305f4ef9f43538a3db394b26765fc5 Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Thu Oct 6 13:04:54 2011 +0200 matroskademux: tune non-update seek handling cases Fixes #661049.