[Bug 739002] capsfilter: Add an optional delayed caps change mode

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=739002
  GStreamer | gstreamer (core) | unspecified

Tim-Philipp Müller <t.i.m> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #289122|none                        |reviewed
             status|                            |

--- Comment #2 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2014-10-22 12:26:55 UTC ---
(From update of attachment 289122)
>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.

-- 
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.