<p dir="ltr">Ola,</p>
<p dir="ltr">Le 15 nov. 2016 13:37, Pierre Ynard via vlc-devel <vlc-devel@videolan.org> a écrit :<br>
><br>
> > The playlist code really should not have to pass the long name. It<br>
> > shokld be inferred from the instantiated SD.<br>
><br>
> I agree. I'd like if it could be probed through pf_control after loading<br>
> the module, but it seems only the lua SD supports it so that would be a<br>
> lot of code to implement for me.</p>
<p dir="ltr">That won't work - catch-22.</p>
<p dir="ltr">The SD needs to report the long name before it starts creating items, so it cannot rely on a control request that would take place at arbitrary time in the future, if at all. Note that LibVLC does not care about the long name.</p>
<p dir="ltr">It can however store it as a const char * within the SD structure.</p>
<p dir="ltr">> There's still a lot more that can be done to fill the long name instead<br>
> of "?": using the SD chain, using the SD stem, using the "sd" parameter<br>
> in the config chain in particular for lua scripts. It's all better than<br>
> "?" and doesn't matter so much since it's for display.<br>
><br>
> > Perhaps the SD plahlist node creation could be deferred to the<br>
> > first invocation of the item_added callback? I don't really see any<br>
> > other way, except for letting interfaces take care (privately and<br>
> > independantly) of SD, which would break -S however.<br>
><br>
> I suppose that would be another path to pass it up from the SD, yes;<br>
> although it may never happen if the SD finds nothing.</p>
<p dir="ltr">Well if the SD finds nothing, not creating a dummy node seems like a feature more than a bug to me.</p>
<p dir="ltr">> But that makes me<br>
> think, why not simply let the SD create its playlist node in Open(), or<br>
> set the name of that node, or really just fill the long name in the SD<br>
> structure to pass it back?</p>
<p dir="ltr">1) SD has no notion of playlist node. There are no such things in LibVLC. Moreover, that sounds like a lot of code duplication.</p>
<p dir="ltr">2) I think that is what I suggested above too, yes.</p>
<p dir="ltr">> Any issue with that? Is it fine to create the<br>
> node, load the SD, then change the name of the existing node?<br>
><br>
> > This ugly hack is a compromise to avoid exposing the deficiency to the<br>
> > playlist code.<br>
><br>
> I'm not sure what this is trying to preserve here. The playlist API<br>
> call has the right prototype, granted, but is still tainted by the<br>
> expectation of a config chain.</p>
<p dir="ltr">The playlist does not know or care about the configuration chain syntax. And as you already noted, the current hack protects against API leakage.</p>
<p dir="ltr">> And the counterpart is that it pollutes<br>
> the SD probing API and a lot of things.</p>
<p dir="ltr">The probing API could be cleaned up nowadays. But it needs to provide a display name anyhow so not really.</p>
<p dir="ltr">>  The playlist code is certainly<br>
> exposed to that deficiency since it has to parse longname from the<br>
> config chain.</p>
<p dir="ltr">Only the SD/playlist binding.</p>
<p dir="ltr">> It even makes sense to me that the playlist API call<br>
> playlist_ServicesDiscoveryAdd() would take as an argument the display<br>
> name of the playlist item to create.</p>
<p dir="ltr">I don't see why. The playlist can't guess the name, especially not if -S is used.</p>
<p dir="ltr">> > I think this is replacing a backward approach with another backward<br>
> > approach though. In my opinion, the long name should be passed from<br>
> > the instantiated module.<br>
><br>
> It still seems like a step in the right direction, but I hope I've<br>
> offered an even better idea.</p>
<p dir="ltr">Adding API leakage is not what I would call a step in the right direction.</p>
<p dir="ltr">> > Also the Is*Loaded test is busted and IMHO unfixable. Interfaces need<br>
> > to stop using it.<br>
><br>
> The proposed change would go some way towards fixing it by removing the<br>
> worst of the SD name string.</p>
<p dir="ltr">Yes... but it remains unfixable.</p>
<p dir="ltr">> I'm also skeptical why a playlist call<br>
> would take a string to match and remove an SD from the playlist, rather<br>
> than an item ID.</p>
<p dir="ltr">Me too.</p>
<p dir="ltr">> And I'm skeptical about the concept of uniqueness for<br>
> SD modules, several instances of the same SD could run with different<br>
> configs, that's certainly what the lua SD does to a different extent.</p>
<p dir="ltr">Precisely.</p>
<p dir="ltr">> Since SD are managed by the playlist, maybe I'd rather they were<br>
> accessed through their playlist node.</p>
<p dir="ltr">I think I'd rather SD were managed outside the playlist altogether. But that requires changes to interfaces that I am not volunteering myself for.</p>
<p dir="ltr">> I'm mildly disturbed by how deleting an SD playlist node doesn't stop<br>
> the SD. I wonder if that can cause issues.<br>
><br>
> By the way, unloading an SD correctly removes its playlist node in 2.2,<br>
> but it doesn't in 3.0, so it seems something got broken somewhere.<br>
><br>
> -- <br>
> Pierre Ynard<br>
> "Une âme dans un corps, c'est comme un dessin sur une feuille de papier."<br>
> _______________________________________________<br>
> vlc-devel mailing list<br>
> To unsubscribe or modify your subscription options:<br>
> https://mailman.videolan.org/listinfo/vlc-devel<br></p>