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

[Timo Rothenpieler]

>> +    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.

>> +            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?
Shouldn't this be something the compiler optimises for me?
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...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150921/23ecde1e/attachment.sig>