xv + silkenmouse bug

[Thomas Winischhofer]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


It seems I am pushing the common code to its limits with my current
work. This time it's about Xv:

1) Xv in combination with silkenmouse:

Even if this is assumingly unneccessary, but please allow me to remind
me and you that Silkenmouse means that the cursor update code is
executed asynchronously, ie it can interrupt on-going other operations
of the server.

The cursor code (pScrn->PointerMoved) is there to eventually pan the
viewport. So it is likely that pScrn->AdjustFrame is executed, which is
wrapped by Xv.

Xv's wrapper for AdjustFrame, if the driver has not installed a
ReputImage function, stops the video (by calling the driver's stopvideo
routine) and removes the port from the window (which is done in
RemovePortFromWindow()). This involves NULLifying the pointer to the
drawable in the Xv private structure.

However, XVAdjustFrame does so only if the video is currently on,
indicated by pPriv->IsOn == XV_ON. This is correct.

But what is not correct is that XvAdjustFrame(), before calling
RemovePortFromWindow, does not check if the pPriv->Draw is set.

So, the condition under which XVAdjustFrame calls RemovePortFromWindow
is only that pPriv->IsOn = XV_ON.

PutImage, on the other hand, removes the port from the window every time
it is called. BUT: It does not set pPriv->IsOn to XV_OFF or XV_PENDING
(whatever would be correct, haven't thought about this yet) at this point.

PutImage removes the port from the window (ie NULLifies the pointer to
the drawable), and proceeds doing its business, without touching
pPriv->IsOn until the very end, after the driver's PutImage() has been
called.

During the time between the RemovePortFromWindow() call and the near end
of PutImage the situation is that

- - the portPriv->pDraw is NULL
- - pPriv->isOn is eventually XV_ON from a previous call to PutImage().

If the silken mouse, ie AdjustFrame interrupts PutImage during this
time, it will see IsOn == XV_ON and therefore eventually call
RemovePortFromWindow with a NULL pDraw (= pWin) - and the rest is easy
to imagine (considering that RemovePortFromWindow does no NULL-check on
its pWin argument).

This discovery frightens me. Sure, the easy way is to check for a NULL
pointer in XVAdjustFrame, or - perhaps better - to add a pPriv->XV_xxx
after or before PutImage's call to RemovePortFromWindow. But this is
only a work-around for a more deeper problem.

Mildly put, I am not sure that letting the driver's stopvideo()
interrupt an on-going PutImage() is safe in any case. I really wonder
why no one ever has noticed all that.

For an easy and quick fix, I vote for inserting

    portPriv->IsOn = XV_OFF;

before the call to xf86XVRemovePortFromWindow() in xf86XVPutImage. This
could also prevent a stopvideo interrupting an on-going Putimage(). But
I am hardly convinced that this is a long-term solution.


2) ReputImage

ReputImage is used to ... reput the image. It is, among other places,
called by XVAdjustFrame to inform the driver about the new coordinates
of the overlay.

Apart from the fact that this might lead to ReputImage call interrupting
an on-going PutImage operation, this design also has the following issue:

If the frame (viewport) is located so that the overlay would not be
visible at all, XVAdjustFrame does not call ReputImage, but does the
same as it does if the driver has not provided a ReputImage function: It
stops the video and removes the port from the window. The latter leads
to IsOn being set to XV_PENDING.

So, by design, once the overlay is entirely off-screen, even if I pan
back so that it would be (partly) visible, it does not reappear. Why?
Because in XVAdjustFrame(), the eventual call to ReputImage is only
executed if IsOn == XV_ON. In case the other alternative was called
previously, IsOn is XV_PENDING as said.

Comments?

Thomas

- --
Thomas Winischhofer
Vienna/Austria
thomas AT winischhofer DOT net	       *** http://www.winischhofer.net

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDPU40zydIRAktyUcRAt9OAKC1viZKWfaGLohRSVIQzES7AVITVACfXo7Q
PuJzXgkFtFbThY2NHui8jno=
=2qKK
-----END PGP SIGNATURE-----