[vlc-devel] [PATCH v3 1/3] Introduce media source and media tree API
remi at remlab.net
Fri Jun 29 20:41:16 CEST 2018
Le vendredi 29 juin 2018, 16:57:28 EEST Romain Vimont a écrit :
> Does it look ok to you?
Yes but it is not about me, it is about Doxygen. So check the result yourself.
> > While const is semantically correct, won't it prevent some "legitimate"
> > use of the objects from the callbacks?
> I don't think so. The caller should never write to these nodes directly.
Of course, but sometimes you have things like muteces or whatever that don't
cope with const very well, even for "reading".
> > > +} vlc_media_tree_callbacks_t;
> > I would argue that pf_ is utmostly redundant in an ops table that by
> > definition contains function pointers :/
> Yes, but many of these hungarian notation prefixes are useless/redundant
Redundant is polite. They are actively harmful, alright.
Not only they waste precious columns in the source editor, but they have
dumbed down the C type system in VLC (look at 15 years old sources and you'll
And then, you get style horrors like this:
...two variables with effectively the same name.
> > Shouldn't that be merged with callback registration (what you call
> > connection, IIUC)?
> I don't think so, reference counting and listeners are different things.
> Of course, adding a listener implies we should hold the object, but not
> necessarily the other way around.
> > Is there a use case for referencing a tree without registering?
> Currently, maybe not, but in theory it may happen (e.g. one-shot
> search in the tree).
I dunno. You should hold the lock, not a reference, when looking things up.
Otherwise you will get interesting problems when a tree is referenced by some
GUI, but no longer by the core, and therefore no longer "findable".
> > Maybe expose the structure layout to registrant and make this fail-free?
> It is expected to be an opaque pointer.
But registering callbacks is not generally expected to fail, and it's hardly
ever handled correctly.
> And it would only move the problem to the caller.
Why? The caller can just embed the structure in its internal state.
More information about the vlc-devel