[FFmpeg-devel] swscale merge

[Michael Niedermayer]

Hi ronald

First id like to thank you for all the great cleanups you do
to swscale.
But as is your code is really full of obvious bugs that you should fix
otherwise its hard to integrate into production quality software.
and the people who use libav also wont be happy

for example:

commit ef1ee362b36893cd1cc1e9c7de378e9cb239b0e8
Author: Ronald S. Bultje <rsbultje at gmail.com>
Date:   Wed Jun 29 09:39:43 2011 -0700
...
+static void lumRangeToJpeg16_c(int16_t *_dst, int width)
+{
+    int i;
+    int32_t *dst = (int32_t *) _dst;
              ^^^^^^^^^^^^^^^^^^
undefined behavior (aliasing violation)


+    for (i = 0; i < width; i++)
+        dst[i] = (FFMIN(dst[i],30189)*19077 - 39057361)>>14;
                                ^^^^^
clips the 32bit values incorrectly
                                                       ^^^^^
overflows

Yes i know you didnt test that and you dont want me to review your
code but really i think it would be for the better if your changes
would be reviewed by someone who knows swscale.
3 bugs in a 4 line function should hopefully show that even to
someone as stubborn as you. ;)

also, the use of 32bit internal arrays is unneeded when the input and
output is less than 16bit (like for example the common 10bit yuv cases)
and will probably cause a 50% speed loss once this is implemented in
SIMD
you could look at how i implemented it long ago in ffmpeg and use that
code. My code also needs fewer passes over the data which improves
speed by another step.

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110630/2f45b836/attachment.asc>