GNOME Bugzilla – Bug 740787
videocrop: No longer apply the new crop if caps have not changed
Last modified: 2014-12-15 23:31:44 UTC
Created attachment 291602 [details] [review] videocrop: Make sure new crop is applied Since "basetransform: Fix caps equality check" commit a7f357, set_info() will not be called anymore if crop didn't change the caps. This patch fixes it by remembering if crop needed to be applied. This check if made in transform_caps, so during negotiation to avoid the race. Some note about the fix, it does not fix the race that always existed when calling set_property() in between transform_caps() and set_info() (one need to use a pad probe to make it work). It also does not fix that this element may fail without using GST_ELEMENT_ERROR().
Comment on attachment 291602 [details] [review] videocrop: Make sure new crop is applied Seems like a good solution for now, it doesn't make things worse ;) Might be good with some refactoring though, I don't see why videocrop should carry around the caps. It probably only uses the info anyway and only has the caps here because GstVideoFilter has that in the vfunc
Caps are only used for debug printing indeed, I don't like storing it in the element either. Just a note, I didn't realize that the element is using both the OBJECT_LOCK and a custom lock, so it is likely I'm using it wrong. I have no idea why a custom lock is needed, neither what it could solve.
Probably because more mutexes are better ;) From looking at the code it should just use the object lock. Would you mind updating your patch to do that and also get rid of the caps?
commit 36f1a9bce1454701d15dbb616f69eb3bddc3c9c3 Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Mon Dec 15 18:19:05 2014 -0500 videocrop: Make sure new crop is applied