[PATCH] Stop accessing the attribute array using the attribute name enum.

[Thomas Klausner]

The whole SMI_VideoAttributes array looks quite strange to me.

static XF86AttributeRec SMI_VideoAttributes[2] = {
    {XvSettable | XvGettable,        0,           255, XV_BRIGHTNESS_NAME},
    {XvSettable | XvGettable, 0x000000,      0xFFFFFF, XV_COLORKEY_NAME},
};

but it is accessed using these defines as indices:

#define XV_ENCODING             0
#define XV_BRIGHTNESS           1
#define XV_CAPTURE_BRIGHTNESS   2
#define XV_CONTRAST             3
#define XV_SATURATION           4
#define XV_HUE                  5
#define XV_COLORKEY             6
#define XV_INTERLACED           7

However, all of this code including the CLAMP has been this way since

commit 78af703cb019a60cc93843efcd4889ccb15abd13
Author: Kaleb Keithley <kaleb at freedesktop.org>
Date:   Fri Nov 14 16:48:55 2003 +0000

    Initial revision


I agree that this CLAMP call should go, perhaps just ifdefed out with
a comment. Not sure what the tradition for this is with xorg.

Cheers,
 Thomas

On Wed, Sep 30, 2015 at 05:02:43PM +1300, Robert Ancell wrote:
> The array has only two elements but the clamping code was assuming it contained
> all the elements in order. This means no clamping is now done but at least it
> wont read off the end of the array.
> 
> Signed-off-by: Robert Ancell <robert.ancell at canonical.com>
> ---
>  src/smi_video.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/src/smi_video.c b/src/smi_video.c
> index 27df68d..a13668e 100644
> --- a/src/smi_video.c
> +++ b/src/smi_video.c
> @@ -681,10 +681,6 @@ SetAttr(ScrnInfoPtr pScrn, int i, int value)
>  
>      if (i < XV_ENCODING || i > XV_HUE)
>  	return BadMatch;
> -    
> -    /* clamps value to attribute range */
> -    value = CLAMP(value, SMI_VideoAttributes[i].min_value,
> -		  SMI_VideoAttributes[i].max_value);
>  
>      if (i == XV_BRIGHTNESS) {
>  	int my_value = (value <= 128? value + 128 : value - 128);
> -- 
> 2.5.0
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel