[gstreamer-bugs] [Bug 527488] [GstXML] can't load elements with request pads from XML

[GStreamer (bugzilla.gnome.org)]

If you have any questions why you received this email, please see the text at
the end of this email. Replies to this email are NOT read, please see the text
at the end of this email. You can add comments to this bug at:
  http://bugzilla.gnome.org/show_bug.cgi?id=527488

  GStreamer | gstreamer (core) | Ver: 0.10.19




------- Comment #2 from Hannes Bistry  2008-09-24 08:38 UTC -------
Created an attachment (id=119282)
 --> (http://bugzilla.gnome.org/attachment.cgi?id=119282&action=view)
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)


-- 
See http://bugzilla.gnome.org/page.cgi?id=email.html for more info about why you received
this email, why you can't respond via email, how to stop receiving
emails (or reduce the number you receive), and how to contact someone
if you are having problems with the system.

You can add comments to this bug at http://bugzilla.gnome.org/show_bug.cgi?id=527488.