[MPlayer-dev-eng] [PATCH] get rid of lots of useless/duplicate code in vo_xv/vo_xvmc

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Tue Mar 6 15:20:22 CET 2007


Hello,
On Tue, Mar 06, 2007 at 04:01:22PM +0200, Ivan Kalvachev wrote:
> 2007/3/6, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> >On Sun, Feb 18, 2007 at 11:32:04AM +0100, Reimar D?ffinger wrote:
> >> see attached patch.
> >> Could not see it breaking anything...
> >
> >Any comments?
> 
> Haven't tested it, but it looks OK.
> 
> I just don't like foo(void) functions that mess with "globals", but I
> guess putting all these variables in struct and passing it around is
> too much work for such trivial change ;)
> 
> btw how about naming the function calc_drwXY() ?
> usually get_something() is expected to return the thing it gets ;)

Well, I can at least pass around pointers to the global variable as in
attached patch, if you prefer that.
In the long term a vo struct would be nice, but that is a much bigger
change to do in a useful way and would be overkill for passing around to
such a simple function, too.
Btw. should I attempt moving the code shared between those two vos to
a xvideo.c (or similar)? At least most of config could be put there I
think (though not sure how ugly it would get with all those globals).

Greetings,
Reimar Döffinger
-------------- next part --------------
Index: libvo/vo_xv.c
===================================================================
--- libvo/vo_xv.c	(revision 22477)
+++ libvo/vo_xv.c	(working copy)
@@ -43,6 +43,8 @@
 #include "Gui/interface.h"
 #endif
 
+#include "libavutil/common.h"
+
 static vo_info_t info = {
     "X11/Xv",
     "xv",
@@ -89,7 +91,6 @@
 
 static Window mRoot;
 static uint32_t drwX, drwY, drwBorderWidth, drwDepth;
-static uint32_t dwidth, dheight;
 static uint32_t max_width = 0, max_height = 0; // zero means: not set
 
 static void (*draw_alpha_fnc) (int x0, int y0, int w, int h,
@@ -141,6 +142,23 @@
 
 static void deallocate_xvimage(int foo);
 
+static void calc_drwXY(uint32_t *drwX, uint32_t *drwY) {
+  *drwX = *drwY = 0;
+  aspect(&vo_dwidth, &vo_dheight, A_NOZOOM);
+  if (vo_fs) {
+    aspect(&vo_dwidth, &vo_dheight, A_ZOOM);
+    vo_dwidth = FFMIN(vo_dwidth, vo_screenwidth);
+    vo_dheight = FFMIN(vo_dheight, vo_screenheight);
+    *drwX = (vo_screenwidth - vo_dwidth) / 2;
+    *drwY = (vo_screenheight - vo_dheight) / 2;
+    mp_msg(MSGT_VO, MSGL_V, "[xv-fs] dx: %d dy: %d dw: %d dh: %d\n",
+           *drwX, *drwY, vo_dwidth, vo_dheight);
+  } else if (WinID == 0) {
+    *drwX = vo_dx;
+    *drwY = vo_dy;
+  }
+}
+
 /*
  * connect to server, create and map window,
  * allocate colors and (shared) memory
@@ -242,23 +260,6 @@
             aspect_save_screenres(modeline_width, modeline_height);
         } else
 #endif
-        if (vo_fs)
-        {
-#ifdef X11_FULLSCREEN
-            /* this code replaces X11_FULLSCREEN hack in mplayer.c
-             * aspect() is available through aspect.h for all vos.
-             * besides zooming should only be done with -zoom,
-             * but I leave the old -fs behaviour so users don't get
-             * irritated for now (and send lots o' mails ;) ::atmos
-             */
-
-            aspect(&d_width, &d_height, A_ZOOM);
-#endif
-
-        }
-//   dwidth=d_width; dheight=d_height; //XXX: what are the copy vars used for?
-        vo_dwidth = d_width;
-        vo_dheight = d_height;
         hint.flags = PPosition | PSize /* | PBaseSize */ ;
         hint.base_width = hint.width;
         hint.base_height = hint.height;
@@ -299,12 +300,7 @@
                              &drwBorderWidth, &drwDepth);
                 if (vo_dwidth <= 0) vo_dwidth = d_width;
                 if (vo_dheight <= 0) vo_dheight = d_height;
-                drwX = drwY = 0; // coordinates need to be local to the window
                 aspect_save_prescale(vo_dwidth, vo_dheight);
