[FFmpeg-devel] port mplayer eq filter to libavfilter
Michael Niedermayer
michaelni
Thu Nov 25 18:25:04 CET 2010
On Thu, Nov 25, 2010 at 10:18:21AM -0500, Ronald S. Bultje wrote:
> Hi,
>
> On Thu, Nov 25, 2010 at 4:27 AM, William Yu <genwillyu at gmail.com> wrote:
> > I have remove unnecessary param from process functions.
> > Please check this patch. Thanks.
>
> Please don't top-post.
>
> > +It accepts the following parameters:
> > + at var{brightness}:@var{constrast}
>
> Range of values?
>
> > + * FFmpeg is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
>
> GStreamer has a LGPL variant of this. why not use that? I hate this
> "everything-is-GPL-except-the-wrapper" concept. For trivial code such
> as this, I'd almost say we should reject patches...
>
> > +static void process_c( uint8_t *line, int stride,
> > + int w, int h, int brightness, int contrast)
> > +{
> [..]
> > + contrast = ((contrast+100)*256*256)/100;
> > + brightness = ((brightness+100)*511)/200-128 - (contrast>>9);
>
> Can FASTDIV() be used here? (Leave the original code in comments for
> easier understanding).
this code does not look speed critical
>
> > +void ff_eq_filter_process_mmx(uint8_t *line, int stride,
> > + int w, int h, int brightness, int contrast)
> [..]
> > + while (h--) {
> > + __asm__ volatile (
> > + "movq (%3), %%mm3 \n\t"
> > + "movq (%4), %%mm4 \n\t"
> > + "pxor %%mm0, %%mm0 \n\t"
> > + "movl %2, %%eax\n\t"
>
> You will recalculate each of these per row iteration, that sounds like
> a huge waste.
per row code is >300 times less important speedwise than per pixel.
Not saying this cant or shoulndt be improved, just saying that it wont help
speed
[...]
> > + __asm__ volatile ( "emms \n\t" ::: "memory" );
>
> Uh... So is every filter going to do this? This is hideous. Let's
> generalize this. Michael, what's the reason for not doing this
> generically?
you leave too little context in the code chunk for me to understand your
question unambihgously
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101125/4b3d5fa4/attachment.pgp>
More information about the ffmpeg-devel
mailing list