[FFmpeg-devel] [PATCH] Port MPlayer 2xSaI filter to libavfilter

[Reimar Döffinger]

On Fri, Nov 26, 2010 at 06:45:38PM +1000, Nielkie wrote:
> Yeah at the time it seemed like a good idea to reproduce the "quirks" of the
> original code :\. Thanks, updated.

Well, there is some advantage to porting the filters unchanged, but
since the code was already quite far from that...

> +static uint32_t readPixel(int bytesPerPixel, uint8_t *src, int offset)

I'd say a "inline" might be advisable

> +    switch(bytesPerPixel) {
> +    case 4:
> +        return AV_RN32A(src + 4*offset);
> +    case 3:
> +        return AV_RL24(src + 3*offset);
> +    case 2:
> +        return AV_RN16(src + 2*offset);
> +    }
> +    return 0;

This should cause one useless branch, just to have a 0 return
value in a case that should never happen.

> +#define GET_RESULT(A, B, C, D) ((A != C || A != D) - (B != C || B != D))
> +
> +#define INTERPOLATE(A, B) (((A & c->colorMask) >> 1) + ((B & c->colorMask) >> 1) + (A & B & c->lowPixelMask))
> +
> +/* Interpolate two rgb colors with weights 3 and 1 */
> +#define INTERPOLATE_3_1(A, B) ((A & c->qcolorMask) >> 2)*3 + ((B & c->qcolorMask) >> 2) \
> +                               + ((((A & c->qlowPixelMask)*3 + (B & c->qlowPixelMask)) >> 2) & c->qlowPixelMask)
> +
> +/* Reads from the current source color neighborhood. */
> +#define GET_COLOR(mx, my) (readPixel(c->bytesPerPixel, src_line[my], FFMIN(x+mx, width-1)))

Macro arguments should be enclosed by () where "necessary". "Necessary"
in this case means everywhere except the src_line[my] in this case.
I suspect that a lot could be done for readability e.g. by using
[4][4] instead of [16] for color, but maybe it's better to get it in first.