-            } else
-            {
-                drwX = vo_dx;
-                drwY = vo_dy;
             }
         } else if (vo_window == None)
         {
@@ -400,25 +396,8 @@
     set_gamma_correction();
 #endif
 
-    aspect(&vo_dwidth, &vo_dheight, A_NOZOOM);
-    if (((flags & VOFLAG_FULLSCREEN) && (WinID <= 0)) || vo_fs)
-    {
-        aspect(&vo_dwidth, &vo_dheight, A_ZOOM);
-        drwX =
-            (vo_screenwidth -
-             (vo_dwidth >
-              vo_screenwidth ? vo_screenwidth : vo_dwidth)) / 2;
-        drwY =
-            (vo_screenheight -
-             (vo_dheight >
-              vo_screenheight ? vo_screenheight : vo_dheight)) / 2;
-        vo_dwidth =
-            (vo_dwidth > vo_screenwidth ? vo_screenwidth : vo_dwidth);
-        vo_dheight =
-            (vo_dheight > vo_screenheight ? vo_screenheight : vo_dheight);
-        mp_msg(MSGT_VO, MSGL_V, "[xv-fs] dx: %d dy: %d dw: %d dh: %d\n",
-               drwX, drwY, vo_dwidth, vo_dheight);
-    }
+    if ((flags & VOFLAG_FULLSCREEN) && WinID <= 0) vo_fs = 1;
+    calc_drwXY(&drwX, &drwY);
 
     panscan_calc();
     
@@ -557,25 +536,7 @@
         mp_msg(MSGT_VO, MSGL_V, "[xv] dx: %d dy: %d dw: %d dh: %d\n", drwX,
                drwY, vo_dwidth, vo_dheight);
 
-        aspect(&dwidth, &dheight, A_NOZOOM);
-        if (vo_fs)
-        {
-            aspect(&dwidth, &dheight, A_ZOOM);
-            drwX =
-                (vo_screenwidth -
-                 (dwidth > vo_screenwidth ? vo_screenwidth : dwidth)) / 2;
-            drwY =
-                (vo_screenheight -
-                 (dheight >
-                  vo_screenheight ? vo_screenheight : dheight)) / 2;
-            vo_dwidth =
-                (dwidth > vo_screenwidth ? vo_screenwidth : dwidth);
-            vo_dheight =
-                (dheight > vo_screenheight ? vo_screenheight : dheight);
-            mp_msg(MSGT_VO, MSGL_V,
-                   "[xv-fs] dx: %d dy: %d dw: %d dh: %d\n", drwX, drwY,
-                   vo_dwidth, vo_dheight);
-        }
+        calc_drwXY(&drwX, &drwY);
     }
 
     if (e & VO_EVENT_EXPOSE || e & VO_EVENT_RESIZE)
Index: libvo/vo_xvmc.c
===================================================================
--- libvo/vo_xvmc.c	(revision 22477)
+++ libvo/vo_xvmc.c	(working copy)
@@ -36,6 +36,8 @@
 #include "Gui/interface.h"
 #endif
 
+#include "libavutil/common.h"
+
 //no chanse xinerama to be suported in near future
 #undef HAVE_XINERAMA
 
@@ -421,6 +423,23 @@
    return 0;
 }
 
+static void calc_drwXY(uint32_t *drwX, uint32_t *drwY) {
+  *drwX = *drwY = 0;
+  aspect(&vo_dwidth, &vo_dheight, A_NOZOOM);
+  if (vo_fs) {
+    aspect(&vo_dwidth, &vo_dheight, A_ZOOM);
+    vo_dwidth = FFMIN(vo_dwidth, vo_screenwidth);
+    vo_dheight = FFMIN(vo_dheight, vo_screenheight);
+    *drwX = (vo_screenwidth - vo_dwidth) / 2;
+    *drwY = (vo_screenheight - vo_dheight) / 2;
+    mp_msg(MSGT_VO, MSGL_V, "[xvmc-fs] dx: %d dy: %d dw: %d dh: %d\n",
+           *drwX, *drwY, vo_dwidth, vo_dheight);
+  } else if (WinID == 0) {
+    *drwX = vo_dx;
+    *drwY = vo_dy;
+  }
+}
+
 static int config(uint32_t width, uint32_t height,
 		       uint32_t d_width, uint32_t d_height,
 		       uint32_t flags, char *title, uint32_t format){
@@ -644,21 +663,6 @@
       }
       else
 #endif
