[MPlayer-dev-eng] [PATCH] blue back tv feature (automute with no signal)

[Reimar Döffinger]

Hello,
On Thu, May 24, 2007 at 09:29:30PM +0600, Vladimir Voroshilov wrote:
> @@ -1321,6 +1328,23 @@
>      int d = pixfmt2depth(priv->format.fmt.pix.pixelformat);
>      int bytesperline = w*d/8;
>  
> +    if(tv_param_automute>0){
> +        if (ioctl(priv->video_fd, VIDIOC_G_TUNER, &priv->tuner) >= 0) {
> +            if(tv_param_automute<<8>priv->tuner.signal){
> +	        if(priv->blank_frame_size!=bytesperline * h){
> +                    if(priv->blank_frame)
> +                        free(priv->blank_frame);
> +                    priv->blank_frame=(unsigned char*)malloc(bytesperline * h);
> +                    priv->blank_frame_size=bytesperline * h;
> +		    fill_blank_frame(priv->blank_frame,bytesperline * h,fcc_vl2mp(priv->format.fmt.pix.pixelformat));
> +	        }
> +                memcpy(dest,priv->blank_frame,bytesperline * h);

WTF is the point of that priv->blank_frame? It only needs a lot of code
and thrashes the caches even more than just calling fill_blank_frame
directly. If fill_blank_frame is significantly slower fix it.
Or if you want it to be really faster use EXPORT type image, then you
won't have to do any memcpy or memset or whatever at all except once.

> Index: stream/tv.c
> ===================================================================
> --- stream/tv.c	(revision 23382)
> +++ stream/tv.c	(working copy)
> @@ -56,6 +56,7 @@
>  float tv_param_fps = -1.0;
>  char **tv_param_channels = NULL;
>  int tv_param_audio_id = 0;
> +unsigned int tv_param_automute = 0;

I am against using unsigned unless there is a good reason - which I
can't see here.

> +/**
> +
> + Fills video frame in given buffer with blue color for yv12,i420,uyvy,yuy2.
> + Other formats will be filled with 0xC0
> + 
> +*/

I'd remove the empty lines.

> +static inline void fill_blank_frame(char* buffer,int len,int fmt){
> +    int i;
> +
> +    memset(buffer,0,len);
> +    //following code will set only nonzero bytes of video frame

I'd guess doing this in two steps is always slower and doesn't even make
the code much simpler...

> +    switch(fmt){
> +    case IMGFMT_YV12:
> +        memset(buffer+5*len/6, 0xFF,len/6);
> +        break;
> +    case IMGFMT_I420:
> +        memset(buffer+4*len/6, 0xff,len/6);
> +        break;
> +    case IMGFMT_UYVY:
> +        for(i=0;i<len;i+=4)
> +            buffer[i]=0xff;
> +        break;
> +    case IMGFMT_YUY2:
> +        for(i=1;i<len;i+=4)
> +            buffer[i]=0xff;
> +        break;
> +    default:
> +        memset(buffer,0xC0,len);

decide for one, either uppercase or lowercase hex numbers.

Greetings,
Reimar Döffinger