diff --git a/ChangeLog b/ChangeLog index acf693cd40..3b38820b7d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +2008-08-14 Wim Taymans + + * libs/gst/base/gstbasetransform.c: + (gst_base_transform_transform_caps), + (gst_base_transform_prepare_output_buffer), + (gst_base_transform_buffer_alloc), (gst_base_transform_suggest): + Don't overwrite the outsize when calculating the expected size of a new + buffer because we still need it in case we cannot process the new + buffer. + When converting the size of the new buffer to an upstream size, actually + use the expected size of the buffer, not some other random value. + Use an atomic int to signal that a new upstream caps suggestion is + available. + When we can convert the current buffer to a new format, check if the + buffer size is of the expected size and allocate a new buffer of the + expected size when this is not the case. + + * tests/check/libs/transform1.c: (GST_START_TEST): + remove ifdeffed code from the unit test. + 2008-08-12 Stefan Kost * pkgconfig/gstreamer-uninstalled.pc.in: diff --git a/libs/gst/base/gstbasetransform.c b/libs/gst/base/gstbasetransform.c index 60a5904afb..c8d5993c6f 100644 --- a/libs/gst/base/gstbasetransform.c +++ b/libs/gst/base/gstbasetransform.c @@ -251,6 +251,7 @@ struct _GstBaseTransformPrivate /* upstream caps and size suggestions */ GstCaps *sink_suggest; guint size_suggest; + gint suggest_pending; }; static GstElementClass *parent_class = NULL; @@ -474,6 +475,7 @@ gst_base_transform_transform_caps (GstBaseTransform * trans, GST_LOG_OBJECT (trans, " to[%d]: %" GST_PTR_FORMAT, i, temp); temp = gst_caps_make_writable (temp); + /* here we need to only append those structures, that are not yet * in there, we use the merge function for this */ gst_caps_merge (ret, temp); @@ -1062,7 +1064,7 @@ gst_base_transform_prepare_output_buffer (GstBaseTransform * trans, GstBaseTransformClass *bclass; GstBaseTransformPrivate *priv; GstFlowReturn ret = GST_FLOW_OK; - guint outsize, newsize; + guint outsize, newsize, expsize; gboolean discard; GstCaps *incaps, *oldcaps, *newcaps; @@ -1148,9 +1150,10 @@ gst_base_transform_prepare_output_buffer (GstBaseTransform * trans, incaps = GST_PAD_CAPS (trans->sinkpad); /* it's possible that the buffer we got is of the wrong size, get the - * expected size here */ + * expected size here, we will check the size if we are going to use the + * buffer later on. */ gst_base_transform_transform_size (trans, - GST_PAD_SINK, incaps, GST_BUFFER_SIZE (in_buf), newcaps, &outsize); + GST_PAD_SINK, incaps, GST_BUFFER_SIZE (in_buf), newcaps, &expsize); /* check if we can convert the current incaps to the new target caps */ can_convert = @@ -1168,6 +1171,14 @@ gst_base_transform_prepare_output_buffer (GstBaseTransform * trans, /* new format configure, and use the new output buffer */ gst_pad_set_caps (trans->srcpad, newcaps); discard = FALSE; + /* if we got a buffer of the wrong size, discard it now and make sure we + * allocate a propertly sized buffer later. */ + if (newsize != expsize) { + if (in_buf != *out_buf) + gst_buffer_unref (*out_buf); + *out_buf = NULL; + } + outsize = expsize; } else { GST_DEBUG_OBJECT (trans, "cannot perform transform on current buffer"); @@ -1189,7 +1200,7 @@ gst_base_transform_prepare_output_buffer (GstBaseTransform * trans, /* not a subset, we have a new upstream suggestion, remember it and * allocate a default buffer. First we try to convert the size */ if (gst_base_transform_transform_size (trans, - GST_PAD_SRC, newcaps, outsize, othercaps, &size_suggest)) { + GST_PAD_SRC, newcaps, expsize, othercaps, &size_suggest)) { /* ok, remember the suggestions now */ GST_DEBUG_OBJECT (trans, @@ -1201,6 +1212,7 @@ gst_base_transform_prepare_output_buffer (GstBaseTransform * trans, gst_caps_unref (priv->sink_suggest); priv->sink_suggest = gst_caps_ref (othercaps); priv->size_suggest = size_suggest; + g_atomic_int_set (&trans->priv->suggest_pending, 1); GST_OBJECT_UNLOCK (trans->sinkpad); } gst_caps_unref (othercaps); @@ -1209,19 +1221,12 @@ gst_base_transform_prepare_output_buffer (GstBaseTransform * trans, gst_buffer_unref (*out_buf); *out_buf = NULL; } - - /* if we got a buffer of the wrong size, discard it now */ - if (newsize != outsize) { - if (in_buf != *out_buf) - gst_buffer_unref (*out_buf); - *out_buf = NULL; - discard = FALSE; - } } if (*out_buf == NULL) { if (!discard) { - GST_DEBUG_OBJECT (trans, "make default output buffer"); + GST_DEBUG_OBJECT (trans, "make default output buffer of size %d", + outsize); /* no valid buffer yet, make one */ *out_buf = gst_buffer_new_and_alloc (outsize); gst_buffer_copy_metadata (*out_buf, in_buf, @@ -1350,7 +1355,8 @@ 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. */ - if (caps && gst_caps_is_equal (priv->sink_alloc, caps)) { + if (g_atomic_int_get (&priv->suggest_pending) == 0 && caps && + gst_caps_is_equal (priv->sink_alloc, caps)) { /* we have seen this before, see if we need to proxy */ GST_DEBUG_OBJECT (trans, "have old caps"); sink_suggest = caps; @@ -1376,6 +1382,7 @@ gst_base_transform_buffer_alloc (GstPad * pad, guint64 offset, guint size, sink_suggest = caps; suggest = FALSE; } + g_atomic_int_set (&priv->suggest_pending, 0); GST_OBJECT_UNLOCK (pad); /* check if we actually handle this format on the sinkpad */ @@ -2233,7 +2240,7 @@ gst_base_transform_suggest (GstBaseTransform * trans, GstCaps * caps, gst_caps_unref (trans->priv->sink_suggest); trans->priv->sink_suggest = gst_caps_copy (caps); trans->priv->size_suggest = size; - gst_caps_replace (&trans->priv->sink_alloc, NULL); + g_atomic_int_set (&trans->priv->suggest_pending, 1); GST_DEBUG_OBJECT (trans, "new suggest %" GST_PTR_FORMAT, caps); GST_OBJECT_UNLOCK (trans->sinkpad); } diff --git a/tests/check/libs/transform1.c b/tests/check/libs/transform1.c index 1039858ca6..a3efb5ddcd 100644 --- a/tests/check/libs/transform1.c +++ b/tests/check/libs/transform1.c @@ -27,8 +27,6 @@ #include #include -#define FAILING_TESTS - #include "test_transform.c" static gboolean buffer_alloc_pt1_called; @@ -81,10 +79,8 @@ GST_START_TEST (basetransform_chain_pt1) set_caps_pt1_called = FALSE;; res = gst_test_trans_push (trans, buffer); fail_unless (res == GST_FLOW_OK); -#ifdef FAILING_TESTS /* FIXME, passthough without pad-alloc, do pad-alloc on the srcpad */ fail_unless (buffer_alloc_pt1_called == TRUE); -#endif fail_unless (set_caps_pt1_called == FALSE); buffer = gst_test_trans_pop (trans); @@ -102,10 +98,8 @@ GST_START_TEST (basetransform_chain_pt1) set_caps_pt1_called = FALSE;; res = gst_test_trans_push (trans, buffer); fail_unless (res == GST_FLOW_OK); -#ifdef FAILING_TESTS /* FIXME, passthough without pad-alloc, do pad-alloc on the srcpad */ fail_unless (buffer_alloc_pt1_called == TRUE); -#endif fail_unless (set_caps_pt1_called == FALSE); buffer = gst_test_trans_pop (trans); @@ -194,10 +188,8 @@ GST_START_TEST (basetransform_chain_pt2) set_caps_pt2_called = FALSE; res = gst_test_trans_push (trans, buffer); fail_unless (res == GST_FLOW_OK); -#ifdef FAILING_TESTS /* FIXME, passthough without pad-alloc, do pad-alloc on the srcpad */ fail_unless (buffer_alloc_pt1_called == TRUE); -#endif fail_unless (set_caps_pt2_called == TRUE); buffer = gst_test_trans_pop (trans); @@ -236,10 +228,8 @@ GST_START_TEST (basetransform_chain_pt2) set_caps_pt2_called = FALSE; res = gst_test_trans_push (trans, buffer); fail_unless (res == GST_FLOW_OK); -#ifdef FAILING_TESTS /* FIXME, passthough without pad-alloc, do pad-alloc on the srcpad */ fail_unless (buffer_alloc_pt1_called == TRUE); -#endif fail_unless (set_caps_pt2_called == TRUE); buffer = gst_test_trans_pop (trans); @@ -325,10 +315,8 @@ GST_START_TEST (basetransform_chain_ip1) fail_unless (res == GST_FLOW_OK); fail_unless (transform_ip_1_called == TRUE); fail_unless (transform_ip_1_writable == TRUE); -#ifdef FAILING_TESTS /* FIXME, in-place without pad-alloc, do pad-alloc on the srcpad */ fail_unless (buffer_alloc_pt1_called == TRUE); -#endif buffer = gst_test_trans_pop (trans); fail_unless (buffer != NULL); @@ -468,10 +456,8 @@ GST_START_TEST (basetransform_chain_ip2) fail_unless (transform_ip_1_called == TRUE); fail_unless (transform_ip_1_writable == TRUE); fail_unless (set_caps_1_called == TRUE); -#ifdef FAILING_TESTS /* FIXME, in-place without pad-alloc, do pad-alloc on the srcpad */ fail_unless (buffer_alloc_pt1_called == TRUE); -#endif buffer = gst_test_trans_pop (trans); fail_unless (buffer != NULL); @@ -673,11 +659,9 @@ GST_START_TEST (basetransform_chain_ct1) buffer_alloc_ct1_called = FALSE; res = gst_pad_alloc_buffer (trans->srcpad, 0, 20, outcaps, &buffer); fail_unless (res == GST_FLOW_NOT_NEGOTIATED); -#ifdef FAILING_TESTS /* FIXME, why would this call the alloc function? we try to alloc something * with caps that are not supported on the sinkpad */ fail_unless (buffer_alloc_ct1_called == FALSE); -#endif /* with caps buffer */ GST_DEBUG_OBJECT (trans, "alloc with caps, size 20"); @@ -763,12 +747,10 @@ GST_START_TEST (basetransform_chain_ct1) buffer_alloc_ct1_called = FALSE; res = gst_pad_alloc_buffer (trans->srcpad, 0, 10, incaps, &buffer); fail_unless (res == GST_FLOW_OK); -#ifdef FAILING_TESTS /* should not call pad-alloc because the caps and sizes are different, it * currently still calls the pad alloc for no reason and then throws away the * buffer. */ fail_unless (buffer_alloc_ct1_called == FALSE); -#endif fail_unless (GST_BUFFER_SIZE (buffer) == 10); gst_buffer_unref (buffer); @@ -778,10 +760,8 @@ GST_START_TEST (basetransform_chain_ct1) buffer_alloc_ct1_called = FALSE; res = gst_pad_alloc_buffer (trans->srcpad, 0, 10, outcaps, &buffer); fail_unless (res == GST_FLOW_NOT_NEGOTIATED); -#ifdef FAILING_TESTS /* should not call the pad-alloc function */ fail_unless (buffer_alloc_ct1_called == FALSE); -#endif gst_caps_unref (incaps); gst_caps_unref (outcaps); @@ -970,10 +950,8 @@ GST_START_TEST (basetransform_chain_ct2) buffer_alloc_ct2_called = FALSE; res = gst_pad_alloc_buffer (trans->srcpad, 0, 20, outcaps, &buffer); fail_unless (res == GST_FLOW_NOT_NEGOTIATED); -#ifdef FAILING_TESTS /* should not call pad-alloc because the caps and sizes are different */ fail_unless (buffer_alloc_ct2_called == FALSE); -#endif /* first try to push a buffer without caps, this should fail */ buffer = gst_buffer_new_and_alloc (20); @@ -1063,10 +1041,8 @@ GST_START_TEST (basetransform_chain_ct2) buffer_alloc_ct2_called = FALSE; res = gst_pad_alloc_buffer (trans->srcpad, 0, 10, outcaps, &buffer); fail_unless (res == GST_FLOW_NOT_NEGOTIATED); -#ifdef FAILING_TESTS /* should not call the pad-alloc function */ fail_unless (buffer_alloc_ct2_called == FALSE); -#endif gst_caps_unref (incaps); gst_caps_unref (outcaps); @@ -1127,10 +1103,8 @@ GST_START_TEST (basetransform_chain_ct3) buffer_alloc_ct2_called = FALSE; res = gst_pad_alloc_buffer (trans->srcpad, 0, 20, outcaps, &buffer); fail_unless (res == GST_FLOW_NOT_NEGOTIATED); -#ifdef FAILING_TESTS /* should not call pad-alloc because the caps and sizes are different */ fail_unless (buffer_alloc_ct2_called == FALSE); -#endif /* first try to push a buffer without caps, this should fail */ buffer = gst_buffer_new_and_alloc (20); @@ -1163,9 +1137,7 @@ GST_START_TEST (basetransform_chain_ct3) fail_unless (res == GST_FLOW_OK); fail_unless (transform_ct2_called == FALSE); fail_unless (set_caps_ct2_called == TRUE); -#ifdef FAILING_TESTS fail_unless (buffer_alloc_ct2_called == TRUE); -#endif buffer = gst_test_trans_pop (trans); fail_unless (buffer != NULL); @@ -1187,9 +1159,7 @@ GST_START_TEST (basetransform_chain_ct3) res = gst_test_trans_push (trans, buffer); fail_unless (res == GST_FLOW_OK); fail_unless (transform_ct2_called == FALSE); -#ifdef FAILING_TESTS fail_unless (buffer_alloc_ct2_called == TRUE); -#endif /* after push, get rid of the final ref we had */ gst_buffer_unref (buffer); @@ -1220,10 +1190,8 @@ GST_START_TEST (basetransform_chain_ct3) buffer_alloc_ct2_called = FALSE; res = gst_pad_alloc_buffer (trans->srcpad, 0, 10, outcaps, &buffer); fail_unless (res == GST_FLOW_NOT_NEGOTIATED); -#ifdef FAILING_TESTS /* FIXME should not call the pad-alloc function but it currently does */ fail_unless (buffer_alloc_ct2_called == FALSE); -#endif /* change the return value of the buffer-alloc function */ GST_DEBUG_OBJECT (trans, "switching transform output"); @@ -1242,21 +1210,17 @@ GST_START_TEST (basetransform_chain_ct3) res = gst_test_trans_push (trans, buffer); fail_unless (res == GST_FLOW_OK); fail_unless (transform_ct2_called == TRUE); -#ifdef FAILING_TESTS /* FIXME, pad alloc must be called to get the new caps, because we don't call * pad alloc */ fail_unless (buffer_alloc_ct2_called == TRUE); -#endif buffer = gst_test_trans_pop (trans); fail_unless (buffer != NULL); -#ifdef FAILING_TESTS /* FIXME changing src caps should produce converted buffer */ GST_DEBUG_OBJECT (trans, "received caps %" GST_PTR_FORMAT, GST_BUFFER_CAPS (buffer)); fail_unless (gst_caps_is_equal (GST_BUFFER_CAPS (buffer), outcaps)); fail_unless (GST_BUFFER_SIZE (buffer) == 20); -#endif /* output buffer has refcount 1 */ fail_unless (GST_MINI_OBJECT_REFCOUNT_VALUE (buffer) == 1); @@ -1272,15 +1236,11 @@ GST_START_TEST (basetransform_chain_ct3) res = gst_pad_alloc_buffer (trans->srcpad, 0, 10, incaps, &buffer); fail_unless (res == GST_FLOW_OK); fail_unless (buffer_alloc_ct2_called == TRUE); -#ifdef FAILING_TESTS /* FIXME a buffer alloc should never set caps */ fail_unless (set_caps_ct2_called == FALSE); -#endif fail_unless (GST_BUFFER_SIZE (buffer) == 10); -#ifdef FAILING_TESTS /* FIXME, ideally we want to reuse these caps */ fail_unless (GST_BUFFER_CAPS (buffer) == incaps); -#endif fail_unless (gst_caps_is_equal (GST_BUFFER_CAPS (buffer), incaps)); gst_buffer_unref (buffer);