[FFmpeg-devel] [PATCH] lavfi/geq: prefer symbolic constants
Clément Bœsch
ubitux at gmail.com
Mon May 20 00:27:36 CEST 2013
On Sun, May 19, 2013 at 11:32:39PM +0200, Stefano Sabatini wrote:
> Hopefully enhance readability.
> ---
> libavfilter/vf_geq.c | 66 ++++++++++++++++++++++++++------------------------
> 1 file changed, 34 insertions(+), 32 deletions(-)
>
> diff --git a/libavfilter/vf_geq.c b/libavfilter/vf_geq.c
> index def3956..ba07ac1 100644
> --- a/libavfilter/vf_geq.c
> +++ b/libavfilter/vf_geq.c
> @@ -42,24 +42,26 @@ typedef struct {
> int is_rgb;
> } GEQContext;
>
> +enum { Y = 0, U, V, A, G, B, R };
> +
> #define OFFSET(x) offsetof(GEQContext, x)
> #define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>
> static const AVOption geq_options[] = {
> - { "lum_expr", "set luminance expression", OFFSET(expr_str[0]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "y", "set luminance expression", OFFSET(expr_str[0]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "cb_expr", "set chroma blue expression", OFFSET(expr_str[1]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "u", "set chroma blue expression", OFFSET(expr_str[1]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "cr_expr", "set chroma red expression", OFFSET(expr_str[2]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "v", "set chroma red expression", OFFSET(expr_str[2]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "alpha_expr", "set alpha expression", OFFSET(expr_str[3]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "a", "set alpha expression", OFFSET(expr_str[3]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "red_expr", "set red expression", OFFSET(expr_str[6]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "r", "set red expression", OFFSET(expr_str[6]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "green_expr", "set green expression", OFFSET(expr_str[4]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "g", "set green expression", OFFSET(expr_str[4]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "blue_expr", "set blue expression", OFFSET(expr_str[5]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> - { "b", "set blue expression", OFFSET(expr_str[5]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "lum_expr", "set luminance expression", OFFSET(expr_str[Y]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "y", "set luminance expression", OFFSET(expr_str[Y]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "cb_expr", "set chroma blue expression", OFFSET(expr_str[U]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "u", "set chroma blue expression", OFFSET(expr_str[U]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "cr_expr", "set chroma red expression", OFFSET(expr_str[V]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "v", "set chroma red expression", OFFSET(expr_str[V]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "alpha_expr", "set alpha expression", OFFSET(expr_str[A]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "a", "set alpha expression", OFFSET(expr_str[A]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "red_expr", "set red expression", OFFSET(expr_str[R]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "r", "set red expression", OFFSET(expr_str[R]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "green_expr", "set green expression", OFFSET(expr_str[G]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "g", "set green expression", OFFSET(expr_str[G]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "blue_expr", "set blue expression", OFFSET(expr_str[B]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> + { "b", "set blue expression", OFFSET(expr_str[B]), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> {NULL},
> };
>
> @@ -103,42 +105,42 @@ static av_cold int geq_init(AVFilterContext *ctx)
> GEQContext *geq = ctx->priv;
> int plane, ret = 0;
>
> - if (!geq->expr_str[0] && !geq->expr_str[4] && !geq->expr_str[5] && !geq->expr_str[6]) {
> + if (!geq->expr_str[Y] && !geq->expr_str[G] && !geq->expr_str[B] && !geq->expr_str[R]) {
> av_log(ctx, AV_LOG_ERROR, "A luminance or RGB expression is mandatory\n");
> ret = AVERROR(EINVAL);
> goto end;
> }
> - geq->is_rgb = !geq->expr_str[0];
> + geq->is_rgb = !geq->expr_str[Y];
>
> - if ((geq->expr_str[0] || geq->expr_str[1] || geq->expr_str[2]) && (geq->expr_str[4] || geq->expr_str[5] || geq->expr_str[6])) {
> + if ((geq->expr_str[Y] || geq->expr_str[U] || geq->expr_str[V]) && (geq->expr_str[G] || geq->expr_str[B] || geq->expr_str[R])) {
> av_log(ctx, AV_LOG_ERROR, "Either YCbCr or RGB but not both must be specified\n");
> ret = AVERROR(EINVAL);
> goto end;
> }
>
> - if (!geq->expr_str[1] && !geq->expr_str[2]) {
> + if (!geq->expr_str[U] && !geq->expr_str[V]) {
> /* No chroma at all: fallback on luma */
> - geq->expr_str[1] = av_strdup(geq->expr_str[0]);
> - geq->expr_str[2] = av_strdup(geq->expr_str[0]);
> + geq->expr_str[U] = av_strdup(geq->expr_str[Y]);
> + geq->expr_str[V] = av_strdup(geq->expr_str[Y]);
> } else {
> /* One chroma unspecified, fallback on the other */
> - if (!geq->expr_str[1]) geq->expr_str[1] = av_strdup(geq->expr_str[2]);
> - if (!geq->expr_str[2]) geq->expr_str[2] = av_strdup(geq->expr_str[1]);
> + if (!geq->expr_str[U]) geq->expr_str[U] = av_strdup(geq->expr_str[V]);
> + if (!geq->expr_str[V]) geq->expr_str[V] = av_strdup(geq->expr_str[U]);
> }
>
> - if (!geq->expr_str[3])
> - geq->expr_str[3] = av_strdup("255");
> - if (!geq->expr_str[4])
> - geq->expr_str[4] = av_strdup("g(X,Y)");
> - if (!geq->expr_str[5])
> - geq->expr_str[5] = av_strdup("b(X,Y)");
> - if (!geq->expr_str[6])
> - geq->expr_str[6] = av_strdup("r(X,Y)");
> + if (!geq->expr_str[A])
> + geq->expr_str[A] = av_strdup("255");
> + if (!geq->expr_str[G])
> + geq->expr_str[G] = av_strdup("g(X,Y)");
> + if (!geq->expr_str[B])
> + geq->expr_str[B] = av_strdup("b(X,Y)");
> + if (!geq->expr_str[R])
> + geq->expr_str[R] = av_strdup("r(X,Y)");
>
> if (geq->is_rgb ?
> - (!geq->expr_str[4] || !geq->expr_str[5] || !geq->expr_str[6])
> + (!geq->expr_str[G] || !geq->expr_str[B] || !geq->expr_str[R])
> :
> - (!geq->expr_str[1] || !geq->expr_str[2] || !geq->expr_str[3])) {
> + (!geq->expr_str[U] || !geq->expr_str[V] || !geq->expr_str[A])) {
> ret = AVERROR(ENOMEM);
> goto end;
> }
You could use them in getpix as well for the plane == U || plane == V case.
Otherwise LGTM
--
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130520/8e537429/attachment.asc>
More information about the ffmpeg-devel
mailing list