From 26a1ac0ce700e7b62bbb12673afedee9c4f6b42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 8 Dec 2011 17:02:28 +0100 Subject: [PATCH 1/4] basetransform: Refactor gst_base_transform_buffer_alloc() code Don't check if upstream provided caps are compatible with upstream and don't try to fixate these caps. They must be fixated in any case. --- libs/gst/base/gstbasetransform.c | 198 ++++++++++++++++--------------- 1 file changed, 102 insertions(+), 96 deletions(-) diff --git a/libs/gst/base/gstbasetransform.c b/libs/gst/base/gstbasetransform.c index 25abdd860e..409b999101 100644 --- a/libs/gst/base/gstbasetransform.c +++ b/libs/gst/base/gstbasetransform.c @@ -1828,7 +1828,7 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, GstBaseTransformPrivate *priv; GstFlowReturn res; gboolean alloced = FALSE; - gboolean proxy, suggest, same_caps; + gboolean proxy, suggest, new_caps; GstCaps *sink_suggest = NULL; guint size_suggest; @@ -1850,32 +1850,33 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, /* we remember our previous alloc request to quickly see if we can proxy or * not. We skip this check if we have a pending suggestion. */ GST_OBJECT_LOCK (pad); - same_caps = !priv->suggest_pending && caps && - gst_caps_is_equal (priv->sink_alloc, caps); + suggest = priv->suggest_pending; GST_OBJECT_UNLOCK (pad); - if (same_caps) { - /* we have seen this before, see below if we need to proxy */ - GST_DEBUG_OBJECT (trans, "have old caps %p, size %u", caps, size); + if (!suggest) { + /* we have no suggestion, see below if we need to proxy */ gst_caps_replace (&sink_suggest, caps); size_suggest = size; suggest = FALSE; - } else { - GST_DEBUG_OBJECT (trans, "new format %p %" GST_PTR_FORMAT, caps, caps); + new_caps = sink_suggest + && !gst_caps_is_equal (sink_suggest, priv->sink_alloc); + if (new_caps) + GST_DEBUG_OBJECT (trans, "new format %p %" GST_PTR_FORMAT, caps, caps); + else + GST_DEBUG_OBJECT (trans, "have old caps %p, size %u", caps, size); + } else { /* if we have a suggestion, pretend we got these as input */ GST_OBJECT_LOCK (pad); - if ((priv->sink_suggest && !gst_caps_is_equal (caps, priv->sink_suggest))) { + if (priv->sink_suggest && !gst_caps_is_equal (caps, priv->sink_suggest)) { sink_suggest = gst_caps_ref (priv->sink_suggest); size_suggest = priv->size_suggest; GST_DEBUG_OBJECT (trans, "have suggestion %p %" GST_PTR_FORMAT " size %u", sink_suggest, sink_suggest, priv->size_suggest); - /* suggest is TRUE when we have a custom suggestion pending that we need - * to unref later. */ - suggest = TRUE; } else { - GST_DEBUG_OBJECT (trans, "using caps %p %" GST_PTR_FORMAT " size %u", - caps, caps, size); + GST_DEBUG_OBJECT (trans, + "have suggestion equal to upstream caps %p %" GST_PTR_FORMAT, caps, + caps); gst_caps_replace (&sink_suggest, caps); size_suggest = size; suggest = FALSE; @@ -1884,8 +1885,7 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, GST_OBJECT_UNLOCK (pad); /* check if we actually handle this format on the sinkpad */ - if (sink_suggest) { - const GstCaps *templ; + if (suggest) { GstCaps *peercaps; /* Always intersect with the peer caps to get correct @@ -1944,90 +1944,95 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, GST_DEBUG_OBJECT (trans, "Caps fixed to: %" GST_PTR_FORMAT, sink_suggest); } - - /* Check if the suggested caps are compatible with our - * sinkpad template caps and if they're not (or were - * empty after intersecting with the peer caps above) - * we try to come up with any supported caps - */ - if (sink_suggest) { - templ = gst_pad_get_pad_template_caps (pad); - - if (!gst_caps_can_intersect (sink_suggest, templ)) { - GstCaps *allowed; - GstCaps *peercaps; - - GST_DEBUG_OBJECT (trans, - "Requested pad alloc caps are not supported: %" GST_PTR_FORMAT, - sink_suggest); - /* the requested pad alloc caps are not supported, so let's try - * picking something allowed between the pads (they are linked, - * there must be something) */ - allowed = gst_pad_get_allowed_caps (pad); - if (allowed && !gst_caps_is_empty (allowed)) { - GST_DEBUG_OBJECT (trans, - "pads could agree on one of the following caps: " "%" - GST_PTR_FORMAT, allowed); - allowed = gst_caps_make_writable (allowed); - - if (klass->fixate_caps) { - peercaps = - gst_pad_get_allowed_caps (GST_BASE_TRANSFORM_SRC_PAD (trans)); - klass->fixate_caps (trans, GST_PAD_SRC, peercaps, allowed); - gst_caps_unref (peercaps); - } - - /* Fixate them to be safe if the subclass didn't do it */ - gst_caps_truncate (allowed); - gst_pad_fixate_caps (pad, allowed); - gst_caps_replace (&sink_suggest, allowed); - gst_caps_unref (allowed); - - suggest = TRUE; - - GST_DEBUG_OBJECT (trans, "Fixated suggestion caps to %" - GST_PTR_FORMAT, sink_suggest); - } else { - if (allowed) - gst_caps_unref (allowed); - goto not_supported; - } - } - } } - /* find the best format for the other side here we decide if we will proxy - * the caps or not. */ - if (sink_suggest == NULL) { - /* always proxy when the caps are NULL. When this is a new format, see if - * we can proxy it downstream */ - GST_DEBUG_OBJECT (trans, "null caps, marking for proxy"); - priv->proxy_alloc = TRUE; - } else { - GstCaps *othercaps; + new_caps = sink_suggest + && !gst_caps_is_equal (sink_suggest, priv->sink_alloc); + } - /* we have a new format, see what we need to proxy to */ - othercaps = gst_base_transform_find_transform (trans, pad, sink_suggest); - if (!othercaps || gst_caps_is_empty (othercaps)) { - /* no transform possible, we certainly can't proxy */ - GST_DEBUG_OBJECT (trans, "can't find transform, disable proxy"); - priv->proxy_alloc = FALSE; - } else { - /* we transformed into something */ - if (gst_caps_is_equal (sink_suggest, othercaps)) { - GST_DEBUG_OBJECT (trans, - "best caps same as input, marking for proxy"); - priv->proxy_alloc = TRUE; - } else { - GST_DEBUG_OBJECT (trans, - "best caps different from input, disable proxy"); - priv->proxy_alloc = FALSE; + /* Check if the new caps are compatible with our + * sinkpad template caps and if they're not + * we try to come up with any supported caps + */ + if (new_caps) { + const GstCaps *templ; + + templ = gst_pad_get_pad_template_caps (pad); + + if (!gst_caps_can_intersect (sink_suggest, templ)) { + GstCaps *allowed; + GstCaps *peercaps; + + GST_DEBUG_OBJECT (trans, + "Requested pad alloc caps are not supported: %" GST_PTR_FORMAT, + sink_suggest); + /* the requested pad alloc caps are not supported, so let's try + * picking something allowed between the pads (they are linked, + * there must be something) */ + allowed = gst_pad_get_allowed_caps (pad); + if (allowed && !gst_caps_is_empty (allowed)) { + GST_DEBUG_OBJECT (trans, + "pads could agree on one of the following caps: " "%" + GST_PTR_FORMAT, allowed); + allowed = gst_caps_make_writable (allowed); + + if (klass->fixate_caps) { + peercaps = + gst_pad_get_allowed_caps (GST_BASE_TRANSFORM_SRC_PAD (trans)); + klass->fixate_caps (trans, GST_PAD_SRC, peercaps, allowed); + gst_caps_unref (peercaps); } + + /* Fixate them to be safe if the subclass didn't do it */ + gst_caps_truncate (allowed); + gst_pad_fixate_caps (pad, allowed); + gst_caps_replace (&sink_suggest, allowed); + gst_caps_unref (allowed); + + suggest = TRUE; + new_caps = !gst_caps_is_equal (sink_suggest, priv->sink_alloc); + + GST_DEBUG_OBJECT (trans, "Fixated suggestion caps to %" + GST_PTR_FORMAT, sink_suggest); + } else { + if (allowed) + gst_caps_unref (allowed); + goto not_supported; } - if (othercaps) - gst_caps_unref (othercaps); } } + + /* find the best format for the other side here we decide if we will proxy + * the caps or not. */ + if (sink_suggest == NULL) { + /* always proxy when the caps are NULL. When this is a new format, see if + * we can proxy it downstream */ + GST_DEBUG_OBJECT (trans, "null caps, marking for proxy"); + priv->proxy_alloc = TRUE; + } else if (new_caps) { + GstCaps *othercaps; + + /* we have a new format, see what we need to proxy to */ + othercaps = gst_base_transform_find_transform (trans, pad, sink_suggest); + if (!othercaps || gst_caps_is_empty (othercaps)) { + /* no transform possible, we certainly can't proxy */ + GST_DEBUG_OBJECT (trans, "can't find transform, disable proxy"); + priv->proxy_alloc = FALSE; + } else { + /* we transformed into something */ + if (gst_caps_is_equal (sink_suggest, othercaps)) { + GST_DEBUG_OBJECT (trans, "best caps same as input, marking for proxy"); + priv->proxy_alloc = TRUE; + } else { + GST_DEBUG_OBJECT (trans, + "best caps different from input, disable proxy"); + priv->proxy_alloc = FALSE; + } + } + if (othercaps) + gst_caps_unref (othercaps); + } + /* remember the new caps */ GST_OBJECT_LOCK (pad); gst_caps_replace (&priv->sink_alloc, sink_suggest); @@ -2079,9 +2084,7 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, } else { GST_DEBUG_OBJECT (trans, "received required caps from peer"); } - } - - if (suggest) { + } else if (suggest) { /* there was a custom suggestion, create a buffer of this format and return * it. Note that this format */ *buf = gst_buffer_new_and_alloc (size_suggest); @@ -2090,6 +2093,9 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, sink_suggest, sink_suggest); GST_BUFFER_CAPS (*buf) = sink_suggest; sink_suggest = NULL; + } else { + /* fallback buffer allocation by gst_pad_alloc_buffer() with the + * caps and size provided by the caller */ } if (sink_suggest) From aad7225eb5201ac5df1ed1044a44df9e5284e224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 8 Dec 2011 17:07:05 +0100 Subject: [PATCH 2/4] basetransform: Fall back to upstream provided caps if fixation of suggested caps failed --- libs/gst/base/gstbasetransform.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/gst/base/gstbasetransform.c b/libs/gst/base/gstbasetransform.c index 409b999101..9acb942d3d 100644 --- a/libs/gst/base/gstbasetransform.c +++ b/libs/gst/base/gstbasetransform.c @@ -1937,8 +1937,11 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, gst_pad_fixate_caps (GST_BASE_TRANSFORM_SINK_PAD (trans), sink_suggest); if (!gst_caps_is_fixed (sink_suggest)) { - gst_caps_unref (sink_suggest); - sink_suggest = NULL; + GST_DEBUG_OBJECT (trans, + "Impossible to fixate caps, using upstream caps"); + gst_caps_replace (&sink_suggest, caps); + size_suggest = size; + suggest = FALSE; } GST_DEBUG_OBJECT (trans, "Caps fixed to: %" GST_PTR_FORMAT, From 57573d8705d717e6dc8c3bf25eae95f59b62dd51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 8 Dec 2011 17:21:30 +0100 Subject: [PATCH 3/4] basetransform: Fall back to upstream provided caps if suggested caps are not supported by the sinkpad --- libs/gst/base/gstbasetransform.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/libs/gst/base/gstbasetransform.c b/libs/gst/base/gstbasetransform.c index 9acb942d3d..d762f7b356 100644 --- a/libs/gst/base/gstbasetransform.c +++ b/libs/gst/base/gstbasetransform.c @@ -1962,7 +1962,20 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, templ = gst_pad_get_pad_template_caps (pad); - if (!gst_caps_can_intersect (sink_suggest, templ)) { + /* Fall back to the upstream caps if the suggested caps + * are not actually supported. Shouldn't really happen + */ + if (suggest && !gst_caps_can_intersect (sink_suggest, templ)) { + GST_DEBUG_OBJECT (trans, + "Suggested caps not supported by sinkpad, using upstream caps"); + gst_caps_replace (&sink_suggest, caps); + size_suggest = size; + suggest = FALSE; + new_caps = sink_suggest + && !gst_caps_is_equal (sink_suggest, priv->sink_alloc); + } + + if (new_caps && (suggest || !gst_caps_can_intersect (sink_suggest, templ))) { GstCaps *allowed; GstCaps *peercaps; From a6bb6d0fc31da908b26ec8157f922e091b54c9b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 8 Dec 2011 18:00:00 +0100 Subject: [PATCH 4/4] basetransform: Fix code path to come up with possible caps if incompatible caps are provided to buffer_alloc() Previous code could almost never work and this should be slightly better. --- libs/gst/base/gstbasetransform.c | 39 ++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/libs/gst/base/gstbasetransform.c b/libs/gst/base/gstbasetransform.c index d762f7b356..9f9c935897 100644 --- a/libs/gst/base/gstbasetransform.c +++ b/libs/gst/base/gstbasetransform.c @@ -1824,7 +1824,6 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, GstCaps * caps, GstBuffer ** buf) { GstBaseTransform *trans; - GstBaseTransformClass *klass; GstBaseTransformPrivate *priv; GstFlowReturn res; gboolean alloced = FALSE; @@ -1835,7 +1834,6 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, trans = GST_BASE_TRANSFORM (gst_pad_get_parent (pad)); if (G_UNLIKELY (trans == NULL)) return GST_FLOW_WRONG_STATE; - klass = GST_BASE_TRANSFORM_GET_CLASS (trans); priv = trans->priv; GST_DEBUG_OBJECT (pad, "alloc with caps %p %" GST_PTR_FORMAT ", size %u", @@ -1976,8 +1974,7 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, } if (new_caps && (suggest || !gst_caps_can_intersect (sink_suggest, templ))) { - GstCaps *allowed; - GstCaps *peercaps; + GstCaps *allowed, *peercaps; GST_DEBUG_OBJECT (trans, "Requested pad alloc caps are not supported: %" GST_PTR_FORMAT, @@ -1990,25 +1987,47 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, GST_DEBUG_OBJECT (trans, "pads could agree on one of the following caps: " "%" GST_PTR_FORMAT, allowed); - allowed = gst_caps_make_writable (allowed); - if (klass->fixate_caps) { - peercaps = - gst_pad_get_allowed_caps (GST_BASE_TRANSFORM_SRC_PAD (trans)); - klass->fixate_caps (trans, GST_PAD_SRC, peercaps, allowed); + /* Check which caps would be possible with downstream */ + peercaps = + gst_pad_get_allowed_caps (GST_BASE_TRANSFORM_SRC_PAD (trans)); + if (peercaps) { + GstCaps *tmp, *intersect; + + tmp = + gst_base_transform_transform_caps (trans, GST_PAD_SRC, peercaps); gst_caps_unref (peercaps); + intersect = gst_caps_intersect (allowed, tmp); + gst_caps_unref (tmp); + gst_caps_unref (allowed); + + if (gst_caps_is_empty (intersect)) { + gst_caps_unref (intersect); + goto not_supported; + } + + allowed = intersect; } + allowed = gst_caps_make_writable (allowed); + /* Fixate them to be safe if the subclass didn't do it */ gst_caps_truncate (allowed); gst_pad_fixate_caps (pad, allowed); + + if (!gst_caps_is_fixed (allowed)) { + GST_ERROR_OBJECT (trans, "Impossible to fixate any caps"); + gst_caps_unref (allowed); + goto not_supported; + } + gst_caps_replace (&sink_suggest, allowed); gst_caps_unref (allowed); suggest = TRUE; new_caps = !gst_caps_is_equal (sink_suggest, priv->sink_alloc); - GST_DEBUG_OBJECT (trans, "Fixated suggestion caps to %" + GST_DEBUG_OBJECT (trans, "Calculated new suggestion caps %" GST_PTR_FORMAT, sink_suggest); } else { if (allowed)