On Thu, Jan 07, 2016 at 06:14:24PM -0300, James Almer wrote: > On 1/6/2016 6:13 AM, Anton Khirnov wrote: > > Quoting Vittorio Giovara (2016-01-04 16:40:23) > >> From: Clément Bœsch <[email protected]> > >> > >> This macro is faster when shift is constant. > >> > >> Signed-off-by: Vittorio Giovara <[email protected]> > >> --- > >> This macro is needed by the cfhd decoder and it slightly improves > >> performance where it is used. > >> Vittorio > >> > >> libavcodec/ffv1dec.c | 4 ++-- > >> libavcodec/ffv1enc.c | 4 ++-- > >> libavcodec/mimic.c | 4 ++-- > >> libavfilter/vf_framepack.c | 4 ++-- > >> libavutil/common.h | 4 ++++ > >> libavutil/frame.c | 2 +- > >> libavutil/imgutils.c | 2 +- > >> libswscale/rgb2rgb_template.c | 8 ++++---- > >> libswscale/swscale.c | 6 +++--- > >> libswscale/swscale_unscaled.c | 6 +++--- > >> libswscale/utils.c | 10 +++++----- > >> libswscale/x86/rgb2rgb_template.c | 8 ++++---- > >> 12 files changed, 33 insertions(+), 29 deletions(-) > >> > >> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c > >> index d32da60..3f93044 100644 > >> --- a/libavcodec/ffv1dec.c > >> +++ b/libavcodec/ffv1dec.c > >> @@ -375,8 +375,8 @@ static int decode_slice(AVCodecContext *c, void *arg) > >> > >> av_assert1(width && height); > >> if (f->colorspace == 0) { > >> - const int chroma_width = -((-width) >> f->chroma_h_shift); > >> - const int chroma_height = -((-height) >> f->chroma_v_shift); > >> + const int chroma_width = FF_CEIL_RSHIFT(width, > >> f->chroma_h_shift); > >> + const int chroma_height = FF_CEIL_RSHIFT(height, > >> f->chroma_v_shift); > >> const int cx = x >> f->chroma_h_shift; > >> const int cy = y >> f->chroma_v_shift; > >> decode_plane(fs, p->data[0] + ps * x + y * p->linesize[0], width, > >> diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c > >> index 0158605..1430564 100644 > >> --- a/libavcodec/ffv1enc.c > >> +++ b/libavcodec/ffv1enc.c > >> @@ -897,8 +897,8 @@ static int encode_slice(AVCodecContext *c, void *arg) > >> } > >> > >> if (f->colorspace == 0) { > >> - const int chroma_width = -((-width) >> f->chroma_h_shift); > >> - const int chroma_height = -((-height) >> f->chroma_v_shift); > >> + const int chroma_width = FF_CEIL_RSHIFT(width, > >> f->chroma_h_shift); > >> + const int chroma_height = FF_CEIL_RSHIFT(height, > >> f->chroma_v_shift); > >> const int cx = x >> f->chroma_h_shift; > >> const int cy = y >> f->chroma_v_shift; > >> > >> diff --git a/libavcodec/mimic.c b/libavcodec/mimic.c > >> index b8b3285..1bc2d85 100644 > >> --- a/libavcodec/mimic.c > >> +++ b/libavcodec/mimic.c > >> @@ -392,8 +392,8 @@ static int mimic_decode_frame(AVCodecContext *avctx, > >> void *data, > >> avctx->height = height; > >> avctx->pix_fmt = AV_PIX_FMT_YUV420P; > >> for (i = 0; i < 3; i++) { > >> - ctx->num_vblocks[i] = -((-height) >> (3 + !!i)); > >> - ctx->num_hblocks[i] = width >> (3 + !!i); > >> + ctx->num_vblocks[i] = FF_CEIL_RSHIFT(height, 3 + !!i); > >> + ctx->num_hblocks[i] = width >> (3 + !!i); > >> } > >> } else if (width != ctx->avctx->width || height != > >> ctx->avctx->height) { > >> avpriv_request_sample(avctx, "Resolution changing"); > >> diff --git a/libavfilter/vf_framepack.c b/libavfilter/vf_framepack.c > >> index f3bb6b3..631fb08 100644 > >> --- a/libavfilter/vf_framepack.c > >> +++ b/libavfilter/vf_framepack.c > >> @@ -158,8 +158,8 @@ static void horizontal_frame_pack(AVFilterLink > >> *outlink, > >> > >> for (plane = 0; plane < s->pix_desc->nb_components; plane++) { > >> if (plane == 1 || plane == 2) { > >> - length = -(-(out->width / 2) >> > >> s->pix_desc->log2_chroma_w); > >> - lines = -(-(out->height) >> > >> s->pix_desc->log2_chroma_h); > >> + length = FF_CEIL_RSHIFT(out->width / 2, > >> s->pix_desc->log2_chroma_w); > >> + lines = FF_CEIL_RSHIFT(out->height, > >> s->pix_desc->log2_chroma_h); > >> } > >> for (i = 0; i < lines; i++) { > >> int j; > >> diff --git a/libavutil/common.h b/libavutil/common.h > >> index 7a43ccf..ce13844 100644 > >> --- a/libavutil/common.h > >> +++ b/libavutil/common.h > >> @@ -50,6 +50,10 @@ > >> #define RSHIFT(a,b) ((a) > 0 ? ((a) + ((1<<(b))>>1))>>(b) : ((a) + > >> ((1<<(b))>>1)-1)>>(b)) > >> /* assume b>0 */ > >> #define ROUNDED_DIV(a,b) (((a)>0 ? (a) + ((b)>>1) : (a) - ((b)>>1))/(b)) > >> +/* assume a>0 and b>0 */ > >> +#define FF_CEIL_RSHIFT(a,b) (!av_builtin_constant_p(b) ? -((-(a)) >> (b)) > >> \ > >> + : ((a) + (1 << > >> (b)) - 1) >> (b)) > > > > This is a public header, so this should probably be AV-prefixed. And > > there should be the bump/apichanges dance. > > There are several other FF* macros in this header already... >
The reason I initially didn't "export" it officially was because I didn't trust so much the av_builtin_constant_p (defined to __builtin_constant_p if __GNUC__ is defined and 0 otherwise). And this is also why I didn't change the documentation of the chroma width/height computation so users don't use it immediately. TBH, nowadays I considered it sufficiently safe, but given how often end users can end up using it, I wanted to make sure it was designed as appropriate. Note1: we actually recently had an issue with __builtin_constant_p and some assembly, unrelated to this macro though. Note2: the range is actually larger than documented, see ff recent changes -- Clément B.
signature.asc
Description: PGP signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
