[MPlayer-dev-eng] New remove-logo filter

[Yartrebo]

Hi,

For the sake of keeping an already hefty code base reasonable (my filter
will become the second largest one if accepted), I don't think that
three separate loops (requiring 3 implementations of get_blur(...) and 3
#defines) are a wise idea.

However, that part about stride and multiplication are interesting, and
I do access the entries sequentially. The speed-up will probably be
marginal, but it will only take a few lines of code, so I guess I will
implement that.

Sincerely,
Robert Edele

On Thu, 2005-03-03 at 13:35 +0200, Oded Shimon wrote:
> On Thursday 03 March 2005 07:49, Yartrebo wrote:
> > As before, any comments or constructive criticism would be appreciated.
> 
> > +#define get_part_of_pixel_yv12(mpi, x, y, plane) (*(mpi->planes[(plane)] + 
> (mpi->stride[(plane)] * (y)) + (x)))
> 
> You shouldn't be using multiplication in time critical areas.
> The whole point of stride, is that you have a variable which sits on the line 
> you are currently at, and for each pixel you add 'x', and every line you add 
> 'stride':
> 
> uint8_t * tmp = dmpi->planes[0];
> int x, y;
> for (y = 0; y < mpi->h; y++) {
>   for (x = 0; x < mpi->w; x++) {
>     do_stuff(tmp[j]);
>   }
>   tmp += dmpi->stride[0];
> }
> 
> Repeat for U and V. You should probably use a separate loop for them.
> (I made the same mistake BTW, even worse :)
> 
> 
> Other than that, patch looks good.. Haven't tested it though. BTW, since you 
> seen to like comments so much, you might want to use doxygen comments. They 
> are pretty much the same as you have now, just built for doxygen... Check 
> DOCS/tech/code-documentation.txt .
> 
> - ods15
> 
> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng