From 6ab4698b95bed4ca4032b791d84f26fd2e11224a Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 25 Nov 2008 16:06:22 +0000 Subject: [PATCH] gst/videocrop/gstvideocrop.*: Fix renegotiation when changing properties using the new basetransform features. Fixes ... Original commit message from CVS: * gst/videocrop/gstvideocrop.c: (gst_video_crop_init), (gst_video_crop_transform), (gst_video_crop_transform_caps), (gst_video_crop_set_caps), (gst_video_crop_set_property): * gst/videocrop/gstvideocrop.h: Fix renegotiation when changing properties using the new basetransform features. Fixes #561502. * tests/icles/Makefile.am: * tests/icles/videocrop2-test.c: (make_pipeline), (main): Add crazy interactive test unit for dynamically changing properties. --- ChangeLog | 13 ++++ gst/videocrop/gstvideocrop.c | 109 ++++++++++++++------------------ gst/videocrop/gstvideocrop.h | 2 - tests/icles/Makefile.am | 7 ++- tests/icles/videocrop2-test.c | 141 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 205 insertions(+), 67 deletions(-) create mode 100644 tests/icles/videocrop2-test.c diff --git a/ChangeLog b/ChangeLog index 391445d..864787d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2008-11-25 Wim Taymans + + * gst/videocrop/gstvideocrop.c: (gst_video_crop_init), + (gst_video_crop_transform), (gst_video_crop_transform_caps), + (gst_video_crop_set_caps), (gst_video_crop_set_property): + * gst/videocrop/gstvideocrop.h: + Fix renegotiation when changing properties using the new basetransform + features. Fixes #561502. + + * tests/icles/Makefile.am: + * tests/icles/videocrop2-test.c: (make_pipeline), (main): + Add crazy interactive test unit for dynamically changing properties. + 2008-11-24 Wim Taymans * gst/rtsp/gstrtspsrc.c: (new_session_pad), diff --git a/gst/videocrop/gstvideocrop.c b/gst/videocrop/gstvideocrop.c index 7837554..e4fcbd3 100644 --- a/gst/videocrop/gstvideocrop.c +++ b/gst/videocrop/gstvideocrop.c @@ -193,8 +193,6 @@ gst_video_crop_init (GstVideoCrop * vcrop, GstVideoCropClass * klass) vcrop->crop_left = 0; vcrop->crop_top = 0; vcrop->crop_bottom = 0; - vcrop->noop = TRUE; - GST_BASE_TRANSFORM (vcrop)->passthrough = vcrop->noop; } static gboolean @@ -432,17 +430,6 @@ gst_video_crop_transform (GstBaseTransform * trans, GstBuffer * inbuf, { GstVideoCrop *vcrop = GST_VIDEO_CROP (trans); - /* we should be operating in passthrough mode if there's nothing to do */ - g_assert (vcrop->noop == FALSE); - - GST_OBJECT_LOCK (vcrop); - - if (G_UNLIKELY ((vcrop->crop_left + vcrop->crop_right) >= vcrop->in.width || - (vcrop->crop_top + vcrop->crop_bottom) >= vcrop->in.height)) { - GST_OBJECT_UNLOCK (vcrop); - goto cropping_too_much; - } - switch (vcrop->in.packing) { case VIDEO_CROP_PIXEL_FORMAT_PACKED_SIMPLE: gst_video_crop_transform_packed_simple (vcrop, inbuf, outbuf); @@ -457,17 +444,7 @@ gst_video_crop_transform (GstBaseTransform * trans, GstBuffer * inbuf, g_assert_not_reached (); } - GST_OBJECT_UNLOCK (vcrop); - return GST_FLOW_OK; - -cropping_too_much: - { - /* is there a better error code for this? */ - GST_ELEMENT_ERROR (vcrop, LIBRARY, SETTINGS, (NULL), - ("Can't crop more pixels than there are")); - return GST_FLOW_ERROR; - } } static gint @@ -537,14 +514,8 @@ gst_video_crop_transform_caps (GstBaseTransform * trans, GST_OBJECT_LOCK (vcrop); - GST_LOG_OBJECT (vcrop, "l=%d,r=%d,b=%d,t=%d noop=%d", - vcrop->crop_left, vcrop->crop_right, vcrop->crop_bottom, - vcrop->crop_top, vcrop->noop); - - if (vcrop->noop) { - GST_OBJECT_UNLOCK (vcrop); - return gst_caps_ref (caps); - } + GST_LOG_OBJECT (vcrop, "l=%d,r=%d,b=%d,t=%d", + vcrop->crop_left, vcrop->crop_right, vcrop->crop_bottom, vcrop->crop_top); if (direction == GST_PAD_SRC) { dx = vcrop->crop_left + vcrop->crop_right; @@ -606,39 +577,48 @@ gst_video_crop_set_caps (GstBaseTransform * trans, GstCaps * incaps, { GstVideoCrop *crop = GST_VIDEO_CROP (trans); - if (!gst_video_crop_get_image_details_from_caps (crop, &crop->in, incaps)) { + if (!gst_video_crop_get_image_details_from_caps (crop, &crop->in, incaps)) + goto wrong_input; + + if (!gst_video_crop_get_image_details_from_caps (crop, &crop->out, outcaps)) + goto wrong_output; + + if (G_UNLIKELY ((crop->crop_left + crop->crop_right) >= crop->in.width || + (crop->crop_top + crop->crop_bottom) >= crop->in.height)) + goto cropping_too_much; + + GST_LOG_OBJECT (crop, "incaps = %" GST_PTR_FORMAT ", outcaps = %" + GST_PTR_FORMAT, incaps, outcaps); + + if ((crop->crop_left | crop->crop_right | crop->crop_top | crop-> + crop_bottom) == 0) { + GST_LOG_OBJECT (crop, "we are using passthrough"); + gst_base_transform_set_passthrough (GST_BASE_TRANSFORM (crop), TRUE); + } else { + GST_LOG_OBJECT (crop, "we are not using passthrough"); + gst_base_transform_set_passthrough (GST_BASE_TRANSFORM (crop), FALSE); + } + + return TRUE; + + /* ERROR */ +wrong_input: + { GST_DEBUG_OBJECT (crop, "failed to parse input caps %" GST_PTR_FORMAT, incaps); return FALSE; } - - if (!gst_video_crop_get_image_details_from_caps (crop, &crop->out, outcaps)) { +wrong_output: + { GST_DEBUG_OBJECT (crop, "failed to parse output caps %" GST_PTR_FORMAT, outcaps); return FALSE; } - - GST_LOG_OBJECT (crop, "incaps = %" GST_PTR_FORMAT ", outcaps = %" - GST_PTR_FORMAT, incaps, outcaps); - - return TRUE; -} - -/* This is extremely hackish, but the only way to force basetransform to - * renegotiated at the moment. There should really be a basetransform - * function for this */ -static void -gst_videocrop_clear_negotiated_caps_locked (GstVideoCrop * crop) -{ - GST_LOG_OBJECT (crop, "clearing negotiated caps"); - GST_BASE_TRANSFORM (crop)->negotiated = FALSE; - gst_caps_replace (&GST_PAD_CAPS (GST_BASE_TRANSFORM (crop)->srcpad), NULL); - gst_caps_replace (&GST_PAD_CAPS (GST_BASE_TRANSFORM (crop)->sinkpad), NULL); - gst_caps_replace (&GST_BASE_TRANSFORM (crop)->cache_caps1, NULL); - GST_BASE_TRANSFORM (crop)->cache_caps1_size = 0; - gst_caps_replace (&GST_BASE_TRANSFORM (crop)->cache_caps2, NULL); - GST_BASE_TRANSFORM (crop)->cache_caps2_size = 0; - GST_LOG_OBJECT (crop, "clearing caps done"); +cropping_too_much: + { + GST_DEBUG_OBJECT (crop, "we are cropping too much"); + return FALSE; + } } static void @@ -649,6 +629,10 @@ gst_video_crop_set_property (GObject * object, guint prop_id, video_crop = GST_VIDEO_CROP (object); + /* don't modify while we are transforming */ + GST_BASE_TRANSFORM_LOCK (GST_BASE_TRANSFORM_CAST (video_crop)); + + /* protect with the object lock so that we can read them */ GST_OBJECT_LOCK (video_crop); switch (prop_id) { case ARG_LEFT: @@ -667,17 +651,14 @@ gst_video_crop_set_property (GObject * object, guint prop_id, G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; } + GST_OBJECT_UNLOCK (video_crop); - video_crop->noop = ((video_crop->crop_left | video_crop->crop_right | - video_crop->crop_top | video_crop->crop_bottom) == 0); - - GST_LOG_OBJECT (video_crop, "l=%d,r=%d,b=%d,t=%d noop=%d", + GST_LOG_OBJECT (video_crop, "l=%d,r=%d,b=%d,t=%d", video_crop->crop_left, video_crop->crop_right, video_crop->crop_bottom, - video_crop->crop_top, video_crop->noop); + video_crop->crop_top); - GST_BASE_TRANSFORM (video_crop)->passthrough = video_crop->noop; - gst_videocrop_clear_negotiated_caps_locked (video_crop); - GST_OBJECT_UNLOCK (video_crop); + gst_base_transform_reconfigure (GST_BASE_TRANSFORM (video_crop)); + GST_BASE_TRANSFORM_UNLOCK (GST_BASE_TRANSFORM_CAST (video_crop)); } static void diff --git a/gst/videocrop/gstvideocrop.h b/gst/videocrop/gstvideocrop.h index 0075997..9911c7c 100644 --- a/gst/videocrop/gstvideocrop.h +++ b/gst/videocrop/gstvideocrop.h @@ -70,8 +70,6 @@ struct _GstVideoCrop GstBaseTransform basetransform; /*< private >*/ - gboolean noop; /* TRUE if crop_left,_right,_top and _bottom are all 0 */ - gint crop_left; gint crop_right; gint crop_top; diff --git a/tests/icles/Makefile.am b/tests/icles/Makefile.am index 0f545ce..3cacdd1 100644 --- a/tests/icles/Makefile.am +++ b/tests/icles/Makefile.am @@ -40,5 +40,10 @@ videobox_test_CFLAGS = $(GST_CFLAGS) videobox_test_LDADD = $(GST_LIBS) videobox_test_LDFLAGS = $(GST_PLUGIN_LDFLAGS) -noinst_PROGRAMS = $(GTK_TESTS) $(V4L2_TESTS) $(X_TESTS) videocrop-test videobox-test +videocrop2_test_SOURCES = videocrop2-test.c +videocrop2_test_CFLAGS = $(GST_CFLAGS) +videocrop2_test_LDADD = $(GST_LIBS) +videocrop2_test_LDFLAGS = $(GST_PLUGIN_LDFLAGS) + +noinst_PROGRAMS = $(GTK_TESTS) $(V4L2_TESTS) $(X_TESTS) videocrop-test videobox-test videocrop2-test diff --git a/tests/icles/videocrop2-test.c b/tests/icles/videocrop2-test.c new file mode 100644 index 0000000..58d1932 --- /dev/null +++ b/tests/icles/videocrop2-test.c @@ -0,0 +1,141 @@ +/* GStreamer interactive videocrop test + * + * Copyright (C) 2008 Wim Taymans + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Library General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Library General Public License for more details. + * + * You should have received a copy of the GNU Library General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include + +#include + +static GstElement * +make_pipeline (gint type) +{ + GstElement *result; + gchar *pstr; + + switch (type) { + case 0: + pstr = g_strdup_printf ("videotestsrc ! videocrop name=crop ! " + "xvimagesink"); + break; + default: + return NULL; + } + + result = gst_parse_launch_full (pstr, NULL, GST_PARSE_FLAG_NONE, NULL); + g_print ("created test %d: \"%s\"\n", type, pstr); + g_free (pstr); + + return result; +} + +#define MAX_ROUND 500 + +int +main (int argc, char **argv) +{ + GstElement *pipe, *crop; + gint left, right; + gint top, bottom; + gint ldir, rdir; + gint tdir, bdir; + gint round, type, stop; + + gst_init (&argc, &argv); + + type = 0; + stop = -1; + + if (argc > 1) { + type = atoi (argv[1]); + stop = type + 1; + } + + while (TRUE) { + GstMessage *message; + + pipe = make_pipeline (type); + if (pipe == NULL) + break; + + crop = gst_bin_get_by_name (GST_BIN (pipe), "crop"); + g_assert (crop); + + top = bottom = left = right = 0; + tdir = bdir = 10; + ldir = rdir = 10; + + for (round = 0; round < MAX_ROUND; round++) { + g_print ("crop to %4d %4d %4d %4d (%d/%d) \r", top, bottom, left, right, + round, MAX_ROUND); + + g_object_set (crop, "top", top, "bottom", bottom, "left", left, "right", + right, NULL); + + if (round == 0) + gst_element_set_state (pipe, GST_STATE_PLAYING); + + top += tdir; + if (top >= 80) + tdir = -10; + else if (top < 10) + tdir = 10; + + bottom += bdir; + if (bottom >= 60) + bdir = -10; + else if (bottom < 10) + bdir = 10; + + left += ldir; + if (left >= 100) + ldir = -10; + else if (left < 10) + ldir = 10; + + right += rdir; + if (right >= 80) + rdir = -10; + else if (right < 10) + rdir = 10; + + message = + gst_bus_poll (GST_ELEMENT_BUS (pipe), GST_MESSAGE_ERROR, + 50 * GST_MSECOND); + if (message) { + g_print ("got error \n"); + + gst_message_unref (message); + } + } + g_print ("test %d done \n", type); + + gst_object_unref (crop); + gst_element_set_state (pipe, GST_STATE_NULL); + gst_object_unref (pipe); + + type++; + if (type == stop) + break; + } + return 0; +} -- 2.7.4