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

[Vladimir Voroshilov]

Hi, Remar.
Thank you for review.

2007/5/24, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> 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.
I thought one memcpy (assuming that video frame size will not be
frequently changed)
will be a bit faster. Fixed to fill_blank_frame_call. I didnt
undestand what did you mean by "EXPORT type image". Could you explain
this for me ?

>
> > 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.
Fixed.

> > +/**
> > +
> > + 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.
Fixed.

> > +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...
Fixed.

> > +    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.
Fixed to uppercase.

Updated patch is attached.

-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: automute2.diff
Type: application/octet-stream
Size: 5017 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20070525/8f53496e/attachment.obj>