[Bug 638168] textoverlay: don't return wrong-state when stopping while in text chain

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=638168
  GStreamer | gst-plugins-base | unspecified

Sebastian Dröge <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #214633|none                        |needs-work
             status|                            |

--- Comment #51 from Sebastian Dröge <slomo at circular-chaos.org> 2012-05-22 10:05:35 UTC ---
Review of attachment 214633:
 --> (https://bugzilla.gnome.org/review?bug=638168&attachment=214633)

The sync mode should be separate from the queueing because both are useful
without the other too. The queue size should have a maximum to keep memory
usage lower in bad cases and it will still improve stream switching latency
then because we at least don't start at the very end of the queue.  Also might
be necessary to keep key frames in mind here, after dropping a buffer you must
not push anything before a keyframe probably.

::: plugins/elements/gstinputselector.c
@@ +381,3 @@
+{
+  GstSelectorPadCachedBuffer *cached_buffer =
+      g_new (GstSelectorPadCachedBuffer, 1);

Use g_slice_new() and g_slice_free() here... less memory fragmentation

@@ +720,3 @@
+    clock = gst_element_get_clock (GST_ELEMENT_CAST (sel));
+    if (clock)
+      clock_time = gst_clock_get_time (clock);

The old mode that doesn't use the clock but only the segment information and
buffer timestamps should be preserved. It's useful in a lot of situations.

Also did you check that buffers are not too late now (by waiting for the clock
they should all be too late, unless we're running a live pipeline).

@@ +808,3 @@
+      if (GST_BUFFER_DURATION_IS_VALID (cached_buffer->buffer))
+        running_time = GST_BUFFER_TIMESTAMP (cached_buffer->buffer) +
+            GST_BUFFER_DURATION (cached_buffer->buffer);

This is not the running time. You need to convert with
gst_segment_to_running_time() here

@@ +859,3 @@
+  /* If we have no valid timestamp we can't sync this buffer */
+  if (!GST_BUFFER_TIMESTAMP_IS_VALID (buf))
+    goto ignore;

You should only ignore the syncing here, nothing else (should still go into the
queue, pushed downstream, etc)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.