[MPlayer-dev-eng] xvmc crop patch

[DivXero at gmx.net]

-------- Original-Nachricht --------
Date: Fri, 23 Jun 2006 22:01:55 +0200
From: Dominik \'Rathann\' Mierzejewski <dominik at rangers.eu.org>
To: mplayer-dev-eng at mplayerhq.hu
Subject: Re: [MPlayer-dev-eng] xvmc crop patch

> On Friday, 23 June 2006 at 19:42, DivXero at gmx.net wrote:
> > Attached, find an xvmc crop patch. It's pretty simple to use and useful
> > for xvmc users. For instance, it helps me crop out the garbage at the
> top
> > of a broadcast (TV) video. Since panscan doesn't help with vertical
> > cropping, this is the only way I can remove the annoying (teletext?)
> > garbage.
> > 
> > Syntax-wise, it's quite similar to vf_crop's syntax:
> > 
> > Usage: mplayer -vc ffmpeg12mc -vo xvmc:crop=WxH[+X+Y]
> > (X and Y offsets are optional, if not given, image is centered)
> > 
> > So for example, I use "-vo xvmc:crop=1912x1048" to "overscan" the edges
> > from a 1080i HDTV broadcast.
> 
> I'd prefer it if you used exactly the same syntax as -vf crop for
> consistency.

I would too but the problem is that it's a driver option and the ":" character is already used for option separation so crop=W:H:X:Y cannot be used.
 
> > I know it's not the prettiest patch you've ever seen, but it's a very
> > important feature for me so if someone could clean it up a bit and
> commit
> > it, that would be great.
> 
> Doesn't look too bad. A few comments:
> 
> @@ -430,8 +442,6 @@
>  #ifdef HAVE_XF86VM
>  int vm=0;
>  unsigned int modeline_width, modeline_height;
> -static uint32_t vm_width;
> -static uint32_t vm_height;

Yeah, remove those too.

>  #endif
>  //end of vo_xv
> 
> @@ -624,16 +677,13 @@
>     {
>        hint.x = vo_dx;
>        hint.y = vo_dy;
> -      hint.width = d_width;
> -      hint.height = d_height;
> +      hint.width = vo_dwidth;
> +      hint.height = vo_dheight;
> +
>  #ifdef HAVE_XF86VM
>        if ( vm )
>        {
> -        if ((d_width==0) && (d_height==0))
> -           { vm_width=image_width; vm_height=image_height; }
> -        else
> -           { vm_width=d_width; vm_height=d_height; }
> -        vo_vm_switch(vm_width, vm_height,&modeline_width,
> &modeline_height);
> +    vo_vm_switch(vo_dwidth, vo_dheight,&modeline_width,
> &modeline_height);
>          hint.x=(vo_screenwidth-modeline_width)/2;
>          hint.y=(vo_screenheight-modeline_height)/2;
>          hint.width=modeline_width;
> 
> Why are you removing vm_height and vm_width? Are you sure the code behaves
> the same way as it did before if you don't use crop?

They're only used for vo_vm_switch() and since I use vm_width and vm_height, I don't need that entire if block. The resulting video seems 100% a-okay, I've been using this patch for almost a year now and have no problems (I've tested a fair share of possible problems too).

> Also, the vo_vm_switch line is indented inconsistently with its
> surroundings after your changes.

Oops. I lost the tab by accident (I use fake tabs in vim).


-ben9
-- 


Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen!
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer