XSync in DisplayVideo (xcommon.c)

[bryanair]

(informal introduction - I am a member of the same software team as Bryan. I'm Bob Frazier, and I am temporarily taking over the work on vlc that Bryan was doing)

One of the problems we have been seeing during video playback is a "tearing" or "shearing" effect that resembles the same effect as rendering a fast-moving video on two consecutive frames. In effect, part of your 'new new' rendering is on one frame, and part is on another, which "tears" an object across the direction of motion. Although some drivers (like NVidia) appear to have support to help prevent this, there is one other thing that can be done for drivers that have no such support.

The problem appears to be related to the use of XSync (a necessary evil) in the DisplayVideo function. The current implementation sends the appropriate commands to the X server (either X11 or X Video) to render a specific block of memory (ideally shared memory) at a specific location on the screen. After sending these commands the 'XSync' function is called to wait until the command completes. The obvious reason here is that you don't want to modify the memory block until the X Server is done with it.

Unfortunately the X Server will occasionally take as long as 100 msecs (or more), even on very fast machines with very fast video hardware, to complete this process. Obviously this is much longer than a single video frame. The result is a degradation in video quality.

To help alleviate this, we did various experiments with vlc 0.9.8a under FreeBSD 7.1 via SDL, using an ATI driver that (unfortunately) does not support X Video. The most successful modification was to insert an 'XFlush' followed bu 'usleep(500)' after rendering, and just before calling XSync. Various timing measurements indicated that average rendering time, frame drops, and the total number of times the rendering process exceeded 30 msecs was at its relative "best point" in this configuration. Since we currently need a player that can display video "as perfectly as possible" we are currently patching the 0.9.8a version as follows:


static void DisplayVideo( vout_thread_t *p_vout, picture_t *p_pic )
{
...
// Airgain begin - XFlush to send commands, usleep to force context switch.
// This is likely to help client/server interaction
XFlush( p_vout->p_sys->p_display ); // force commands to be sent
usleep(500); // then switch contexts before waiting
// Airgain end
/* Make sure the command is sent now - do NOT use XFlush !*/
XSync( p_vout->p_sys->p_display, False );

vlc_mutex_unlock( &p_vout->p_sys->lock );
}

The comment about 'do NOT use XFlush' is correct in that you need to wait for completion. However, XSync normally does the equivalent of XFlush internally. In this case we're interested in the X Server returning back a success or failure before continuing, and the idea of forcing a thread context switch (for 500 mucroseconds) IMMEDIATELY after flushing the messages should improve the X Server's ability to schedule garbage collection more appropriately. It is likely to show even better improvement on multi-core processors.

Both measurements and observations of videos that easily exhibit "tears" or "shears" show a dramatic improvement after adding these two lines of code.

There may be additional performance improvements that could be done in the main loop, but for now this seems to be enough to eliminate the tearing and shearing problems we've observed with our test videos.

At any rate, I apologize if I have been too verbose. It seems that I needed to put this in context in order to show why the patch is needed.

[Rémi Denis-Courmont]

We should just call XFlush, but retain the picture buffer until we get the PutImage response (asynchronously). I am not sure if this is do-able with Xlib though, and the VLC video output core does not seem to implement this notion anyway.