[FFmpeg-devel] [PATCH V2 2/3] lavfi/procamp_vaapi: fix the green video issue if without arguments.

[Mark Thompson]

On 24/01/18 10:30, Carl Eugen Hoyos wrote:
> 2018-01-24 4:04 GMT+01:00 Jun Zhao <mypopydev at 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