[FFmpeg-devel] [PATCH] avfilter/fade: add color option.

[Clément Bœsch]

On Sat, Nov 09, 2013 at 12:48:38AM +0100, Lukasz M wrote:
> On 9 November 2013 00:08, Clément Bœsch <u at pkh.me> wrote:
> 
> > Fixes Ticket #1822.
> > ---
> >  doc/filters.texi      | 13 +++++++++----
> >  libavfilter/vf_fade.c | 50
> > ++++++++++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 57 insertions(+), 6 deletions(-)
> >
> > +static int filter_slice_rgb(AVFilterContext *ctx, void *arg, int jobnr,
> > +                            int nb_jobs)
> > +{
> > +    FadeContext *s = ctx->priv;
> > +    AVFrame *frame = arg;
> > +    int slice_start = (frame->height *  jobnr   ) / nb_jobs;
> > +    int slice_end   = (frame->height * (jobnr+1)) / nb_jobs;
> > +    int i, j;
> > +
> > +    const uint8_t r  = s->rgba_map[R];
> > +    const uint8_t g  = s->rgba_map[G];
> > +    const uint8_t b  = s->rgba_map[B];
> > +    const uint8_t a  = s->rgba_map[A];
> > +    const uint8_t *c = s->color_rgba;
> > +
> > +    for (i = slice_start; i < slice_end; i++) {
> > +        uint8_t *p = frame->data[0] + i * frame->linesize[0];
> > +        for (j = 0; j < frame->width; j++) {
> > +#define INTERP(layer, value) av_clip_uint8(((c[value]<<16) +
> > ((int)p[layer] - (int)c[value]) * s->factor + (1<<15)) >> 16)
> > +            p[r] = INTERP(r, 0);
> > +            p[g] = INTERP(g, 1);
> > +            p[b] = INTERP(b, 2);
> > +            if (s->alpha)
> >
> 
> This check is done a lot of times with the same result. Maybe move it
> outside inner loop and implement loop twice: for 3 and 4 bytes per pixel
> 

I tried this:

#define INTERP(layer, value) av_clip_uint8(((c[value]<<16) + ((int)p[layer] - (int)c[value]) * s->factor + (1<<15)) >> 16)
#define FILTER_SLICE_RGB(do_alpha, step) do { \
    for (i = slice_start; i < slice_end; i++) { \
        uint8_t *p = frame->data[0] + i * frame->linesize[0]; \
        for (j = 0; j < frame->width; j++) { \
            p[r] = INTERP(r, 0); \
            p[g] = INTERP(g, 1); \
            p[b] = INTERP(b, 2); \
            if (do_alpha) \
                p[a] = INTERP(a, 3); \
            p += step; \
        } \
    } \
} while (0)

    if (s->alpha) FILTER_SLICE_RGB(1, 4);
    else          FILTER_SLICE_RGB(0, s->bpp);

Before:
  24512648 decicycles in filter_slice_rgb, 32767 runs, 1 skips
After:
  22688265 decicycles in filter_slice_rgb, 32768 runs, 0 skips

(with no alpha)

Do you think it's worth the change? If so, consider the code changed locally.

> 
> > +                p[a] = INTERP(a, 3);
> > +            p += s->bpp;
> >
> 
> I previous remark applied than can be constant +3/+4 maybe
> 

Having alpha does not mean it will be +3/+4 (because alpha filtering can
be disabled, or if later we add rgb0 and similar pix fmt). So in the case
where there is no alpha (which is the common case), I can't skip this.

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131109/bf1c93df/attachment.asc>