great to see your plugin in the addonpack finally. here some suggestions concering naming:
how about naming it FileStream (VLC) to be clearly a counterpart to FileStream (DShow9)
you’re setting version to ‘0.5’ which is the version of the vlc-lib you’re wrapping, right? if so, thats not supposed to be there. versions in nodenames are there to distinguish two else identically named nodes. when 0.6 of that vlc lib comes out you’d ideally just rebuild the node with that and not bother users about it. still you could of course mention the version in the plugins help-string and/or in the helppatch…which brings me to the next:
please add a simple helppatch. with links to websites discussing the option syntax…
can you make the Filename of actual subtype ‘filename’ and add an extra string-input for those options? aren’t they optional? and would be nice to be able to click filename-pin and get a browser to choose a file…
pinorder: would also be great to have inputs and outputs in exactly the same order and naming (where they correspond) as those on the FileStream (DShow9) node. also _ Texture Out_ would then be the leftmost output.
Forced Width/Height could be set to visible-in-inspektor-only since most likely you wouldn’t want to change those i’d argue.
i’d at least add the term ‘video’ as a tag for the node.
Width and Height outputs: are they actually the textures width/height? if so i’d remove those since there is the Info (EX9.Texture) to get those. Texture Aspect Ratio can easily be calculated from there. not sure about Pixel Aspect Ratio if that is something else…leave it there.
0.5 is the version of the plugin. But I will set it to 0.1 when renaming to FileStream…
Where do I put it?
Yes, in a way, but it would cause trouble if you want to open a strean (http or rtsp or webcam or dvb), so in that sense I don’t think it’s a good idea. My trick for now is to quickly create a filename node if I want a filebrowser.
Rearranged some inputs.
Since it’s based on DXTextureOutPluginBase, I don’t know how I could put the TextureOut pin on the left…
You’re right, I’ll hide these.
I’ll send you another pull request, so you can see the changes. Let me know if there’s any remaining problems.
Filename should be rightmost and Next Filename (with space between ‘next’ and ‘filename’) the first left to it
regarding the version: as mentioned above it is not supposed to tell us the version of the node. no other node has such a version number. versions are only supposed to be used to distinguish two likewise named nodes in the same category. since there is no other node FileStream (VLC) no version should be specified.
the helppatch goes where all the others are:
concering filestream/options: do i get it right that you’re arguing that opening a stream would be more difficult with subtype-filename set? if so i’d argue: it is always important to find the best default behavior. and in that case i’d argue that playing back a file from disk is more likely to happen than playing back a net-stream. so playback from disk should be the quickest thing to do.
playback of a stream can still easily be done by popping a string iobox on the input and doubleclicking that to enter the string manually. (note to self: make ctrl+click on filename-pins not open a file-dialog).
concering output order: there is a ‘Priority’ attribute for pins: set it to maxint to get the pin leftmost. we should do that for the texture-template…
I wasn’t too shure if the Looping pins behaved similarly enough to justify their being named similarly.
I was wondering (but didn’t try it before), that because the pins were called ‘Start Time’ and ‘End Time’ that they also might have an effect when Looping was not enabled. In that sense, I think that Loop Start makes it more clear that it will only have an effect when Looping is enabled.
So maybe ‘Loop Start/End Time’ would be the better choice (a bit long maybe).
And another one about the existing namings: why not use Seek Time, if you also use Start Time? ‘Position’ seems to suggest this uses another unity (for example 0.0 being start and 1.0 being the end).
Apart from that:
help patch was already done
Add filename behaviour to Filename input pins, and reorder them (although ‘next’ before ‘current’ does feel a bit strange too :) )