Tracking down bug #12503, looking for comment on my theory

Alex Villacís Lasso a_villacis at palosanto.com
Sun Jan 27 18:30:49 PST 2008


> I'm thinking we could pad the size out to 16 pixels and then use the
> scissors (if necessary) to clip the final image.
>
> Alex
>

I don't quite follow the comment about the sissors. Could you please
elaborate? There is no support, as far as I can see, for using just a
portion of the source pitch for the secondary stream.

I resend the patch for the multiples of 16 in BCI, along with three other
patches that solve various issues with XVideo.

0001-BCI-can-only-handle-widths-that-are-multiple-of-16.patch

Already explained in a previous mail, this patch prevents a lockup on an
attempt to use the BCI to transform a frame with a non-multiple of 16
pixels.

0002-Prevent-use-of-BCI-for-YV12-YUY2-conversion-from.patch

This patch fixes an issue I found when toying with the idea of
implementing a double-buffer for XVideo. When displaying an image in
SavagePutImage(), the driver allocates video memory of a size of new_size
= dstPitch * height. If the driver chooses to use the BCI for YV12 -> YUY2
conversion, the function SavageCopyPlanarDataBCI() builds a pointer to an
intermediate area to hold the data of the planar format, at an offset of 2
* srcPitch * h (height) from the start of the allocated video buffer. From
previous calculations in SavagePutImage(), srcPitch roughly equals
dstPitch/2 (because dstPitch is calculated from width << 1). Therefore,
the temporary area in the BCI case is approximately equal to an offset of
dstPitch * height. However, the BCI case writes (srcPitch + srcPitch2) *
height bytes to this area, past the allocated size of dstPitch * height.
Therefore, the BCI case corrupts whatever happens to be lying past the end
of the buffer (for example, offscreen pixmaps). This patch prevents this
by including the required area for BCI planar data in the allocation of
video memory. The condition for this depends on patch 1 (no BCI for
multiples of 16 pixels) being already applied.

0003-Reset-lastKnownPitch-to-0-right-after-enabling-strea.patch

In SavageDisplayVideoOld() and SavageDisplayVideoNew(), the pitch must be
programmed in CRTC index 0x92 and 0x93 for optimal display speed of the
YUY2 stream. This register is programmed whenever the last known pitch (in
pPriv->lastKnownPitch) changes from the current pitch. The problem is that
this cached value is never flushed on video shutdown, but the register
seems to be reset when shutting down the streams. If the next movie
happens to have the exact same pitch as a previously shown one, the
register is never programmed, resulting in a significant slowdown, most
noticeable when displaying as fullscreen, and snow (when fullscreen). This
patch assigns 0 to the last known pitch right after turning on the
streams, so any previous value is flushed, and the register is programmed
anew. This fixes the issue of snow and video slowdowns on repeated
displays of the same movie in MPlayer.

0004-Do-not-wait-for-vertical-retrace-anymore-on-old-stre.patch

The old streams engine includes a wait for the vertical retrace. This wait
is the single most expensive operation in SavageDisplayVideoOld().
Removing the wait does not seem to cause much shearing (almost none in my
case), and significantly speeds up the video display for MPlayer and
Totem.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-BCI-can-only-handle-widths-that-are-multiple-of-16.patch
Type: text/x-patch
Size: 1906 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20080127/baa95337/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Prevent-use-of-BCI-for-YV12-YUY2-conversion-from.patch
Type: text/x-patch
Size: 1491 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20080127/baa95337/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Reset-lastKnownPitch-to-0-right-after-enabling-strea.patch
Type: text/x-patch
Size: 1157 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20080127/baa95337/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Do-not-wait-for-vertical-retrace-anymore-on-old-stre.patch
Type: text/x-patch
Size: 851 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20080127/baa95337/attachment-0003.bin>


More information about the xorg mailing list