<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi Rémi,</p>
<p>On 2017-02-17 20:13, Rémi Denis-Courmont wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Le torstaina 16. helmikuuta 2017, 22.44.11 EET Filip Roséen a écrit :</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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</code></pre>
</blockquote>
<pre><code> 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.</code></pre>
</blockquote>
<p>Yes, and that approach also crossed my mind before implementing it the first way, and now the second way; though I considered changes to <code>pf_activate</code> to be a more intrusive than the path taken.</p>
<p>I also decided to use a separate type the first time around to make it clear that <code>vlc_stream_directory_CreateMRL</code> (formerly <code>vlc_stream_extractor_CreateMRL</code>) shall only be called with a <code>vlc_stream_directory_t</code> (<code>stream_extractor_t</code> when I proposed a single type).</p>
<p>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 <code>stream_t</code> really was (until it was a somewhat uniform handle to everything that had to do with a vlc-stream).</p>
<p>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 <code>stream_t</code>, but what “generates” content is hidden through <em>type-erasure</em>)</p>
<h3 id="irrelevant-data-members">Irrelevant data-members</h3>
<p>Another argument for not making <code>stream_t</code> the “internal” type for the modules with “stream_directory” and “stream_extractor” functionality is that many of the members in <code>stream_t</code> are not relevant to such modules.</p>
<p>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 access).</p>
<ul>
<li><code>stream_t::psz_name</code></li>
<li><code>stream_t::psz_url</code></li>
<li><code>stream_t::psz_location</code></li>
<li><code>stream_t::psz_filepath</code></li>
<li><code>stream_t::p_input</code></li>
</ul>
<p>It would , hopefully, prevent misuse of such members to provide “functionality” that should be elsewhere in the chain.</p>
<p>For a <code>stream_directory_t</code> the only relevant callback is <code>pf_readdir</code>, so having a distinguished type that only exposes that <em>callback</em> is (in my opinion) a plus.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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
instantation.</code></pre>
</blockquote>
<p>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.</p>
<p>I welcome more opinions on the matter, but I have my fingers crossed for the currently proposed design to be accepted.</p>
<p>Thank you for your valuable feedback,<br />
Filip Roséen</p>
</body>
</html>