<!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 Pierre,</p>
<p>On 2016-11-06 22:11, Pierre Ynard wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> The previous implementations usage of input_item_GetName resulted in
unexpected data in the filename attribute</code></pre>
</blockquote>
<pre><code> I can agree so far.</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> (given that input_item_GetName will first query vlc_meta_Title, and if
present return that).</code></pre>
</blockquote>
<pre><code> I don't follow, that would be input_item_GetTitleFbName() instead.</code></pre>
</blockquote>
<p>The usage of <code>vlc_meta_Title</code> is perhaps confusing (and a little misleading since I should have mentined the name entity on its own too), but I will try to explain the relevant code-paths.</p>
<p><code>item->psz_name</code>, the value yield by <code>input_item_GetName</code> is the name set for an item via either <code>input_item_SetName</code> or the relevant initializer passed to <code>input_item_NewExt</code>.</p>
<p><code>input_item_SetName</code> is however called from within <code>src/input/es_out.c:EsOutMeta</code>, with the value of <code>vlc_meta_Title</code> (if that is available); giving the semantics previously explained.</p>
<p>In other words; when you start playing an item, the <em>name</em> will update to that of <em>Title</em> if such metadata is available.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> The only other relationship I'm aware of between name field and title
metadata is that the lua binding to add new items copies from name to
title if the title is missing.</code></pre>
</blockquote>
<p>See previous comment.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> These changes extracts the filename for a given item, including
support for trailing slashes (so that we do not get an empty filename
for a path such as file:///media/).</code></pre>
</blockquote>
<pre><code> That's a change in the API that can break current users. Have you
analyzed the potential users relying on that behavior, in tree and out
of tree? I see at least two meta scripts in tree (both seemingly broken
for years).</code></pre>
</blockquote>
<p>Yes, I have analyzed the usage and both <code>02_frenchtv.lua</code> and <code>filename.lua</code> actually expects the filename to be within the field.</p>
<p>Given that these two expects the filename to be there, as well as the field-name suggesting that it is populated by the filename; I did not want to remove the field (since, as you say, it is a breaking change).</p>
<p>I also took a look at public repositories at <em>github</em>, only finding usages where the actual <em>filename</em> is expected (not a conditional merge).</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> I'm not sure what's your real problem here. I agree that this field is
confusing, "filename" is misleading and it doesn't necessary belong
within metadata. The name field is exposed in the CLI through the
misnamed get_title command. The URI is exposed through the lua bindings
through item:uri(), and the "file name" part of it can be easily
obtained from it; alhough it's not available in the CLI.</code></pre>
</blockquote>
<p>Somewhat, yes; though it suffers from the same problem as fixed by <code>24d0d1e</code>.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> I'm all for 1/ deprecating that field, 2/ improving known users of that
weird field to use something else like item:uri() or item:name() and 3/
improving CLI commands to display more, relevant information, but moving
away from that metadata abuse. Aliasing get_title to get_name, adding a
get_uri, and adding a section to info or a new command summarizing lua
item fields, all sound like good ideas to me.</code></pre>
</blockquote>
<p>Agreed, though I was looking for an initial fix while we await a bigger refactoration of the relevant paths.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> + if( psz_filename && psz_filename[1] == '\0' )
+ {
+ psz_filename[0] = '\0';
+ psz_filename = strrchr( psz_uri, '/' );
+ }</code></pre>
</blockquote>
<pre><code> In any case this could use a comment because it took me way too long to
understand what it's for.</code></pre>
</blockquote>
<p>I was actually about to introduce a helper for <em>filename</em>-extraction given that we have several cases in the codebase where this is done, but decided not to.</p>
<p>The lack of comment is probably due to me writing such code from the back of my head. C, the language where you reinvent the wheel (or bring an old one) in every new project. <code>;-)</code></p>
</body>
</html>