[MPlayer-cvslog] r30870 - in trunk/loader/dshow: graph.c graph.h

[Diego Biurrun]

On Tue, Mar 09, 2010 at 01:39:01PM +0100, Steinar H. Gunderson wrote:
> On Tue, Mar 09, 2010 at 01:25:21PM +0100, Diego Biurrun wrote:
> >> +static void FilterGraph_Destroy(FilterGraph* This)
> >> +{
> >> +    Debug printf("FilterGraph_Destroy(%p) called  (%d, %d)\n", This, This->refcount, GraphKeeper);
> > Isn't there a dbg_printf in loader/ somewhere?
> 
> It's not in use in loader/dshow/ at least, but I could include loader/debug.h
> and use it?

I guess.  Duplicate functionality should be avoided.

> >> +    if (!This->vt)
> >> +    {
> > K&R style for new files please, i.e. keep the { on the same line...
> 
> FWIW, this is only technically a new file; it's based on allocator.c.
> But fixed.

Then it should be created with 'svn cp'.

> > The = could be vertically aligned.
> 
> I don't really see the point, but if that is indeed the style, so be it.
> Changed.

The point is improved readability.

> >> --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> >> +++ trunk/loader/dshow/graph.h	Tue Mar  9 13:00:30 2010	(r30870)
> >> @@ -0,0 +1,38 @@
> >> +#ifndef MPLAYER_GRAPH_H
> >> +#define MPLAYER_GRAPH_H
> > This is missing the standard license header, same for graph.c.  If it
> > was written for MPlayer, it should carry MPlayer's standard license
> > header.
> 
> Written for MPlayer, but based heavily upon allocator.c. You decide.

I suggest you add our license header and mention the ancestry at the top
of the file.

Diego