Message ID | bf1783fd-e339-3b59-fec1-1b3c9dcb5c7d@gmail.com |
---|---|
State | Accepted |
Commit | 658ac0672f46cef483e68440061da763e908b68a |
Headers | show |
2018-01-24 4:04 GMT+01:00 Jun Zhao <mypopydev@gmail.com>: > - procamp_params[i].type = VAProcFilterColorBalance; > - procamp_params[i].attrib = VAProcColorBalanceBrightness; > - procamp_params[i].value = map(ctx->bright, BRIGHTNESS_MIN, BRIGHTNESS_MAX, > - procamp_caps[VAProcColorBalanceBrightness-1].range.min_value, > - procamp_caps[VAProcColorBalanceBrightness-1].range.max_value); > - i++; > - procamp_params[i].type = VAProcFilterColorBalance; > - procamp_params[i].attrib = VAProcColorBalanceContrast; > - procamp_params[i].value = map(ctx->contrast, CONTRAST_MIN, CONTRAST_MAX, > - procamp_caps[VAProcColorBalanceContrast-1].range.min_value, > - procamp_caps[VAProcColorBalanceContrast-1].range.max_value); > - i++; > - procamp_params[i].type = VAProcFilterColorBalance; > - procamp_params[i].attrib = VAProcColorBalanceHue; > - procamp_params[i].value = map(ctx->hue, HUE_MIN, HUE_MAX, > - procamp_caps[VAProcColorBalanceHue-1].range.min_value, > - procamp_caps[VAProcColorBalanceHue-1].range.max_value); > - i++; > - procamp_params[i].type = VAProcFilterColorBalance; > - procamp_params[i].attrib = VAProcColorBalanceSaturation; > - procamp_params[i].value = map(ctx->saturation, SATURATION_MIN, SATURATION_MAX, > - procamp_caps[VAProcColorBalanceSaturation-1].range.min_value, > - procamp_caps[VAProcColorBalanceSaturation-1].range.max_value); > - i++; Please do not reindent these lines in the same patch that removes the conditions, this has two advantages: It makes reviewing your changes easier now, and allows somebody who looks at your change in the future to understand it more quickly. Send an independent patch for the re-indentation. Carl Eugen
On 24/01/18 03:04, Jun Zhao wrote: > > From 1d3b388c313881e931928dc4049896265919725d Mon Sep 17 00:00:00 2001 > From: Jun Zhao <jun.zhao@intel.com> > Date: Wed, 24 Jan 2018 09:28:24 +0800 > Subject: [PATCH V2 2/3] lavfi/procamp_vaapi: fix the green video issue if > without arguments. > > Fix the green output issue when use procamp_vaapi without any > arguments, now if use procamp_vaapi without any arguments, will use > the default value to setting procamp_vaapi. > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > --- > libavfilter/vf_procamp_vaapi.c | 73 ++++++++++++++++++------------------------ > 1 file changed, 31 insertions(+), 42 deletions(-) > On 24/01/18 03:04, Jun Zhao wrote: > > From 7a4a53fcc5a9b31b11818976dc02ea8e004d2c5a Mon Sep 17 00:00:00 2001 > From: Jun Zhao <jun.zhao@intel.com> > Date: Wed, 24 Jan 2018 09:32:50 +0800 > Subject: [PATCH V2 3/3] lavfi/misc_vaapi: use default value setting if without > arguments. > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > --- > libavfilter/vf_misc_vaapi.c | 64 +++++++++++++++++++++------------------------ > 1 file changed, 30 insertions(+), 34 deletions(-) > Ah right, I didn't consider the same issue on the misc filters as well. Always passing the arguments ends up being a bit simpler too, which is nice. Both LGTM, tested, applied. Thanks, - Mark
2018-01-26 0:21 GMT+01:00 Mark Thompson <sw@jkqxz.net>: > On 24/01/18 03:04, Jun Zhao wrote: >> >> From 1d3b388c313881e931928dc4049896265919725d Mon Sep 17 00:00:00 2001 >> From: Jun Zhao <jun.zhao@intel.com> >> Date: Wed, 24 Jan 2018 09:28:24 +0800 >> Subject: [PATCH V2 2/3] lavfi/procamp_vaapi: fix the green video issue if >> without arguments. >> >> Fix the green output issue when use procamp_vaapi without any >> arguments, now if use procamp_vaapi without any arguments, will use >> the default value to setting procamp_vaapi. >> >> Signed-off-by: Jun Zhao <jun.zhao@intel.com> >> --- >> libavfilter/vf_procamp_vaapi.c | 73 ++++++++++++++++++------------------------ >> 1 file changed, 31 insertions(+), 42 deletions(-) >> > On 24/01/18 03:04, Jun Zhao wrote: >> >> From 7a4a53fcc5a9b31b11818976dc02ea8e004d2c5a Mon Sep 17 00:00:00 2001 >> From: Jun Zhao <jun.zhao@intel.com> >> Date: Wed, 24 Jan 2018 09:32:50 +0800 >> Subject: [PATCH V2 3/3] lavfi/misc_vaapi: use default value setting if without >> arguments. >> >> Signed-off-by: Jun Zhao <jun.zhao@intel.com> >> --- >> libavfilter/vf_misc_vaapi.c | 64 +++++++++++++++++++++------------------------ >> 1 file changed, 30 insertions(+), 34 deletions(-) >> > > Ah right, I didn't consider the same issue on the misc filters as well. Always passing the arguments ends up being a bit simpler too, which is nice. > > Both LGTM, tested, applied. So you prefer that nobody else comments on patches? Carl Eugen
On 24/01/18 10:30, Carl Eugen Hoyos wrote: > 2018-01-24 4:04 GMT+01:00 Jun Zhao <mypopydev@gmail.com>: > >> - procamp_params[i].type = VAProcFilterColorBalance; >> - procamp_params[i].attrib = VAProcColorBalanceBrightness; >> - procamp_params[i].value = map(ctx->bright, BRIGHTNESS_MIN, BRIGHTNESS_MAX, >> - procamp_caps[VAProcColorBalanceBrightness-1].range.min_value, >> - procamp_caps[VAProcColorBalanceBrightness-1].range.max_value); >> - i++; > >> - procamp_params[i].type = VAProcFilterColorBalance; >> - procamp_params[i].attrib = VAProcColorBalanceContrast; >> - procamp_params[i].value = map(ctx->contrast, CONTRAST_MIN, CONTRAST_MAX, >> - procamp_caps[VAProcColorBalanceContrast-1].range.min_value, >> - procamp_caps[VAProcColorBalanceContrast-1].range.max_value); >> - i++; > >> - procamp_params[i].type = VAProcFilterColorBalance; >> - procamp_params[i].attrib = VAProcColorBalanceHue; >> - procamp_params[i].value = map(ctx->hue, HUE_MIN, HUE_MAX, >> - procamp_caps[VAProcColorBalanceHue-1].range.min_value, >> - procamp_caps[VAProcColorBalanceHue-1].range.max_value); >> - i++; > >> - procamp_params[i].type = VAProcFilterColorBalance; >> - procamp_params[i].attrib = VAProcColorBalanceSaturation; >> - procamp_params[i].value = map(ctx->saturation, SATURATION_MIN, SATURATION_MAX, >> - procamp_caps[VAProcColorBalanceSaturation-1].range.min_value, >> - procamp_caps[VAProcColorBalanceSaturation-1].range.max_value); >> - i++; > > Please do not reindent these lines in the same patch that removes the > conditions, this has two advantages: It makes reviewing your changes > easier now, and allows somebody who looks at your change in the > future to understand it more quickly. > Send an independent patch for the re-indentation. I don't believe this is merited. I find the change very easy to review now, and in future it will be clearer for these lines to be attributed to the patch which actually modifies them rather than an "indent after previous commit" change. Such splitting is reasonable where complex blocks change indentation levels while staying mostly or completely the same, but these short blocks do not meet such a threshold. - Mark
diff --git a/libavfilter/vf_procamp_vaapi.c b/libavfilter/vf_procamp_vaapi.c index aad76aa371..45a3120b23 100644 --- a/libavfilter/vf_procamp_vaapi.c +++ b/libavfilter/vf_procamp_vaapi.c @@ -44,8 +44,6 @@ #define SATURATION_MAX 10.0F #define SATURATION_DEFAULT 1.0F -#define EPSILON 0.00001F - typedef struct ProcampVAAPIContext { VAAPIVPPContext vpp_ctx; // must be the first field @@ -65,11 +63,6 @@ static float map(float x, float in_min, float in_max, float out_min, float out_m return (float)output; } -static int fequal(float a, float b) -{ - return fabs(a-b) < EPSILON; -} - static int procamp_vaapi_build_filter_params(AVFilterContext *avctx) { VAAPIVPPContext *vpp_ctx = avctx->priv; @@ -93,41 +86,37 @@ static int procamp_vaapi_build_filter_params(AVFilterContext *avctx) return AVERROR(EIO); } - if (!fequal(ctx->bright, BRIGHTNESS_DEFAULT)) { - procamp_params[i].type = VAProcFilterColorBalance; - procamp_params[i].attrib = VAProcColorBalanceBrightness; - procamp_params[i].value = map(ctx->bright, BRIGHTNESS_MIN, BRIGHTNESS_MAX, - procamp_caps[VAProcColorBalanceBrightness-1].range.min_value, - procamp_caps[VAProcColorBalanceBrightness-1].range.max_value); - i++; - } - - if (!fequal(ctx->contrast, CONTRAST_DEFAULT)) { - procamp_params[i].type = VAProcFilterColorBalance; - procamp_params[i].attrib = VAProcColorBalanceContrast; - procamp_params[i].value = map(ctx->contrast, CONTRAST_MIN, CONTRAST_MAX, - procamp_caps[VAProcColorBalanceContrast-1].range.min_value, - procamp_caps[VAProcColorBalanceContrast-1].range.max_value); - i++; - } - - if (!fequal(ctx->hue, HUE_DEFAULT)) { - procamp_params[i].type = VAProcFilterColorBalance; - procamp_params[i].attrib = VAProcColorBalanceHue; - procamp_params[i].value = map(ctx->hue, HUE_MIN, HUE_MAX, - procamp_caps[VAProcColorBalanceHue-1].range.min_value, - procamp_caps[VAProcColorBalanceHue-1].range.max_value); - i++; - } - - if (!fequal(ctx->saturation, SATURATION_DEFAULT)) { - procamp_params[i].type = VAProcFilterColorBalance; - procamp_params[i].attrib = VAProcColorBalanceSaturation; - procamp_params[i].value = map(ctx->saturation, SATURATION_MIN, SATURATION_MAX, - procamp_caps[VAProcColorBalanceSaturation-1].range.min_value, - procamp_caps[VAProcColorBalanceSaturation-1].range.max_value); - i++; - } + /* brightness */ + procamp_params[i].type = VAProcFilterColorBalance; + procamp_params[i].attrib = VAProcColorBalanceBrightness; + procamp_params[i].value = map(ctx->bright, BRIGHTNESS_MIN, BRIGHTNESS_MAX, + procamp_caps[VAProcColorBalanceBrightness-1].range.min_value, + procamp_caps[VAProcColorBalanceBrightness-1].range.max_value); + i++; + + /* contrast */ + procamp_params[i].type = VAProcFilterColorBalance; + procamp_params[i].attrib = VAProcColorBalanceContrast; + procamp_params[i].value = map(ctx->contrast, CONTRAST_MIN, CONTRAST_MAX, + procamp_caps[VAProcColorBalanceContrast-1].range.min_value, + procamp_caps[VAProcColorBalanceContrast-1].range.max_value); + i++; + + /* hue */ + procamp_params[i].type = VAProcFilterColorBalance; + procamp_params[i].attrib = VAProcColorBalanceHue; + procamp_params[i].value = map(ctx->hue, HUE_MIN, HUE_MAX, + procamp_caps[VAProcColorBalanceHue-1].range.min_value, + procamp_caps[VAProcColorBalanceHue-1].range.max_value); + i++; + + /* saturation */ + procamp_params[i].type = VAProcFilterColorBalance; + procamp_params[i].attrib = VAProcColorBalanceSaturation; + procamp_params[i].value = map(ctx->saturation, SATURATION_MIN, SATURATION_MAX, + procamp_caps[VAProcColorBalanceSaturation-1].range.min_value, + procamp_caps[VAProcColorBalanceSaturation-1].range.max_value); + i++; return ff_vaapi_vpp_make_param_buffers(avctx, VAProcFilterParameterBufferType,