On Mon, Nov 1, 2010 at 7:12 AM, Ville Syrjälä <[email protected]> wrote: > On Sat, Oct 30, 2010 at 02:57:25PM +0200, ext Luc Verhaegen wrote: >> On Fri, Oct 29, 2010 at 09:18:56PM +0300, [email protected] wrote: >> > I'm trying to eliminate some rather nasty looking on/off blinking >> > that's bothering my video overlays. >> > >> > The xfree86 xv code hooks into ClipNotify, WindowExposures and >> > AdjustFrame and does something a little different in each of them. >> > I tried to decipher the intention of the original code, but even >> > after trawling through the xfree86 cvs, I was unable to find any >> > explanation for most of the differences. I suspect the code reached >> > it's current state simply due to some ad-hoc copy pasting in an >> > effort to fix some specific issues. >> > >> > So I just gave up on the history and tried to think what would make >> > the most sense for each case and did that. The end result is that >> > Reput/Reget is done in most cases if the window is visible, >> > StopVideo is done if the window is invisible, and if ReputImage >> > isn't supported the port is removed from the window. >> > >> > With these changes most of the blinking is gone. There is still at >> > least one case left though. Window (un)redirection does an internal >> > UnmapWindow+MapWindow cycle which causes ClipNotify to get called with >> > visibility = NotViewable. If anyone has a suggestion how to eliminate >> > that problem without a majore rewrite, I'd be happy to try it. >> > >> > I'd also like the overlays to track RandR state properly so I added a >> > new hook 'ModeSet' that gets called when something interesting happens. >> > I just realized that I should probably call it from >> > xf86DisableUnusedFunctions() as well. Hmm, actually it looks like >> > xf86DisableUnusedFunctions() already has this crtc_notify hook I could >> > maybe use. Would there be objections to adding more calls to that >> > hook from set_origin (and perhaps some other mode setting functions)? >> > >> > Also CCing Luc since he did something for xf86XVAdjustFrame at some >> > point in the past... >> >> Hi Ville, >> >> Since i have a rather busy time now (family visits, and then visiting >> the nokia family later next week), i will only be able to review this >> properly on the train over to the munich airport on thursday. Sadly, i >> will not be able to give this code a run on a reputimage capable driver >> in at least two weeks (i am not dragging a unichrome over to helsinki >> :)) >> >> The modeset hook does make me frown, that is a serious addition to the >> api, which will make backwards compatibility (of for instance the omapfb >> driver) impossible. > > You mean this? http://cgit.pingu.fi/xf86-video-omapfb/ > It doesn't seem to have any RandR support so it should not hit the new > hook. > >> I am not sure whether this is really necessary. Can >> you explain why this is needed and why driver internal modesetting >> cannot handle this? > > I suppose you can push everything to the driver if you want, but then > why is AdjustFrame handled by the xf86xv code? > > Also AdjustFrame is most conveniently implemented by simply calling > set_origin on the crtc, and if you do that you end up doing two Reputs > for one AdjustFrame operation (one via the driver's set_origin hook and > another one via the xf86xv AdjustFrame hook). > > Drivers would also need to keep more Xv state around (most of the > original arguments passed to PutImage) to be able to recalculate the > clipping and scaling. In fact they already have to do some of that to > support ReputImage since ReputImage doesn't pass all the necessary > coordinates to the driver, but that's something I'd like to change. > ReputImage is also somewhat poorly named since it will be used for > stills as well as images. > > If there's some real compatibility issue with the hook, I could perhaps > add a new flag to the Xv registration which would allow the driver to > specify whether the hook is used or not. > > > > BTW another problem with ReputImage occured to me when I was thinking > about this stuff. If the original PutImage/Still is fully clipped the > port is never marked as ON/PENDING and the driver's hook is never > called. Thus ReputImage will never be called and that image/still > never gets shown even if the window clipping changes. That seems OK > if PutImage/Still is supposed to work like a normal XPutImage. > > However when the PutImage/Still is not fully clipped, the area covered > by the image/still can actually expand if the window clip changes. That > seems wrong, and doesn't match XPutImage behaviour. So I was thinking > of making the composite clip unable to grow, even if the window clip > grows. I suppose for PutVideo/GetVideo it would be OK to allow the > composite clip to grow since they're not supposed to be one-shot > operations. > > That also means that CLIP_TO_VIEWPORT is fundementally broken wrt. > ReputImage as it can cause the initial PutImage/Still to be fully > clipped, and thus ReputImage is never called even if the viewport > moves to a more favorable position later.
FWIW, as I recall ReputImage had issues with xinerama which is why it's implemented in so few drivers as it didn't work with multi-head. At this point I don't recall the details, but it might be worth looking at if are are planning on reworking that code. Alex > > -- > Ville Syrjälä > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
