[MPlayer-dev-eng] [PATCH] blue back tv feature (automute with no signal)
Vladimir Voroshilov
voroshil at gmail.com
Thu May 24 20:04:48 CEST 2007
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>
More information about the MPlayer-dev-eng
mailing list