From 6ab4698b95bed4ca4032b791d84f26fd2e11224a Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 25 Nov 2008 16:06:22 +0000 Subject: 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. --- gst/videocrop/gstvideocrop.c | 109 ++++++++++++++++++------------------------- gst/videocrop/gstvideocrop.h | 2 - 2 files changed, 45 insertions(+), 66 deletions(-) (limited to 'gst/videocrop') diff --git a/gst/videocrop/gstvideocrop.c b/gst/videocrop/gstvideocrop.c index 78375545..e4fcbd30 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 0075997f..9911c7c5 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; -- cgit