[Ffmpeg-devel] [PATCH] watermark.c optimizations for single-frameinput (previous patch included)

[Michael Niedermayer]

Hi

On Fri, Oct 20, 2006 at 06:07:59PM +0300, Mihail Stoyanov wrote:
[...]
> >
> >[...]
> >>+     if (thrR == (int)((pixelm >> 16) & 0xff) &&
> >>+ thrG == (int)((pixelm >> 8) & 0xff) &&
> >>+ thrB == (int)((pixelm >> 0) & 0xff)) {
> >
> >precalculate thr= thrB + (thrG<<8) + (thrR<<16)
> >and then use if(thr == pixelm)
> >or
> >if(thr == (pixelm & 0xFFFFFF))
> >or something like that
> >
> 
> sorry, i won't.
> it works faster my way. 

if thats true then iam certainly ok with that part but i doubt it


> it has 33% chance of probability to fallback to 
> "else" when checking the first argument (red). and 66% when checking the 
> second (green).
> precalculation is even more cpu intensive. 

it is not as thr* doesnt change, the precalculation is just done once per 
image


> additionally you can't use "if 
> (thr == pixelm)" but "if(thr == (pixelm & 0xFFFFFF))" since you need just 3 
> bytes of the int
> if you don't believe me - try it yourself. get a bigger source so you can 
> make more accurate test
> 
> 
> test times
> my way -> Convert time: 00:18
> your way -> Convert time: 00:19

maybe you might want to try, START/STOP_TIMER (see libavutil/common.h)
they are much more accurate for testing the speed of some code

also in the original code and your patched code

            // R
            tmp = (int)((pixel >> 16) & 0xff) + (int)((pixelm >> 16) & 0xff) - thrR;
            if (tmp > 255) tmp = 255;
            if (tmp < 0) tmp = 0;
            pixel_meck |= (tmp << 16) & 0xff0000;
            // G
            tmp = (int)((pixel >> 8) & 0xff) + (int)((pixelm >> 8) & 0xff) - thrG;
            if (tmp > 255) tmp = 255;
            if (tmp < 0) tmp = 0;
            pixel_meck |= (tmp << 8) & 0xff00;
            // B
            tmp = (int)((pixel >> 0) & 0xff) + (int)((pixelm >> 0) & 0xff) - thrB;
            if (tmp > 255) tmp = 255;
            if (tmp < 0) tmp = 0;
            pixel_meck |= (tmp << 0) & 0xff;

the & 0xff0000, & 0xff00 and & 0xff do nothing they can be removed
and maybe you want to try 
if(tmp&(~255)) tmp= ((-tmp)>>31)&0xFF;
instead of the tmp>255 and tmp<0 its one check instead of 2

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is