GNOME Bugzilla – Bug 739002
capsfilter: Add an optional delayed caps change mode
Last modified: 2014-11-03 07:27:11 UTC
In this mode we accept previously set filter caps until upstream renegotiates to something that is compatible to the current filter caps. This allows dynamic caps changes in the pipeline even if there is a queue between any conversion element and the capsfilter. With this we would get not-negotiated errors if timing is bad.
Created attachment 289122 [details] [review] capsfilter: Add an optional delayed caps change mode
Comment on attachment 289122 [details] [review] capsfilter: Add an optional delayed caps change mode >With this we would get not-negotiated >errors if timing is bad. "Without this" you mean? >+#define GST_TYPE_CAPS_FILTER_PULL_MODE (gst_caps_filter_pull_mode_get_type()) >+static GType >+gst_caps_filter_pull_mode_get_type (void) The naming of the function isn't right, it should match the GType name (copy'n'past I presume). >+ g_object_class_install_property (gobject_class, PROP_CAPS_CHANGE_MODE, >+ g_param_spec_enum ("caps-change-mode", _("Caps Change Mode"), >+ _("Filter caps change behaviour"), GST_TYPE_CAPS_FILTER_PULL_MODE, >+ DEFAULT_CAPS_CHANGE_MODE, >+ G_PARAM_READWRITE | GST_PARAM_MUTABLE_PLAYING | >+ G_PARAM_STATIC_STRINGS)); >+ > gstelement_class = GST_ELEMENT_CLASS (klass); > gst_element_class_set_static_metadata (gstelement_class, > "CapsFilter", >@@ -136,6 +165,7 @@ gst_capsfilter_init (GstCapsFilter * filter) > gst_base_transform_set_gap_aware (trans, TRUE); > gst_base_transform_set_prefer_passthrough (trans, FALSE); > filter->filter_caps = gst_caps_new_any (); >+ filter->caps_change_mode = DEFAULT_CAPS_CHANGE_MODE; > } > > static void >@@ -160,6 +190,11 @@ gst_capsfilter_set_property (GObject * object, guint prop_id, > GST_OBJECT_LOCK (capsfilter); > old_caps = capsfilter->filter_caps; > capsfilter->filter_caps = new_caps; >+ if (old_caps >+ && capsfilter->caps_change_mode == >+ GST_CAPS_FILTER_CAPS_CHANGE_MODE_DELAYED) >+ capsfilter->previous_caps = >+ g_list_prepend (capsfilter->previous_caps, gst_caps_ref (old_caps)); > GST_OBJECT_UNLOCK (capsfilter); Should setting the mode to IMMEDIATE clear the previous_caps list? Both makes sense, just wondering. >@@ -211,11 +252,13 @@ gst_capsfilter_transform_caps (GstBaseTransform * base, > { > GstCapsFilter *capsfilter = GST_CAPSFILTER (base); > GstCaps *ret, *filter_caps, *tmp; >+ gboolean retried = FALSE; > > GST_OBJECT_LOCK (capsfilter); > filter_caps = gst_caps_ref (capsfilter->filter_caps); > GST_OBJECT_UNLOCK (capsfilter); I think you should read the caps_change_mode into a local variable here as well, within the object lock.
Review of attachment 289122 [details] [review]: Only this to add to tim's review, seems fine by me. ::: tests/check/elements/capsfilter.c @@ +357,3 @@ + GST_STATE_CHANGE_SUCCESS); + + /* push the stream start */ Can't you just use gst_check_setup_events here ?
Created attachment 289124 [details] [review] capsfilter: Add an optional delayed caps change mode In this mode we accept previously set filter caps until upstream renegotiates to something that is compatible to the current filter caps. This allows dynamic caps changes in the pipeline even if there is a queue between any conversion element and the capsfilter. Without this we would get not-negotiated errors if timing is bad.
*** Bug 668001 has been marked as a duplicate of this bug. ***
commit 9606e0489585b34d666b76c536b208608c1fa37e Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Oct 22 14:07:09 2014 +0200 capsfilter: Add an optional delayed caps change mode In this mode we accept previously set filter caps until upstream renegotiates to something that is compatible to the current filter caps. This allows dynamic caps changes in the pipeline even if there is a queue between any conversion element and the capsfilter. Without this we would get not-negotiated errors if timing is bad. https://bugzilla.gnome.org/show_bug.cgi?id=739002