GNOME Bugzilla – Bug 750027
concat: Reset internal start offset to 0 after flushing seek
Last modified: 2015-07-16 08:51:31 UTC
Created attachment 304149 [details] [review] Patch for resetting the start offset after flushing seek This fixes seek problems that occur after concat switched to a next stream. In such cases, concat will produce a segment with a nonzero base value. This way, segments from individual streams are concatenated seamlessly. However, after a flushing seek, the element base-time is reset. This means that current_start_offset has to be reset to 0, since it was calculated with the old base-time in mind. This patch depends on https://bugzilla.gnome.org/show_bug.cgi?id=745366 .
Created attachment 304152 [details] [review] Patch for resetting the start offset after flushing seek , v2 Revised version which fixes indentation issues
Review of attachment 304152 [details] [review]: ::: plugins/elements/gstconcat.c @@ +523,3 @@ + "resetting start offset to 0 after flushing seek"); + self->current_start_offset = 0; + self->reset_ofs_after_flushseek = FALSE; I think resetting the base time makes sense *always* (not only after seeks) when we forward flush events. The sink will have its segment reset, so base starts from 0 again. @@ +593,3 @@ + GST_DEBUG_OBJECT (self, + "flushing seek detected; will reset start offset after flush"); + self->reset_ofs_after_flushseek = TRUE; To reliably match a flush event to the seek event you'll have to compare the sequence numbers
In an earlier version of the patch I did reset the segment base with any flush event, not just seek-related ones. However, the docs only say that the base time is reset after a flushing seek. They do not state if it is reset after *every* kind of flush.
Every FLUSH_STOP that has reset_time==TRUE, it's independent of any seek events.
Ping? :)
Basically in all the forward==TRUE cases on FLUSH_STOP (with reset_time==TRUE inside the event) it should reset the time. And just not do anything specific about seek events or something
Created attachment 305051 [details] [review] Updated patch to reset if FLUSH_STOP reset_time is TRUE Updated the patch. It is no longer flush-seek specific, and only needs the reset_time value from FLUSH_STOP.
Review of attachment 305051 [details] [review]: ::: plugins/elements/gstconcat.c @@ +525,3 @@ + GST_DEBUG_OBJECT (self, + "resetting start offset to 0 after flushing with reset_time = TRUE"); + self->current_start_offset = 0; I think this should only be done if the FLUSH_STOP event came from the active pad, not from some random other pad. For other pads we don't care really We should also do the same in the src_event function
Created attachment 305052 [details] [review] Updated patch to only look at FLUSH_STOP from active pad Second update. Only look at FLUSH_STOP if it comes from the active pad (forward==TRUE).
Hrm, forgot to ammend your commit before pushing. Please don't put the "Signed-off-by" noise into commit messages, and instead put a link to the bug report there :) git-bz does that automatically for you. commit d8db88d078dc2688858ec6393379ade6b5990771 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jun 11 11:05:53 2015 +0200 concat: Also reset the current start offset when receiving a FLUSH_STOP on the srcpad commit c324e31c26e7eb088ac79001e4eb5ed76fe8f094 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jun 11 11:05:38 2015 +0200 concat: Add some newlines to event handling code to make the code look a bit less dense commit 25c66e60045e44d50a591f5d62706f6f243bb9ca Author: Carlos Rafael Giani <dv@pseudoterminal.org> Date: Thu Jun 11 10:53:30 2015 +0200 concat: Reset segment base offset after FLUSH_STOP with reset_time = TRUE If the reset_time value of a FLUSH_STOP event is set to TRUE, the pipeline will have the base_time of its elements reset. This means that the concat element's current_start_offset has to be reset to 0, since it was calculated with the old base-time in mind. Only FLUSH_STOP events coming from the active pad are looked at. Signed-off-by: Carlos Rafael Giani <dv@pseudoterminal.org>
A similar problem seems to exist with the latest versions of GStreamer git master. The problem was fixed for a short period of time, but now seeks are inaccurate again when using a concat element in a pipeline. Any seek I perform one a pipeline with a concat element seeks the pipeline to a position before the time I request. Playing or Paused.
(In reply to David Griffin from comment #11) > A similar problem seems to exist with the latest versions of GStreamer git > master. The problem was fixed for a short period of time, but now seeks are > inaccurate again when using a concat element in a pipeline. Any seek I > perform one a pipeline with a concat element seeks the pipeline to a > position before the time I request. Playing or Paused. Can you add some details about your pipeline? It would make replicating the issue easier.