[MPlayer-dev-eng] [PATCH][RESEND] XV page flipping sync

[Joey Parrish]

On Sun, Jun 20, 2004 at 07:28:41PM -0500, Joey Parrish wrote:
> On Sun, Jun 20, 2004 at 07:26:18PM -0500, Joey Parrish wrote:
> > Hello,
> > 
> > This mail has been sitting in my inbox for some time now.  There was no
> > [PATCH] in the subject, which may be why it's not committed.
> 
> Oh, damn damn damn.  Here's the patch this time.
> If there was an award for sending [PATCH] emails with no patches in
> them, I think I'd be a strong contender.

Hahahaha!  Oops.  Yes, once again, _this time_ is a patch.  You have to
admit, that is the first time I've attached another email instead of a
patch.

> The original mail is from Piotr Neuman <sikkh at wp.pl> on Mar 14.
> 
> > On my desktop machine (Athlon XP 2000+, Gigabyte VIA VT266a, Radeon 9000, 
> > XFree 4.3, kernel 2.6.4-ck1) there were some problems in synchronization of 
> > video output (XV driver) - I could see for example 'tearing' (when double 
> > buffering was used).
> 
> > After some investigaton into it with help of Con Kolivas we were able to 
> > conclude that on my kernel/hw configuration, there is priviledge inversion 
> > issue (mplayer niced to +1 would work just fine). Such problems are usually 
> > caused by how programs are written.
> 
> > It boils down to the usage of XFlush() in libvo/vo_xv.c . The problem is that 
> > when synchronizing mplayer video buffer with X display, this function can't 
> > ensure that X server will be given timeslice soon enough.
> 
> > Mplayer can continue rendering of next frame/frames (when page flipping is 
> > used) after calling XFlush but X server may not be able to update screen on 
> > time, so few pages may be 'lost' (flipped at wrong time intervals beetween 
> > each other).
> 
> > My patch changes XFlush() call into XSync() which ensures that page will be 
> > flipped properly on time (ie. when using -double switch), becose this call 
> > waits till X has reported completion and gives free hand for system scheduler 
> > to assing timeslice for X (thanks to blocking mplayer - preempting it).
> 
> > Performance degradation is neglible, according to my limited testing. For 
> > example for Matrix mirage trailer (matrix_tr_mirage_480.mov) (sound output 
> > thru alsa1x):
> 
> > before patching:
> > BENCHMARKs: VC:  13,746s VO:   1,344s A:   0,954s Sys:  43,573s =   59,617s
> > BENCHMARK%: VC: 23,0570% VO:  2,2551% A:  1,5997% Sys: 73,0882% = 100,0000%
> 
> > after:
> > BENCHMARKs: VC:  11,691s VO:   4,802s A:   0,912s Sys:  42,216s =   59,621s
> > BENCHMARK%: VC: 19,6089% VO:  8,0539% A:  1,5296% Sys: 70,8077% = 100,0000%
> 
> > With this change the output is synchronized very well, even though mplayer 
> > uses usleep() timing (note that 2.6 kernels have clock set to 1000Hz, so 
> > using rtc is uneceseary - A-V sync is ~ +- 0,005 at worst). The issue of 
> > prority inversion is gone.
> 
> > The patch also contains removal of unneceseary calls to XFlush function just 
> > before XSync is called (XSync() alone suffices).
> 
> > Regards.
> 
> > Piotr Neuman
> 
> I can't remember if anyone responded to this, but I saw some improvement
> on my box.  But I don't know much about XV.  Any reason not to apply this?

--Joey

-- 
"Are disorders necessarily bad?" --Ike
-------------- next part --------------
Index: vo_xv.c
===================================================================
RCS file: /cvsroot/mplayer/main/libvo/vo_xv.c,v
retrieving revision 1.151
diff -u -r1.151 vo_xv.c
--- vo_xv.c	30 Nov 2003 16:36:10 -0000	1.151
+++ vo_xv.c	14 Mar 2004 15:31:33 -0000
@@ -273,7 +273,6 @@
     
     if ( vo_gc != None ) XFreeGC( mDisplay,vo_gc );
     vo_gc = XCreateGC(mDisplay, vo_window, 0L, &xgcv);
-    XFlush(mDisplay);
     XSync(mDisplay, False);
 #ifdef HAVE_XF86VM
     if ( vm )
@@ -403,8 +402,7 @@
    free(xvimage[foo]->data);
   }
  XFree(xvimage[foo]);
- 
- XFlush( mDisplay );
+
  XSync(mDisplay, False);
  return;
 }
@@ -465,11 +463,9 @@
          0, 0,  image_width, image_height,
          drwX-(vo_panscan_x>>1),drwY-(vo_panscan_y>>1),vo_dwidth+vo_panscan_x,(vo_fs?vo_dheight - 1:vo_dheight)+vo_panscan_y);
   }
- if (num_buffers>1){
+ if (num_buffers>1)
     current_buf=vo_directrendering?0:((current_buf+1)%num_buffers);
-    XFlush(mDisplay);
- } else
-    XSync(mDisplay, False);   
+ XSync(mDisplay, False);
  return;
 }