[Bug 729760] appsrc: Changing caps and pushing buffers is not serialized

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #19 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-08-01 15:54:40 UTC ---
(In reply to comment #18)
> (In reply to comment #17)
> > Review of attachment 282124 [details] [details]:
> > 
> > ::: gst-libs/gst/app/gstappsrc.c
> > @@ +537,3 @@
> > +  while ((caps_or_buffer = g_queue_pop_head (priv->queue))) {
> > +    if (caps_or_buffer) {
> > +      gst_mini_object_unref (caps_or_buffer);
> > 
> > Can it ever be NULL?
> 
> the original implementation allows set NULL as new caps ,so the queued caps can
>  also be NULL . that's also the reason I'm trying to use any caps instead of
> NULL caps:)

Makes sense, keep it at NULL though. You can only set NULL caps on a pad, not
ANY caps.

> > 
> > @@ +1190,3 @@
> >    GST_DEBUG_OBJECT (appsrc, "setting caps to %" GST_PTR_FORMAT, caps);
> > +
> > +  caps_or_buffer = g_queue_peek_tail (priv->queue);
> > 
> > Why this complicated code? Just push the caps on the queue and handle them
> > above :)
> 
> caller may set new caps multi times but didn't push any buffers between them,
> my patch tried to skip these unnecessary re-negotitate . if every new caps
> (even without buffer of this caps) event should be pushed downstream , I should
> correct my patch.

Good point. I think you would want to just put them always into the queue, but
check if they are equal to the current caps right before setting them. That
will keep the code a bit simpler while having the same effect.

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