Library design opinions...

[rep_movsd]

Hi

I have developed a simple free library that is used for creating and
managing Directshow filter graphs, that I (and possibly several
others ) have been using for more than a year now...

Recently I thought of rewriting it with a more "intuitive" usage style
( mostly to satisfy my desire to play with template metaprogramming ),
and gain a bit of static error checking ( and making the user code
look like magic )

Heres some equivalent code for the old and new libraries ( its a bit
specific to the dshow framework, but it is fairly obvious what the
code does, also all the includes etc have been omitted )

Old library usage :
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////

void BuildGraph(CGraph &g)
{
g.Connect(L"SOURCE", L"TEE");
g.Connect(L"TEE", L"RENDERER");
g.Connect(L"TEE", L"RENDERER2", 1, 0); // connect 2nd output pin
of tee to renderer2's first pin
}

void BuildGraphBad(CGraph &g)
{
g.Connect(L"SOURCE", L"TEE");
g.Connect(L"TEE", L"RENDERERER"); // Runtime error "filter
not found "
g.Connect(L"TEE", L"RENDERER2", 1, 0);
}

int main(int argc, char **argv)
{
CGraph g;
g.AddSourceFilter(argv[1], L"SOURCE"); // Create a source
filter based on filename
g.AddFilter(CLSID_InfTee, L"TEE"); // Create a Tee
g.AddFilter(CLSID_VideoRenderer, L"RENDERER"); // Create
renderers
g.AddFilter(CLSID_VideoRenderer, L"RENDERER2");
BuildGraph(g);
BuildGraphBad(g);
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////


New library usage:
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////

DECLARE_FILTER(SOURCE);
DECLARE_FILTER(TEE);
DECLARE_FILTER(RENDERER);
DECLARE_FILTER(RENDERER2);

void BuildGraph()
{
FILTER(SOURCE);
FILTER(TEE);
FILTER(RENDERER);
FILTER(RENDERER2);

SOURCE >> TEE >> RENDERER; // Chain the filters
TEE >> RENDERER2; // Add an extra renderer
}

void BuildGraphBad()
{
FILTER(SOURCE);
FILTER(TEE);
FILTER(RENDERERER); // Misspelling - compile error

SOURCE >> TEE >> RENDERER; // Compile error
}

int main(int argc, char **argv)
{
CGraf g;
FILTER(SOURCE);
FILTER(TEE);
FILTER(RENDERER);
FILTER(RENDERER2);

SOURCE = argv[1]; // Create a source filter
based on filename
TEE = CLSID_InfTee; // Create filters based on
CLSIDs
RENDERER = CLSID_VideoRenderer;
RENDERER2 = CLSID_VideoRenderer;

g += SOURCE, TEE, RENDERER, RENDERER2; // FIlters get added to the
graph
BuildGraph();
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////


Some of the advantages of the new style are :
1) FIlter names are identifiers, so u get intellisense and compile
time checking
2) A filter with a given name always refers to the same object, so
there is no need to store a bunch of pointers to filter objects and
pass them around various functions ( or worse yet keep them global as
most directshow code does )
3) The operator overloading makes the code very readable,

The only disadvantage I see is that the implementation is complex and
I am not 100% confident that my code is perfect.

I was wondering to overload the unary minus operator to remove
filters from the graph and the divide by operator to disconnect
filters. I would also add some manipulator stuff ( like in iostreams)
to control the way filters are
connected with >> e.g.

// Remove renderer2 from graph
g -= RENDERER2;

// Connect the first audio pin of the source to the renderer
SOURCE >> pinType(MEDIATYPE_Audio) >> AUDIORENDER;

// Disconnect any connection betwen these 2 filters
SOURCE / AUDIORENDER;

Any opinions, suggestions?

Vivek

 

[rep_movsd]

Notes:

[1] That means, the all uppercase. Are those really macros? If so, get rid of them.

FILTER and DECLARE_FILTER are macros...
The rest are identifiers....

 

