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 329069 - API: add gst_parse_bin_from_description
API: add gst_parse_bin_from_description
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 0.10.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 324508 329106
 
 
Reported: 2006-01-29 12:13 UTC by Tim-Philipp Müller
Modified: 2006-02-04 15:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adds gst_bin_parse_from_description() and gst_bin_find_unconnected_pad() (6.84 KB, patch)
2006-01-29 12:14 UTC, Tim-Philipp Müller
needs-work Details | Review
updated patch, using iterators (8.23 KB, patch)
2006-01-31 09:59 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2006-01-29 12:13:15 UTC
gst_bin_parse_bin_from_description() + unlinking unconnected ghost pads is code that is used and duplicated all over the place (gconf plugin, gnome-sound-recorder, sound-juicer, ...), so it seems it would make sense to put this as a utility function into the core.

gst_bin_find_unconnected_pad() is needed by gst_bin_parse_bin_from_description(), and seems a useful utility function to expose.

Patch to add both gst_parse_bin_from_description() and gst_bin_find_unconnected_pad() attached.
Comment 1 Tim-Philipp Müller 2006-01-29 12:14:24 UTC
Created attachment 58317 [details] [review]
adds gst_bin_parse_from_description() and gst_bin_find_unconnected_pad()
Comment 2 Tim-Philipp Müller 2006-01-29 12:15:32 UTC
erm, _linking_ unconnected ghost pads I meant of course.
Comment 3 Andy Wingo 2006-01-30 10:01:53 UTC
gst_bin_find_unconnected_pad has a locking order of parent -> child, which is not the case. You should iterators to avoid locking order problems.
Comment 4 Andy Wingo 2006-01-30 10:14:28 UTC
Well Wim claims that the locking order isn't really settled -- to me that means that to avoid problems one should if possible avoid taking more than one object lock at a time.
Comment 5 Tim-Philipp Müller 2006-01-31 09:59:02 UTC
Created attachment 58454 [details] [review]
updated patch, using iterators


Updated patch, this time using iterators. The gst_bin_find_unconnected_pad() function is primarily meant for newly-created elements that are in NULL state, so I'm not sure how important locking order is here in practice (we could just document it and worry about the other cases when needed).

So, can I use iterators like this? Or do I have to create a list of elements first and then traverse that list and iterate their pads?
Comment 6 Andy Wingo 2006-01-31 10:20:10 UTC
Well, it's probably best when adding new API to ensure there aren't any gotchas. Hence the iterators. I think your patch is correct, but it's tough to tell without a test. I prefer to use gst_iterator_fold, but that's me.

So for me, I would say go ahead and commit -- but a short test would be most excellent.
Comment 7 Tim-Philipp Müller 2006-02-01 16:53:41 UTC
Not entirely sure what exactly a test case would be supposed to test here - did you have anything specific in mind?

Just create a bin with ten identities in it and see if it successfully ghostpads the loose ends and prerolls when put in between a fakesrc and fakesink?

gconf*{sink|src} would be testing the functionality of these functions as well.
Comment 8 Tim-Philipp Müller 2006-02-02 09:54:42 UTC
Committed to CVS, keeping bug open for the test case.
Comment 9 Andy Wingo 2006-02-02 09:55:39 UTC
To me a test would take a data structure like this:

{ { 'identity ! fakesink', '/identity0/sink' },
  { 'identity ! fakesink identity ! fakesink0.',
    '/identity0/sink,/identity1/sink' },
  { NULL, NULL } }

Where the first member is a pipeline description, and the second is a
comma-separated list of pad names or so. The test would then run
parse_bin_from_description on the description, and then check that the ghost
pad targets are correct. I wouldn't bother with testing dataflow, that is
tested elsewhere.
Comment 10 Tim-Philipp Müller 2006-02-02 10:03:43 UTC
The current code only creates a max. of one ghost pad per direction though (which I've mentioned in the documentation). Not sure it should be doing more because of pad names and such - it's only a convenience function after all. More complex stuff the caller has to do himself IMHO.

Comment 11 Andy Wingo 2006-02-02 10:46:01 UTC
Sure then, makes it simpler -- but I would have basic tests for this one. Same structure, just one pad path.
Comment 12 Tim-Philipp Müller 2006-02-04 12:53:01 UTC
Tests added:

2006-02-04  Tim-Philipp Müller  <tim at centricular dot net>

        * tests/check/gst/gstutils.c: (test_parse_bin_from_description),
        (gst_utils_suite):
          Add some simple tests for gst_parse_bin_from_description() and
          gst_bin_find_unconnected_pad() (#329069).

Comment 13 Andy Wingo 2006-02-04 15:04:04 UTC
Excellent!