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.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