[Boris]

[...]Recently I thought of rewriting it with a more "intuitive" usage
style
( mostly to satisfy my desire to play with template metaprogramming ),
and gain a bit of static error checking ( and making the user code
look like magic )

Your new code doesn't look like magic to me - it looks more like it has
been deliberately obfuscated. Once you start using the library in another
use case where you don't do exactly what is explained in the documentation
you have to fiddle around with all the macros trying to understand what's
going on there in detail. It's more useful to know what is a class, a
function, an object etc. as every C++ programmer understands then
immediately what they can do.

Boris

[...]

 

[rep_movsd]

Your new code doesn't look like magic to me - it looks more like it has  
been deliberately obfuscated. Once you start using the library in another  
use case where you don't do exactly what is explained in the documentation  
you have to fiddle around with all the macros trying to understand what's  
going on there in detail. It's more useful to know what is a class, a  
function, an object etc. as every C++ programmer understands then  
immediately what they can do.

Boris

Yes, I agree that the declaration macros have quite complex
implementations and are opaque, But what about the use of overloaded
operators to make the connections etc?
Does that look bad too?

Vivek

 

[Boris]

[...]Yes, I agree that the declaration macros have quite complex
implementations and are opaque, But what about the use of overloaded
operators to make the connections etc?
Does that look bad too?

I try either to reuse an idea from the standard (for example everybody has
an idea what operator<< and operator>> do because of streams) or make sure
that the usage of operators feels natural (which might be more difficult
as what is natural for me might not be natural for someone else). Your
library seems to use operator>> to chain filters and operator+= to add
filters? I probably wouldn't use overloaded operators here: operator>>
makes me think that there is a data flow while in fact it's just a
chaining of filters (if it was a lambda function it would make more sense
maybe . And when operator+= is overloaded you wonder if and how
operator+ fits into the schema.

Boris

 

[Looney]

Hi

I have developed a simple free library that is used for creating and
managing Directshow filter graphs, that I (and possibly several
others ) have been using for more than a year now...

Recently I thought of rewriting it with a more "intuitive" usage style
( mostly to satisfy my desire to play with template metaprogramming ),
and gain a bit of static error checking ( and making the user code
look like magic )

Heres some equivalent code for the old and new libraries ( its a bit
specific to the dshow framework, but it is fairly obvious what the
code does, also all the includes etc have been omitted )

Old library usage :
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////

void BuildGraph(CGraph &g)
{
g.Connect(L"SOURCE", L"TEE");
g.Connect(L"TEE", L"RENDERER");
g.Connect(L"TEE", L"RENDERER2", 1, 0); // connect 2nd output pin
of tee to renderer2's first pin

}

void BuildGraphBad(CGraph &g)
{
g.Connect(L"SOURCE", L"TEE");
g.Connect(L"TEE", L"RENDERERER"); // Runtime error "filter
not found "
g.Connect(L"TEE", L"RENDERER2", 1, 0);

}

int main(int argc, char **argv)
{
CGraph g;
g.AddSourceFilter(argv[1], L"SOURCE"); // Create a source
filter based on filename
g.AddFilter(CLSID_InfTee, L"TEE"); // Create a Tee
g.AddFilter(CLSID_VideoRenderer, L"RENDERER"); // Create
renderers
g.AddFilter(CLSID_VideoRenderer, L"RENDERER2");
BuildGraph(g);
BuildGraphBad(g);

};

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////

New library usage:
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////

DECLARE_FILTER(SOURCE);
DECLARE_FILTER(TEE);
DECLARE_FILTER(RENDERER);
DECLARE_FILTER(RENDERER2);

void BuildGraph()
{
FILTER(SOURCE);
FILTER(TEE);
FILTER(RENDERER);
FILTER(RENDERER2);

SOURCE >> TEE >> RENDERER; // Chain the filters
TEE >> RENDERER2; // Add an extra renderer

}

void BuildGraphBad()
{
FILTER(SOURCE);
FILTER(TEE);
FILTER(RENDERERER); // Misspelling - compile error

SOURCE >> TEE >> RENDERER; // Compile error

}

int main(int argc, char **argv)
{
CGraf g;
FILTER(SOURCE);
FILTER(TEE);
FILTER(RENDERER);
FILTER(RENDERER2);

SOURCE = argv[1]; // Create a source filter
based on filename
TEE = CLSID_InfTee; // Create filters based on
CLSIDs
RENDERER = CLSID_VideoRenderer;
RENDERER2 = CLSID_VideoRenderer;

g += SOURCE, TEE, RENDERER, RENDERER2; // FIlters get added to the
graph
BuildGraph();

};

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Some of the advantages of the new style are :
1) FIlter names are identifiers, so u get intellisense and compile
time checking
2) A filter with a given name always refers to the same object, so
there is no need to store a bunch of pointers to filter objects and
pass them around various functions ( or worse yet keep them global as
most directshow code does )
3) The operator overloading makes the code very readable,

