[MPlayer-cvslog] r23062 - trunk/vidix

[Reimar Döffinger]

Hello,
On Sun, Apr 22, 2007 at 03:25:51PM +0200, ben wrote:
> -static int __verbose = 0;
> -#ifdef RADEON
> -static int is_shift_required = 0;
> -#endif
> +#define RADEON_ASSERT(msg) printf(RADEON_MSG"################# FATAL:"msg);
>  
> +#define VERBOSE_LEVEL 0
> +static int __verbose = 0;

This was there before, but aren't names starting with __ reserved for
use by the compiler?

> -#define INREG8(addr)		GETREG(uint8_t,(uint8_t*)(radeon_mmio_base),addr)
> -#define OUTREG8(addr,val)	SETREG(uint8_t,(uint8_t*)(radeon_mmio_base),addr,val)
> -
> +#define INREG8(addr)		GETREG(uint8_t,(uint8_t *)(radeon_mmio_base),addr)
> +#define OUTREG8(addr,val)	SETREG(uint8_t,(uint8_t *)(radeon_mmio_base),addr,val)
>  static inline uint32_t INREG (uint32_t addr) {
> -	uint32_t tmp = GETREG(uint32_t,(uint8_t*)(radeon_mmio_base),addr);
> -	return le2me_32(tmp);
> +    uint32_t tmp = GETREG(uint32_t,(uint8_t *)(radeon_mmio_base),addr);
> +    return le2me_32(tmp);
>  }
> -//#define OUTREG(addr,val)	SETREG(uint32_t,(uint8_t*)(radeon_mmio_base),addr,val)
> -#define OUTREG(addr,val)	SETREG(uint32_t,(uint8_t*)(radeon_mmio_base),addr,le2me_32(val))
> -#define OUTREGP(addr,val,mask)  					\
> +#define OUTREG(addr,val)	SETREG(uint32_t,(uint8_t *)(radeon_mmio_base),addr,le2me_32(val))
> +#define OUTREGP(addr,val,mask)						\

That you don't want to spend much time on cleaning up the patch I can
understand, but there us no excuse that you didn't take the few minutes
time to at least run "diff -duwbB" over it to clean out the biggest cosmetics
like I suggested (or at least speak up against my suggestion)!
Size difference is not that big (128890 vs. 121051 bytes) but since a
lot is simply added or removed code the readability difference is
visible.
You can do
svn di --diff-cmd diff -x '-duwbB' -r23061:23062
to get it.

>  #ifdef RAGE128
> -  OUTREG(OV0_COLOUR_CNTL,0x00101000UL); /* Default brightness and saturation for Rage128 */
> +  besr.saturation = 0x0F;
> +  besr.brightness = 0;
> +  OUTREG(OV0_COLOUR_CNTL,0x000F0F00UL); /* Default brihgtness and saturation for Rage128 */

The original way of writing brightness was more correct...

Greetings,
Reimar Döffinger