On Mon, 26 Jan 2015 11:31:49 +0100
Christophe Gisquet <christophe.gisq...@gmail.com> wrote:
> Hi,
>
> so my comments are not aimed at that specific patch at all, but when looking
> at:
>
> 2015-01-24 22:06 GMT+01:00 Paul B Mahol <one...@gmail.com>:
> > +static void process_c(EQParameters *param, uint8_t *dst, int dst_stride,
> > + uint8_t *src, int src_stride, int w, int h)
> > +{
> > + int x, y, pel;
> > + int contrast, brightness;
> > +
> > + contrast = (int) (param->contrast * 256 * 16);
> > + brightness = ((int) (100.0 * param->brightness + 100.0) * 511) / 200 -
> > 128 - contrast / 32;
> > +
> > + for (y = 0; y < h; y++) {
> > + for (x = 0; x < w; x++) {
> > + pel = ((src[y * src_stride + x] * contrast) >> 12) +
> > brightness;
> > +
> > + if (pel & 768)
> > + pel = (-pel) >> 31;
> > +
> > + dst[y * dst_stride + x] = pel;
> > + }
> > + }
> > +}
>
> versus
>
> > +static void process_MMX(EQParameters *param, uint8_t *dst, int dst_stride,
> > + uint8_t *src, int src_stride, int w, int h)
> > +{
> [...]
> > +
> > + while (h--) {
> [...]
> > + for (i = w&7; i; i--) {
> > + pel = ((*src++ * contrast) >> 12) + brightness;
> > + if (pel & 768)
> > + pel = (-pel) >> 31;
> > + *dst++ = pel;
> > + }
> > + }
>
> ... what's the sense in this?
>
> The MMX function looks broken, as if the "overflow" check would only
> apply to the end of the lines. And iirc that doesn't pass fate, as C
> and MMX produces different output. (see my reply in some thread where
> I also display the CRC of the output sequence)
But it's faster!!!!!!!!!111111111111
> Overall, why 768? And not eg ~255? or "<0" ? Sorry for being dense and
> not understanding what's done here. To me, it looks like just a normal
> clipping is needed.
>
> So maybe that thing should be checked by people that have a clue on
> what that filter attempts to do, or that have a purpose in using it.
> Then make that valid for MMX, and have the C match it.
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
