Comment 6 for bug 670790

Revision history for this message
Tormod Volden (tormodvolden) wrote :

After looking at it, I think the cast to float in the other code is overkill, since the result also is an integer, and there is no intermediate quotient, like in 3*(2/3) = 0. There are parentheses but even without, multiplication and division goes from left to right so it is like (3*2)/3 and nothing is lost. The only worry would be if the intermediate product overflows, in which case a (single) cast to a higher precision integer would make sense. In our case, dstBox->x1 etc are screen coordinates so maximum in the order of 1000, and XExp1 etc (from InitStreamsForExpansion) are up to 0xC. Since XExp1 etc are "int", the multiplication is done with "int" (at least 32 bit) so it will not overflow.

For preparing a proper patch there is not much to do, and I can walk you through it:
 git clone git://anongit.freedesktop.org/xorg/driver/xf86-video-savage # to retrieve the upstream tree
 (apply your patches)
 git commit -a -s # and make a short comment
 git format-patch -o .. HEAD^
Send the resulting file to me for style review if you want. Make sure name and address in the patch header is how you would like to have it in the git history.

To start with I'd suggest you leave out the +7 removal. The rest is a good and straight forward fix I would like to apply as soon as possible, but the +7 removal is a separate issue which is more delicate.

Other improvements to the driver would be very cool! I agree this hardware is perfectly usable for most tasks. Also the driver took many years to get in good shape and it would a pity to let it rot. I have no greater ambitions for its maintenance than to keep it falling into pieces, but if there are still interested users and developers like you I will help with what I can. I am not familiar with all of the code, and I never did much with xvideo or tv-out.

We should maybe take all this somewhere else, but just to comment quickly on your issues:
- TV resolutions are usually different than for monitors. I think PAL is 720x576.
- Scaling down should work fine with xvideo. Seems to work here.
- I believe the hardware is limited to 1024x1024 (source) for xvideo. So a movie of 1280x800 can not be scaled down with xvideo on savage.

Unfortunately I have no documentation. Just the savage guide linked from here: http://dri.freedesktop.org/wiki/S3Savage