GNOME Bugzilla – Bug 683842
Fix race condition in videomixer2 on 0.10
Last modified: 2012-10-19 10:11:23 UTC
While doing some tests, I noticed that videomixer2 would sometimes give a wrong window size. In my use case I had two windows of 100x100, and I would use videomixer2 to mix them into a 200x100 image (by setting the xpos to 100 on one of videomixer's pads). When streaming starts, gst_videomixer2_pad_sink_setcaps gets called for each of the pads, and gst_videomixer2_update_src_caps gets called that finds/defines the best output resolution. The first time, it will decide on 100x100 (the resolution of the first video), then it will get called again for the second pad and decide on 200x100 (the resolution of the second video + its xpos). In both cases it calls gst_pad_set_caps, and a race condition could cause the second set_caps to be called while the first set_caps is in progress, and gst_pad_set_caps will simply output a "pad was dispatching" debug message but will not call the setcaps function, which is the one storing the width/height to the private struct. In the end, the 200x100 resolution might never be seen by the setcaps function and we end up with a 100x100 image and the second data source not being mixed. I have not found a good way to fix this race condition, but I've made a patch that at least works around the problem... : http://cgit.collabora.com/git/user/kakaroto/gst-plugins-good.git/commit/?h=videomixer&id=1a5a48888509ae0c4a11db4d65d85e411ed92a3a If anyone knows of a better fix for this issue, let me know, otherwise please review/merge my patch. Thanks.
I would think that only allowing one concurrent setcaps on the sinkpad would be what you really want. So some sort of lock (maybe an existing one or a new one) sounds better.
Humm... There already is a lock but it needs to be unlocked before calling the set_caps since the setcaps function takes it back... But I think it's a good idea to add a new lock only for the sink_setcaps purposes, this way it doesn't need to be unlocked when we call anything and it would block the second setcaps. Thanks for the idea, I will implement that and send a new patch. Could you also confirm that this thing isn't needed for GStreamer 1.0?
As discussed, solution implemented here : http://cgit.collabora.com/git/user/kakaroto/gst-plugins-good.git/commit/?h=videomixer&id=be5a7e7e006d5166edfe10edaedd03aa03b51af0
commit 13328bc129c1bec86637df561968e75eceddc291 Author: Youness Alaoui <youness.alaoui@collabora.co.uk> Date: Thu Sep 13 00:10:00 2012 +0000 videomixer2: Fix race condition where a src setcaps is ignored If both pads receive data at the same time, they will both get their sink_setcaps called which will call the src_setcaps, but there is a race condition where the second one might not be called. Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=683842