[MPlayer-dev-eng] [RFC] Color management filter

[Diego Biurrun]

Please do not top-post.

On Tue, Aug 26, 2008 at 09:22:34AM -0400, Yue Shi Lai wrote:
>
> This new patch addresses all issues have been raised (i.e. including  
> interface to vo_gl). There is still few comment gaps and comments in  
> need of update (especially where the modifications have been applied),  
> which I will fill up, but I would appreciate any other issues you might  
> find.
>
> --- libmpcodecs/vf_cm.c	(revision 0)
> +++ libmpcodecs/vf_cm.c	(revision 0)
> @@ -0,0 +1,2627 @@
> +/*
> + * vf_cm - Color management filter

We know from the filename that this is vf_cm...

> + * Copyright (c) Yue Shi Lai 2008, licensed under GNU GPL v2 or v3

Sorry, this is not acceptable, we need contributions to be licensed
under the GPL version 2 or later.  So please use the standard license
header that you can find on many files, for example vidix/vidix.c.

> +#define SMPTE_R_X		0.630
> +#define SMPTE_R_Y		0.340
> +#define SMPTE_G_X		0.310
> +#define SMPTE_G_Y		0.595
> +#define SMPTE_B_X		0.155
> +#define SMPTE_B_Y		0.070

(Four) spaces are preferred as indentation for new files, please use
this.

> --- libmpcodecs/vf_cm.h	(revision 0)
> +++ libmpcodecs/vf_cm.h	(revision 0)
> @@ -0,0 +1,22 @@

missing license header

> +#ifndef LIBMPCODECS_VF_CM_H_
> +#define LIBMPCODECS_VF_CM_H_

MPLAYER_VF_CM_H

> --- configure	(revision 27482)
> +++ configure	(working copy)
> @@ -5614,7 +5619,23 @@
>  
> +echocheck "Little CMS"
> +if test "$_lcms" = auto ; then
> + _lcms=no
> + _def_lcms='#undef LCMS'

This line needs to be moved up, otherwise it will not be set when _lcms
is "no".

The patch is missing a manual page update.

Diego