Xv Reput/Reget behaviour changes

[Alex Deucher]

On Mon, Nov 1, 2010 at 7:12 AM, Ville Syrjälä <ville.syrjala at nokia.com> 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, ville.syrjala at nokia.com 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ä
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>