GNOME Bugzilla – Bug 527488
[GstXML] can't load elements with request pads from XML
Last modified: 2009-05-20 08:19:58 UTC
Gstreamer has some problems with XML. Example: gst-launch videotestsrc ! tee name=t ! {queue ! ximagesink } { t. ! queue ! ximagesink } this will create a videosource, that will be splitted to two videosinks. Thats works. But if you save it to XML (with -o name.xml) and run it again with gst-xmllaunch name.xml you will see only one videosink. Reason: the tee element has only one sink pad instead of two, but in the xml file you can see two sink pads. if tried this example with some c-code: if you load this xml file with: GstXML* xml = gst_xml_new (); bool ret = gst_xml_parse_file (xml, (guchar*)"/home/herzogd/Desktop/test.xml", NULL); GstElement *pipeline = gst_xml_get_element (xml, (guchar*)"pipeline0"); GstElement *tee = gst_xml_get_element (xml, (guchar*)"tee0"); g_print("src count: %d", tee->numsrcpads); you will see, that the tee element had only one srcpad (should be two)! I've made some experiments: tee -> one sink tee->numsrcpads = 1, that is correct tee -> tho sinks tee->numsrcpads = 1, that is NOT correct (should be 2) tee -> three sinks tee->numsrcpads = 2, that is NOT correct (should be 3)
FWIW, GstXML and gst-xmllaunch are pretty much known to be in a bad state, if not broken, and no one really uses them. A patch would be nice though.
Created attachment 119282 [details] [review] Patch for gstpad.c (affects only loading from XML) Hi, since I want/need to get gst-editor to work properly I was also investigating this bug and found several quite fishy things. What happens?: The processing chain gets saved in the opposite direction, because g_list_prepend inside gst_bin_add turns it around. At the moment this opposite direction is even necessary, as there is another BUG in "gst_pad_load_and_link" function, that allows only linking from src to sink, but not the other way around. If the srcpad was created first, linking would fail. Reproduce it with: gst_bin_add_many (GST_BIN (pipeline), testsink,testsrc,NULL);//false direction gst_element_link_many (testsrc,testsink, NULL);//would be able to play now gst_xml_write_file...//will NOT play if this xml file is loaded This can be corrected with my patch. Before we call gst_pad_link we will check whether the pad is src or sink. if (gst_pad_get_direction(pad)==GST_PAD_SRC) gst_pad_link (pad,targetpad); else gst_pad_link (targetpad,pad); Ok but back to the main Bug: The chain is reloaded in the opposite direction, and both queues and both imagesinks are set up. Then GStreamer tries to restore the pads of tee. At the time of creation, they have been added to the element in "gst_element_add_pad" with g_list_prepend, so src1 will be restored first. In gsttee.c the Function gst_tee_request_new_pad (GstElement * element, GstPadTemplate * templ, const gchar * unused) is called...and ignores the proposed name, calling this pad src0. Linking succeeds, but now we try to restore pad src0; And we are getting smashed by the next bug: gst_element_get_static_pad(...), asked for src0, joyfully returns a pointer to our previously created request-pad. Linking fails because this pad is already linked. How to fix this: In gst_pad_load_and_link: if ((!pad)||(GST_PAD_ALWAYS!=GST_PAD_TEMPLATE_PRESENCE (gst_pad_get_pad_template (pad)))... instead of if (!pad)... This is also part of my patch. If we successfully created a request pad, but linking fails for some reason, we should release it in cleanup. But for some reason the tee element refuses to provide a pad again after we released it once. Anyway, the pad will be used later, but there may be some cases when this leads to further confusions...So what...It works better than before... The src1 and src0 are still exchanged. This would be fixed either by using prepend or by fixing tee or by both... My suggestions: 1. apply my patch to fix "gst_pad_load_and_link" 2. use append instead of prepend in gst_bin_add, gst_element_add_pad, we will never ever notice any loss of performance even on smartphones (NOT part of my patch as this may have side effects..., for me it works) this would fix that loading and saving a pipeline changes the order of elements 3. we should fix tee(as well as inputselector and outputselector) to respect the wishes of the user (also NOT part of my patch)
Created attachment 120793 [details] [review] New Patch This patch includes changes of my first patch and patches storing functions ind gstelement.c and gstbin.c that way that the lists are saved in the "wrong" direction, thus we can keep prepending each element/pad the way it is and everything works well.
Created attachment 121258 [details] [review] Yet another update of my patch Although the interest in this bug seems to be rather low, I am posting another update. In few days I will post a big patch-set of gst-editor, needing all this stuff for loading and saving chains, maybe then someone will be interested:) -As some bad element (at least input-selector from 0.10.7) return NULL on a gst_pad_get_pad_template, we have to check it to avoid segfaults. -New feature: Restore GstCaps from xml-description(if anyone knows a smarter way to do this please tell me:)
----PING---- Hello, this bug is still alive, although the patch seems to be stable(works perfectly now for me). Maybe you could add it to the next GStreamer release. Thanks!
-----PING------
Created attachment 134396 [details] [review] update for latest version of gstreamer modifications to gstutils.c are not needed for newer versions of gstreamer since restoring caps is supported by default now. (all done within gst_value_deserialize)
Hannes, I committed the patch with minor modifications. commit b47f4250697e2c4996596ebcf82a584a8eedbe82 Author: Hannes Bistry <bistry@informatik.uni-hamburg.de> Date: Wed May 20 10:56:11 2009 +0300 loadsave: fix requestpad handling and serialisation order. Support request pads when loading. Reverse pad serialisation order to preserve it when recreating the pipeline.