<br><br><div class="gmail_quote">On Feb 9, 2008 2:43 PM, Ulrich von Zadow <<a href="mailto:uzadow@libavg.de">uzadow@libavg.de</a>> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi,<br><br>first of all, apologies for the multiple mails. The misconfiguration<br>that caused this has been fixed :-).<br><br>And second: Very good idea. Good on it's own, and good since it opens<br>the door for a few more enhancements. I'd probably commit it straight<br>
away if there wasn't any code duplication in the tests...<br><br>I spent some time trying to figure out if we can use this as a base to<br>make the setting of the Node member variables (i.e. most code in the<br>Node constructors) completely automatic. To do this, Arg would have to<br>
know where to place the value. There are two ways this could could be<br>implemented:</blockquote><div><br>You know, automatic variable binding crossed my mind as I was working on this too. In the end, I could not think of a clean solution, so I gave up on it.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br><br>1. Store C++ member pointers in each Arg. C++ member pointers are very<br>seldom used, so I'll explain ;-). They basically provide a way to access<br>
a member variable given an object. In the simplest case (single<br>inheritance), they just store an offset to the member variable. This:<br><br> AVGNode::*int pMember<br><br>is a pointer to a member of AVGNode, dereferenced by<br>
<br> pNode->*pMember<br><br>Anyway, since the type of a member pointer is dependent on the type of<br>the object and the type of the member variable, this is pretty useless<br>without some template trickery involving something like<br>
NodeDefinition<NodeT> and Arg<NodeT, ArgT>. To put Args and<br>NodeDefinitions into containers, there needs to be a non-templated base<br>class for them as well... since that is going to get convulted and drive<br>
compile times up, I came up with idea 2.<br></blockquote><div><br>Yeah, this could get messy... <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>2. Store the offset of the member variable directly, like so:<br><br> NodeDefinition(...)<br> .addArg("enablecrop", "true", (void*)&m_bEnableCrop-(void*)this)<br><br>This isn't really nice either (though we could probably hide the void *<br>
casting ugliness in a base class), and we need a this pointer in a<br>static function, so we'd probably need a temporary bogus object as well.<br>Still, maybe there are more people who will read and understand that<br>
kind of code than there are people willing to understand template trickery.<br></blockquote><div><br>Hmm, I don't really like the idea of having to use a bogus object instance for some reason... We may be able to clean it up though as you say.<br>
<br>We would also have to store the type of the argument so that it can be set converted correctly. Also, for strings, we might need multiple string types to handle std::string versus char* versus char[] (if there are even instances of this), which could get messy as well.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>On the other hand, both methods would cause the member variable setting<br>in the node constructors to mostly evaporate down to a single line,<br>
which, I think, would be incredibly cool. Instead of:<br><br>CameraNode::CameraNode(const ArgList& Args, Player * pPlayer)<br> : VideoBase(Args, pPlayer),<br> m_FrameNum(0)<br> {<br> string sDevice = Args.getStringArg ("device");<br>
double FrameRate = Args.getDoubleArg ("framerate");<br> string sSource = Args.getStringArg ("source");<br> int Width = Args.getIntArg ("capturewidth");<br> int Height = Args.getIntArg ("captureheight");<br>
string sPF = Args.getStringArg ("pixelformat");<br> }<br><br>we'd have:<br><br>CameraNode::CameraNode(const ArgList& Args, Player * pPlayer)<br> : VideoBase(Args, pPlayer),<br> m_FrameNum(0)<br>
{<br> ArgList.setMembers(this);<br> }</blockquote><div><br>That would be pretty nice. On the other hand, it is also somewhat nice to see the variables be set explicitly. Also, there are some instances where values obtained from the ArgList are not simply assigned to a variable, but instead used as an argument for another setter function. e.g. Words: "setAlignment(Args.getStringArg ("alignment"));". To get around this, we would have to add string parameters corresponding to these in addition to any other representation (in this case, the argument values are used to set Pango typed variables) and set the other variables after the strings have been set. Or, we could tell the ArgList not to set anything for certain args (pass -1 as the offset or something), and simply set them manually after the ArgList.setMembers(this) call. Both options - multiple representations of an argument and special casing some arguments - seem messy to me.<br>
<br>So in the end, I agree that this would be a pretty cool feature, but I am not sure that it can be implemented without sacrificing some code readability/design. Please prove me wrong if you see ways around these issues, however.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br><br>Just an idea. Tell me what you think.<br><br>More comments on the patch inline.<br>
<br>Cheers,<br><br> Uli<br><div><div></div><div class="Wj3C7c"><br>Nick Hebner wrote:<br>> Over the weekend, I took a little break from audio to work on some<br>> enhancements to dynamic node creation. With the included patch, nodes may be<br>
> created through Player::createNode(typeString, argsPythonDictionary). (e.g.<br>> newNode = Player.createNode("video", {"href":"video.mpg"}) ). To support<br>> this, I have reworked how arguments are passed to node constructors, and<br>
> created a generic registration mechanism for adding support for different<br>> types of nodes (also resulting in the ability to dynamically build/change<br>> the avg DTD). The changes are discussed below.<br>><br>
> NodeDefinition<br>> A NodeDefinition is basically represents all of the information formerly<br>> found in the avg DTD, that is information about what a type of node is. It<br>> is composed of a type string, all valid arguments and their default values,<br>
> all valid child nodes, and a function that may be used to build a node of<br>> this type (Node::buildNode() is a generic template NodeBuilder function that<br>> can be used in most cases). NodeDefinitions may be extended by subclasses,<br>
> making inheritance of arguments/child nodes simple. Each node type now<br>> implements a static getNodeDefinition() function which returns the<br>> information about the type.<br>><br>> NodeFactory<br>> NodeDefinitions are registered with a NodeFactory, which can then construct<br>
> nodes of the registered type. The NodeFactory handles validation of<br>> arguments passed to the create function based on the registered<br>> NodeDefinition of the type being created. Also, all unset args are replaced<br>
> with the default values defined in the NodeDefinition for the type so the<br>> node constructor may safely request any arg defined in its NodeDefinition.<br>> Additionally, the NodeFactory can dynamically build a DTD from the complete<br>
> set of NodeDefinitions currently registered with it. The DTD contains a<br>> single special entity called %anyNode; which contains a | separated list of<br>> all of the node types registered, and can be used for generic container<br>
> nodes (e.g. avg and div). The dynamic nature of the DTD is very flexible,<br>> and has been designed with the idea of node plugins in mind, however, it is<br>> now less visible then before. It would be nice to make the full DTD<br>
> available to the designer (should we add a Player::getDTD() that returns the<br>> current DTD string in python?).<br><br></div></div>I think the most important thing here is to have an executable that runs<br>as part of the make process and generates the dtd. (Granted, we don't<br>
have that now either.)</blockquote><div><br>The problem that I see with this is when we have node plugins that register with the player at runtime, the dtd will be different from the one generated at compile time. I think that we need to keep dtd generation a runtime process.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br><div class="Ih2E3d"><br>> Arg/ArgList<br>> An ArgList is the structure passed to node constructors. It may be build<br>
> from either an xmlNode or a python dictionary object. It provides a simple<br>> interface for getting arguments in various formats.<br>><br>> The dynamic node tests currently in place have been expanded to use both<br>
> interfaces, and all still pass.<br><br></div>[...]<br> > Index: /home/nick/workspace/libavg_trunk/src/player/ArgList.h<br> > ===================================================================<br>[...]<br> > +#include "BoostPython.h"<br>
<br>Just something to keep in mind for the future: I'd like to keep the<br>amount of python-specific code in src/player/ to a minimum to allow for<br>people who want to use libavg without python.</blockquote><div><br>
Should we start wrapping python stuff with configuration #ifdefs? <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br><br>[...]<br>
<br> > Index: /home/nick/workspace/libavg_trunk/src/test/Test.py<br> > ===================================================================<br> > --- /home/nick/workspace/libavg_trunk/src/test/Test.py (revision 2609)<br>
> +++ /home/nick/workspace/libavg_trunk/src/test/Test.py (working copy)<br> > @@ -913,8 +913,11 @@<br> > Player.stop))<br> ><br> > def testImgDynamics(self):<br><br>Why not:<br>
<br> def testImgDynamics(self, useXml):<br><br>?<br><font color="#888888"></font></blockquote><div><br>That would work. <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<font color="#888888"><br>--<br></font><div><div></div><div class="Wj3C7c"><br>Ulrich von Zadow | +49-172-7872715<br>Jabber: <a href="mailto:cocacoder@jabber.berlin.ccc.de">cocacoder@jabber.berlin.ccc.de</a><br>Skype: uzadow<br>
<br>_______________________________________________<br>libavg-devel mailing list<br><a href="mailto:libavg-devel@datenhain.de">libavg-devel@datenhain.de</a><br><a href="https://mail.datenhain.de/mailman/listinfo/libavg-devel" target="_blank">https://mail.datenhain.de/mailman/listinfo/libavg-devel</a><br>
</div></div></blockquote></div><br><br>Nick Hebner<br>