[FFmpeg-devel] [PATCHv3] avfilter/vf_chromakey: Add chromakey video filter
u at pkh.me
Mon Sep 21 16:00:39 CEST 2015
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[frame->linesize * 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 and frame->linesize before the loop?
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 473 bytes
Desc: not available
More information about the ffmpeg-devel