[vlc-devel] [PATCH 1/8] stream_extractor: split joint capability into two
filip at atch.se
Fri Feb 17 20:39:30 CET 2017
On 2017-02-17 20:13, Rémi Denis-Courmont wrote:
> Le torstaina 16. helmikuuta 2017, 22.44.11 EET Filip Roséen a écrit :
> > These changes splits the functionality for a stream-extractor into two
> > different objects, one being a stream_extractor_t (used to extract
> > data for an entity within a stream based on an identifier), and the
> > other, stream_directory_t, is to list entities within a stream.
> > Summary of changes:
> > - update and refine documentation belonging to stream-extractors
> > - stream_extractor_t is now stream_extractor_t and stream_directory_t
> To me, that looks better than the union.
> Although AFAICT, stream_directory_t is a "subset" of stream_t, and
> stream_extractor_t would also be a subset except for the identifier. Then you
> don´t even need new types: you can just define a new pf_activate callback
> prototype to carry the identifier.
Yes, and that approach also crossed my mind before implementing it the
first way, and now the second way; though I considered changes to
`pf_activate` to be a more intrusive than the path taken.
I also decided to use a separate type the first time around to make it
clear that `vlc_stream_directory_CreateMRL` (formerly
`vlc_stream_extractor_CreateMRL`) shall only be called with a
`vlc_stream_directory_t` (`stream_extractor_t` when I proposed a
Hopefully such decoupling allows for easier reasoning when writing a
new module, at least that is the idea. I remember when first starting
out with the codebase and the initial (although somewhat minor)
confusion to what a `stream_t` really was (until it was a somewhat
uniform handle to everything that had to do with a vlc-stream).
A uniform handle is both a blessing and a curse, and I think
decoupling allows for easier reasoning from a maintainer point of
view (given that you still always read from a `stream_t`, but what
"generates" content is hidden through *type-erasure*)
Another argument for not making `stream_t` the "internal" type for the
modules with "stream_directory" and "stream_extractor" functionality
is that many of the members in `stream_t` are not relevant to such
A module with the mentioned capability should not care about the
following members at all, so introducing a separate type allows for
easier reasoning to what a module-maintainer should care about (and
It would , hopefully, prevent misuse of such members to provide
"functionality" that should be elsewhere in the chain.
For a `stream_directory_t` the only relevant callback is `pf_readdir`,
so having a distinguished type that only exposes that *callback* is
(in my opinion) a plus.
> That being written, I have not actually checked if that would enable any
> further simplification on the core side. I just hope it would allow treating
> extractors and directories as regular streams for purpose other than
It would allow for some simplification, though in my opinion somewhat
minor given the required changes (and mental processing of how modules
are initialized). I am not sure that I (personally) would like
extractors and directories to be "regular streams" given what was
written in previous sections.
I welcome more opinions on the matter, but I have my fingers crossed
for the currently proposed design to be accepted.
Thank you for your valuable feedback, \
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel