[FFmpeg-devel] [PATCHv3] avfilter/vf_chromakey: Add chromakey video filter

[Clément Bœsch]

On Mon, Sep 21, 2015 at 03:51:37PM +0200, Timo Rothenpieler wrote:
> >> +    if (frame->format == AV_PIX_FMT_YUVA420P || frame->format == AV_PIX_FMT_YUVA422P)
> >> +        x /= 2;
> >> +
> >> +    if (frame->format == AV_PIX_FMT_YUVA420P)
> >> +        y /= 2;
> > 
> > Why not use the usual subsampling mechanism? (using vsub/hsub etc)
> 
> That seemed more complex to me, as this filter just supports those 3
> pixel formats anyway.
> It would involve getting the pixel format description in some init
> function, calculating and storing the h/vsub values. Is there any
> benefit to that? Otherwise I think I'd prefer this solution.
> 

Benefit would be to save 2 branching and 2 div (the div can already be
replaced by a shift here though - which would have a benefit since x & y
are signed) and keep the code generic.

> >> +            frame->data[3][frame->linesize[3] * y + x] = do_chromakey_pixel(ctx, u, v);
> > 
> > You might want to check if saving a bunch of dereferencing in the inner
> > loop helps performance.
> 
> You mean getting frame->data[3] and frame->linesize[3] before the loop?

yes

> Shouldn't this be something the compiler optimises for me?

it should, but i've observe performance enhancement in similar situations.
You might want to try.

> Anyway, can easily be changed.
> 
> >> +    if (res = av_frame_make_writable(frame))
> >> +        return res;
> > 
> > You have a .needs_writable attribute somewhere for that purpose
> 
> Didn't know about that attribute.
> 
> >> +#define FIXNUM(x) ((int) ((x) * (1 << 10) + 0.5))
> > 
> > lrint()?
> 
> These defines were taken from the old colorspace.h, where they got
> removed from in 7944b0ce94.
> No idea if lrint optimizes in the same way, but as this is used only
> once during startup, it shouldn't matter here.
> 

OK

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