The only disadvantage I see is that the implementation is complex and
I am not 100% confident that my code is perfect.

I was wondering to overload the unary minus operator to remove
filters from the graph and the divide by operator to disconnect
filters. I would also add some manipulator stuff ( like in iostreams)
to control the way filters are
connected with >> e.g.

// Remove renderer2 from graph
g -= RENDERER2;

// Connect the first audio pin of the source to the renderer
SOURCE >> pinType(MEDIATYPE_Audio) >> AUDIORENDER;

// Disconnect any connection betwen these 2 filters
SOURCE / AUDIORENDER;

Any opinions, suggestions?

Vivek

Hi Vivek,
I would advice that only overload operators which your library's users
would find intuitive, also while following the normal semantics of
those operators. Remember the goal of your rewrite is to make a more
user friendly library, your library's users all ready know that you
can write nifty code, so do not just overload operators because u can.

Also I am a big fan of c++ Meta programming, though i do not
necessarily see that you can well use a lot of it for the task at
hand . You might be able to create cool types which know at compile
time what filters to add, but u still need to make runtime calls to
Direct show library to actually setup the filter graph. Meta
programming usually is more usefull when all or majority of the info
is available at compile time and you can get compiler to evaluate
expressions, inline calls and un-roll loops etc to generate faster
code. In your case say even if you had a types to represent a filter
at compile time, you still actually do need to setup the filter graph
at runtime with DS library, also consider what happens if the user of
your library wants to iterate over all the available filter types at
runtime and decide which ones to actually add to the filter graph. A
Meta programming solution is set out in stone and would fall short to
meet the user's needs.

Please do not be offended as i do not intend to offend, but i too like
others prefer the first code snippet over the second as upon a quick
glance, it's much clearer to me what the code is doing. The second
code snippet just seems to create unnecessary confusion.
My 2 cents

 

[rep_movsd]

I try either to reuse an idea from the standard (for example everybody has  
an idea what operator<< and operator>> do because of streams) or make sure  
that the usage of operators feels natural (which might be more difficult  
as what is natural for me might not be natural for someone else). Your  
library seems to use operator>> to chain filters and operator+= to add  
filters? I probably wouldn't use overloaded operators here: operator>>  
makes me think that there is a data flow while in fact it's just a  
chaining of filters (if it was a lambda function it would make more sense  
maybe . And when operator+= is overloaded you wonder if and how  
operator+ fits into the schema.

Boris

Well in dshow architecture, a graph is a bag of filters and the
connections will have data flow through them ( in the direction of the
'>>' ). Filters are more like digital ICs with input and output pins
( the term filter isnt so fitting in fact )

Graphs are always directed, so i thought using >> to hook up filters
together would be intuitive.

The fact that you saw it as a "data flow" is actually a good thing

Yes, now that you mention it, the meaning of + is vague if += adds
filters to the graph.

Vivek