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 612881 - [utils] gst_pad_proxy_setcaps() doesn't handler iterator resyncs
[utils] gst_pad_proxy_setcaps() doesn't handler iterator resyncs
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal critical
: 0.10.29
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-03-14 16:22 UTC by Håvard Graff (hgr)
Modified: 2010-04-25 18:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (4.23 KB, patch)
2010-03-14 16:22 UTC, Håvard Graff (hgr)
rejected Details | Review
utils: Handle iterator resyncs in gst_pad_proxy_setcaps() (2.41 KB, patch)
2010-03-15 13:06 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Håvard Graff (hgr) 2010-03-14 16:22:30 UTC
Created attachment 156125 [details] [review]
proposed patch

When setcaps is called on gsttee's sinkpad, it will make an iterator over all the srcpads. While it is iterating, the padlist might change with requestpads being requested or released, which again will make the iteration abort, and setcaps fail.

This solution proposes a separate lock to serialize the sinkpad setcaps and the adding and removing of pads to the element.
Comment 1 Wim Taymans 2010-03-15 10:00:55 UTC
how about making the proxy_setcaps function retry when the pads changed (like gst_pad_proxy_getcaps)?
Comment 2 Håvard Graff (hgr) 2010-03-15 11:02:41 UTC
sure, that could work too! And might fix the issue for other request-pad elements? If you make a patch I can try out.
Comment 3 Sebastian Dröge (slomo) 2010-03-15 12:59:55 UTC
(In reply to comment #1)
> how about making the proxy_setcaps function retry when the pads changed (like
> gst_pad_proxy_getcaps)?

That was my first thought when seeing this bug too ;) I'll prepare a patch...
Comment 4 Sebastian Dröge (slomo) 2010-03-15 13:06:26 UTC
Created attachment 156173 [details] [review]
utils: Handle iterator resyncs in gst_pad_proxy_setcaps()

Fixes bug #612881.
Comment 5 Sebastian Dröge (slomo) 2010-03-15 13:54:33 UTC
commit fbbb671e54b15d34fac09e4926fda877372a465b
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Mon Mar 15 14:05:35 2010 +0100

    utils: Handle iterator resyncs in gst_pad_proxy_setcaps()
    
    Fixes bug #612881.
Comment 6 Håvard Graff (hgr) 2010-04-13 08:18:03 UTC
I see from your fix that GST_ITERATOR_OK is handled like an error.

I have seen a couple of times that this indeed can be the case when stressing pad-request/release on gsttee, so my question is wether GST_ITERATOR_OK is always an error, or if this also could get a resync?

So that you could have:

    switch (res) {
      case GST_ITERATOR_RESYNC:
      case GST_ITERATOR_OK:
        /* reset return value */
        g_value_set_boolean (&ret, TRUE);
        gst_iterator_resync (iter);
        break;
      case GST_ITERATOR_DONE:
        /* all pads iterated, return collected value */
        goto done;
      default:
        /* iterator returned _ERROR or premature end with _OK,
         * mark an error and exit */
        goto error;
    }

Any thoughts?
Comment 7 Wim Taymans 2010-04-13 08:27:32 UTC
OK is returned when the fold function returns FALSE, which is when a set_caps function call on the pad fails.

Usually set_caps doesn't fail unless something is seriously wrong, such as setting caps on a pad that are incompatible with the template.

I guess doing a resync is not a good idea, there is no guarantee that the setcaps will succeed the next time.

Can you see why a particular setcaps call fails?
Comment 8 Sebastian Dröge (slomo) 2010-04-13 08:28:54 UTC
OK means that the fold function returned FALSE, which means that setting the caps on one of the pads has failed. And then we should simply error out IMHO, resyncing would lead to an infinite loop in the worst case.
ERROR is returned if there was an iterator error, which should give the same behaviour I think.


DONE will be returned if everything was handled and the func always returned TRUE.


So your problem is, that sometimes gst_pad_set_caps() fails but on a second try it works?
Comment 9 Tim-Philipp Müller 2010-04-25 18:23:02 UTC
Håvard: ping!

I'm closing this bug for now, since the original issue seems fixed. 

If you have further information about the set_caps() issue, please re-open or (preferably) file a new bug, thanks!
Comment 10 Håvard Graff (hgr) 2010-04-25 18:25:49 UTC
Ok with me! :)