-      if ( vo_fs )
-      {
-#ifdef X11_FULLSCREEN
-     /* this code replaces X11_FULLSCREEN hack in mplayer.c
-      * aspect() is available through aspect.h for all vos.
-      * besides zooming should only be done with -zoom,
-      * but I leave the old -fs behaviour so users don't get
-      * irritated for now (and send lots o' mails ;) ::atmos
-      */
-
-         aspect(&d_width,&d_height,A_ZOOM);
-#endif
-
-      }
-   vo_dwidth=d_width; vo_dheight=d_height;
    hint.flags = PPosition | PSize /* | PBaseSize */;
    hint.base_width = hint.width; hint.base_height = hint.height;
    XGetWindowAttributes(mDisplay, DefaultRootWindow(mDisplay), &attribs);
@@ -685,9 +689,8 @@
          XGetGeometry(mDisplay, vo_window, &mRoot,
                       &drwX, &drwY, &vo_dwidth, &vo_dheight,
                       &drwBorderWidth, &drwDepth);
-         drwX = drwY = 0; // coordinates need to be local to the window
          aspect_save_prescale(vo_dwidth, vo_dheight);
-      } else { drwX=vo_dx; drwY=vo_dy; }
+      }
    } else 
       if ( vo_window == None ){
          vo_window = XCreateWindow(mDisplay, mRootWin,
@@ -733,16 +736,8 @@
 #endif
    }
 
-   aspect(&vo_dwidth,&vo_dheight,A_NOZOOM);
-   if ( (( flags&VOFLAG_FULLSCREEN )&&( WinID <= 0 )) || vo_fs )
-   {
-      aspect(&vo_dwidth,&vo_dheight,A_ZOOM);
-      drwX=( vo_screenwidth - (vo_dwidth > vo_screenwidth?vo_screenwidth:vo_dwidth) ) / 2;
-      drwY=( vo_screenheight - (vo_dheight > vo_screenheight?vo_screenheight:vo_dheight) ) / 2;
-      vo_dwidth=(vo_dwidth > vo_screenwidth?vo_screenwidth:vo_dwidth);
-      vo_dheight=(vo_dheight > vo_screenheight?vo_screenheight:vo_dheight);
-      mp_msg(MSGT_VO,MSGL_V, "[xvmc-fs] dx: %d dy: %d dw: %d dh: %d\n",drwX,drwY,vo_dwidth,vo_dheight );
-   }
+   if ((flags & VOFLAG_FULLSCREEN) && WinID <= 0) vo_fs = 1;
+   calc_drwXY(&drwX, &drwY);
 
    panscan_calc();
 
@@ -1099,7 +1094,6 @@
 }
 
 static void check_events(void){
-int dwidth,dheight;
 Window mRoot;
 uint32_t drwBorderWidth,drwDepth;
 
@@ -1110,20 +1104,10 @@
 
       XGetGeometry( mDisplay,vo_window,&mRoot,&drwX,&drwY,&vo_dwidth,&vo_dheight,
                    &drwBorderWidth,&drwDepth );
-      drwX = drwY = 0;
       mp_msg(MSGT_VO,MSGL_V, "[xvmc] dx: %d dy: %d dw: %d dh: %d\n",drwX,drwY,
               vo_dwidth,vo_dheight );
 
-      aspect(&dwidth,&dheight,A_NOZOOM);
-      if ( vo_fs )
-      {
-         aspect(&dwidth,&dheight,A_ZOOM);
-         drwX=( vo_screenwidth - (dwidth > vo_screenwidth?vo_screenwidth:dwidth) ) / 2;
-         drwY=( vo_screenheight - (dheight > vo_screenheight?vo_screenheight:dheight) ) / 2;
-         vo_dwidth=(dwidth > vo_screenwidth?vo_screenwidth:dwidth);
-         vo_dheight=(dheight > vo_screenheight?vo_screenheight:dheight);
-         mp_msg(MSGT_VO,MSGL_V, "[xvmc-fs] dx: %d dy: %d dw: %d dh: %d\n",drwX,drwY,vo_dwidth,vo_dheight );
-      }
+      calc_drwXY(&drwX, &drwY);
    }
    if ( e & VO_EVENT_EXPOSE )
    {


More information about the MPlayer-dev-eng mailing list