After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 683842 - Fix race condition in videomixer2 on 0.10
Fix race condition in videomixer2 on 0.10
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.x
Other Linux
: Normal normal
: 1.0.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-09-12 01:14 UTC by Youness Alaoui
Modified: 2012-10-19 10:11 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Youness Alaoui 2012-09-12 01:14:13 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.
Comment 1 Wim Taymans 2012-09-12 08:59:38 UTC
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.
Comment 2 Youness Alaoui 2012-09-12 18:30:59 UTC
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?
Comment 4 Sebastian Dröge (slomo) 2012-10-19 10:11:23 UTC
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