[gstreamer-bugs] [Bug 618853] add caps intersection iterators
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Tue May 25 04:29:14 PDT 2010
https://bugzilla.gnome.org/show_bug.cgi?id=618853
GStreamer | gstreamer (core) | unspecified
--- Comment #33 from Benjamin Otte (Company) <otte at gnome.org> 2010-05-25 11:29:11 UTC ---
So, I reviewed the code and am going to dump my thoughts about the API here, in
no particular order:
- The whole implementation is not threadsafe
I have no idea how we want to guarantee that. And it definitely has to be
threadsafe. I suspect we'll find the first problems the moment we use it for
gst_caps_from_string(). Anyway, I don't want to hunt races in my Fedora crash
reports, so it must be threadsafe.
I looked into lock-free arrays (for the structure array) yesterday and while
they do exist it's not clear to me that they provide a noticable benefit to
just putting a lock into every (lazy) caps. Locks are cheap these days after
all...
- Do we consider caps fields private?
They're marked as private, but would replacing the structures array with a
threadsafe variant be considered an API break?
- Why do we need a copy func?
It doesn't seem necessary to me as we can just do a copy with
gst_caps_new_lazy() that copies structures on-demand. Also, requiring the copy
code of _intersect() to compute the intersection again seems like overkill to
me.
- I don't like the EvaluateStructureFunc prototype
What I don't like about it is that it hands ou the caps and tells you to add
more structures to it using public API. This fails the moment the caps are not
writable anymore (i.e. when somebody reffed them).
I had imagined a prototype like:
GstStructure * (* GetNextStructureFunc) (gpointer user_data);
so that the functions never get handed the caps and the caps take care of
adding the structures internally.
I'm also not sure if an API like
GstCaps * (* GetNextStructuresFunc) (gpointer user_data);
would be better. That way the functions would be able to return more than one
structure at a time.
- The free func should be a GDestroyNotify
Same reason as above: Do not give the function access to the caps, but only to
the data pointer.
- The current intersection implementation uses private API.
This makes for a lousy test, as most of the users of the lazy API (think
plugins) will not have that luxury. I'd much prefer if the intersection
implementation only uses pu blic API.
- We might need API for modifying caps lazily
The transform-caps functions probably want API that makes it easy to transform
caps. When I'm looking at ffmpegcolorspace, what it does is take the
templatecaps, restrict width, height, framerate and aspect ratio to the input
and return the union of original and restricted template caps. Can we even do
this lazily?
- How far do we want to take this?
Do we want to transition so all caps are lazy? A place where this might be very
useful is probing code in sources and sinks which can take a while. If we were
able to delay a complete probe of all the weird formats, we might save quite a
bit of time on osssinks and v4lsrcs. But that probably requires not closing the
device before the caps is fully evaluated...
--
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.
More information about the Gstreamer-bugs
mailing list