[MPlayer-dev-eng] [PATCH] add 8 bit Bayer format and filter

[Dominik 'Rathann' Mierzejewski]

On Monday, 03 March 2008 at 12:06, Guennadi Liakhovetski wrote:
> Hi all
> 
> This patch implements support for a new image format - 8 bit Bayer (BA81 / 
> BYR1). To actually be able to represent the video a new filter has been 
> written. In tests a command like
> 
> mplayer -v tv:// -tv \
> driver=v4l2:input=0:width=640:height=480:device=/dev/video0:outfmt=ba81 \
> -vf ba81 -vo fbdev
> 
> has been used.

I don't know if a new video filter is the correct solution.

> The patch also fixes a couple of potential NULL pointer dereferences.

Don't mix new features with bugfixes in one patch. Partial review follows.

[...]
> diff -u a/libmpcodecs/img_format.h~ a/libmpcodecs/img_format.h
> --- a/libmpcodecs/img_format.h~	2007-10-07 21:49:25.000000000 +0200
> +++ a/libmpcodecs/img_format.h	2008-02-21 12:27:26.000000000 +0100
> @@ -91,6 +91,11 @@
>  #define IMGFMT_YUVP 0x50565559
>  #define IMGFMT_UYVP 0x50565955
>  
> +/* Bayer Formats */
> +#define IMGFMT_BA81 0x31384142
> +#define IMGFMT_BYR1 IMGFMT_BA81

What is this duplication useful for?

> +#define IMGFMT_BYR2 0x32525942
> +
>  /* Compressed Formats */
>  #define IMGFMT_MPEGPES (('M'<<24)|('P'<<16)|('E'<<8)|('S'))
>  #define IMGFMT_MJPEG (('M')|('J'<<8)|('P'<<16)|('G'<<24))
[...]
> diff -u a/libmpcodecs/vf.c~ a/libmpcodecs/vf.c
> --- a/libmpcodecs/vf.c~	2007-10-07 21:49:25.000000000 +0200
> +++ a/libmpcodecs/vf.c	2008-02-21 14:56:10.000000000 +0100
> @@ -99,6 +99,7 @@
>  extern vf_info_t vf_info_blackframe;
>  extern vf_info_t vf_info_geq;
>  extern vf_info_t vf_info_ow;
> +extern vf_info_t vf_info_ba81;
>  
>  // list of available filters:
>  static vf_info_t* filter_list[]={
> @@ -193,6 +194,7 @@
>      &vf_info_yadif,
>      &vf_info_blackframe,
>      &vf_info_ow,
> +    &vf_info_ba81,
>      NULL
>  };
>  
> @@ -326,7 +328,8 @@
>  	    mpi->height=h; mpi->chroma_height=(h + (1<<mpi->chroma_y_shift) - 1)>>mpi->chroma_y_shift;
>  	}
>      }
> -    if(!mpi->bpp) mp_image_setfmt(mpi,outfmt);
> +    if(!mpi->bpp)
> +	mp_image_setfmt(mpi,outfmt);

Cosmetics.

>      if(!(mpi->flags&MP_IMGFLAG_ALLOCATED) && mpi->type>MP_IMGTYPE_EXPORT){
>  
>  	// check libvo first!
> @@ -394,7 +397,9 @@
>          }
>      }
>      if(mpi->flags&MP_IMGFLAG_DRAW_CALLBACK)
> -	if(vf->start_slice) vf->start_slice(vf,mpi);
> +	if(vf->start_slice)
> +	    vf->start_slice(vf,mpi);
> +

Cosmetics.

>      if(!(mpi->flags&MP_IMGFLAG_TYPE_DISPLAYED)){
>  	    mp_msg(MSGT_DECVIDEO,MSGL_V,"*** [%s] %s%s mp_image_t, %dx%dx%dbpp %s %s, %d bytes\n",
>  		  vf->info->name,
> diff -u a/libmpcodecs/Makefile~ a/libmpcodecs/Makefile
> --- a/libmpcodecs/Makefile~	2007-10-07 21:49:25.000000000 +0200
> +++ a/libmpcodecs/Makefile	2008-02-21 12:17:13.000000000 +0100
> @@ -94,6 +94,7 @@
>                vf_yuy2.c \
>                vf_yvu9.c \
>                vf_ow.c \
> +	      vf_ba81.c

Inconsistent spacing, lack of \ at the end.

Regards,
R.

-- 
MPlayer developer and RPMs maintainer: http://mplayerhq.hu http://rpm.livna.org
There should be a science of discontent. People need hard times and
oppression to develop psychic muscles.
	-- from "Collected Sayings of Muad'Dib" by the Princess Irulan