Renamed oxv and oyv to oxcv and oycv too.

diff --git a/ngx_http_image_filter_module.c b/ngx_http_image_filter_module.c
index c853c33..325140e 100644
--- a/ngx_http_image_filter_module.c
+++ b/ngx_http_image_filter_module.c
@@ -32,6 +32,11 @@
#define NGX_HTTP_IMAGE_GIF 2
#define NGX_HTTP_IMAGE_PNG 3

+#define NGX_HTTP_IMAGE_OFFSET_CENTER 0
+#define NGX_HTTP_IMAGE_OFFSET_LEFT 1
+#define NGX_HTTP_IMAGE_OFFSET_RIGHT 2
+#define NGX_HTTP_IMAGE_OFFSET_TOP 3
+#define NGX_HTTP_IMAGE_OFFSET_BOTTOM 4

#define NGX_HTTP_IMAGE_BUFFERED 0x08

@@ -43,11 +48,15 @@ typedef struct {
ngx_uint_t angle;
ngx_uint_t jpeg_quality;
ngx_uint_t sharpen;
+ ngx_uint_t crop_offset_x;
+ ngx_uint_t crop_offset_y;

ngx_flag_t transparency;

ngx_http_complex_value_t *wcv;
ngx_http_complex_value_t *hcv;
+ ngx_http_complex_value_t *oxcv;
+ ngx_http_complex_value_t *oycv;
ngx_http_complex_value_t *acv;
ngx_http_complex_value_t *jqcv;
ngx_http_complex_value_t *shcv;
@@ -66,6 +75,8 @@ typedef struct {
ngx_uint_t height;
ngx_uint_t max_width;
ngx_uint_t max_height;
+ ngx_uint_t crop_offset_x;
+ ngx_uint_t crop_offset_y;
ngx_uint_t angle;

ngx_uint_t phase;
@@ -110,6 +121,8 @@ static char
*ngx_http_image_filter_jpeg_quality(ngx_conf_t *cf,
ngx_command_t *cmd, void *conf);
static char *ngx_http_image_filter_sharpen(ngx_conf_t *cf, ngx_command_t *cmd,
void *conf);
+static char *ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,
+ void *conf);
static ngx_int_t ngx_http_image_filter_init(ngx_conf_t *cf);


@@ -150,6 +163,13 @@ static ngx_command_t ngx_http_image_filter_commands[] = {
offsetof(ngx_http_image_filter_conf_t, buffer_size),
NULL },

+ { ngx_string("image_filter_crop_offset"),
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
+ ngx_http_image_filter_offset,
+ NGX_HTTP_LOC_CONF_OFFSET,
+ 0,
+ NULL },
+
ngx_null_command
};

@@ -737,7 +757,8 @@ ngx_http_image_resize(ngx_http_request_t *r,
ngx_http_image_filter_ctx_t *ctx)
{
int sx, sy, dx, dy, ox, oy, ax, ay, size,
colors, palette, transparent, sharpen,
- red, green, blue, t;
+ red, green, blue, t,
+ crop_offset_x, crop_offset_y;
u_char *out;
ngx_buf_t *b;
ngx_uint_t resize;
@@ -932,8 +953,27 @@ transparent:
return NULL;
}

- ox /= 2;
- oy /= 2;
+ crop_offset_x = conf->crop_offset_x;
+ crop_offset_y = conf->crop_offset_y;
+
+ crop_offset_x = ngx_http_image_filter_get_value(r, conf->oxcv,
+ crop_offset_x);
+ crop_offset_y = ngx_http_image_filter_get_value(r, conf->oycv,
+ crop_offset_y);
+
+ if (crop_offset_x == NGX_HTTP_IMAGE_OFFSET_LEFT) {
+ ox = 0;
+
+ } else if (crop_offset_x == NGX_HTTP_IMAGE_OFFSET_CENTER) {
+ ox /= 2;
+ }
+
+ if (crop_offset_y == NGX_HTTP_IMAGE_OFFSET_TOP) {
+ oy = 0;
+
+ } else if (crop_offset_y == NGX_HTTP_IMAGE_OFFSET_CENTER) {
+ oy /= 2;
+ }

ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"image crop: %d x %d @ %d x %d",
@@ -1149,10 +1189,27 @@ ngx_http_image_filter_value(ngx_str_t *value)
return (ngx_uint_t) -1;
}

- n = ngx_atoi(value->data, value->len);
+ if (ngx_strcmp(value->data, "center") == 0) {
+ return NGX_HTTP_IMAGE_OFFSET_CENTER;
+
+ } else if (ngx_strcmp(value->data, "left") == 0) {
+ return NGX_HTTP_IMAGE_OFFSET_LEFT;
+
+ } else if (ngx_strcmp(value->data, "right") == 0) {
+ return NGX_HTTP_IMAGE_OFFSET_RIGHT;
+
+ } else if (ngx_strcmp(value->data, "top") == 0) {
+ return NGX_HTTP_IMAGE_OFFSET_TOP;

- if (n > 0) {
- return (ngx_uint_t) n;
+ } else if (ngx_strcmp(value->data, "bottom") == 0) {
+ return NGX_HTTP_IMAGE_OFFSET_BOTTOM;
+
+ } else {
+ n = ngx_atoi(value->data, value->len);
+
+ if (n > 0) {
+ return (ngx_uint_t) n;
+ }
}

return 0;
@@ -1175,6 +1232,8 @@ ngx_http_image_filter_create_conf(ngx_conf_t *cf)
conf->angle = NGX_CONF_UNSET_UINT;
conf->transparency = NGX_CONF_UNSET;
conf->buffer_size = NGX_CONF_UNSET_SIZE;
+ conf->crop_offset_x = NGX_CONF_UNSET_UINT;
+ conf->crop_offset_y = NGX_CONF_UNSET_UINT;

return conf;
}
@@ -1223,6 +1282,17 @@ ngx_http_image_filter_merge_conf(ngx_conf_t
*cf, void *parent, void *child)
ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size,
1 * 1024 * 1024);

+ ngx_conf_merge_uint_value(conf->crop_offset_x,
prev->crop_offset_x, NGX_HTTP_IMAGE_OFFSET_CENTER);
+ ngx_conf_merge_uint_value(conf->crop_offset_y,
prev->crop_offset_y, NGX_HTTP_IMAGE_OFFSET_CENTER);
+
+ if (conf->oxcv == NULL) {
+ conf->oxcv = prev->oxcv;
+ }
+
+ if (conf->oycv == NULL) {
+ conf->oycv = prev->oycv;
+ }
+
return NGX_CONF_OK;
}

@@ -1474,6 +1544,68 @@ ngx_http_image_filter_sharpen(ngx_conf_t *cf,
ngx_command_t *cmd,
}


+static char *
+ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,
+ void *conf)
+{
+ ngx_http_image_filter_conf_t *imcf = conf;
+
+ ngx_str_t *value;
+ ngx_http_complex_value_t cv;
+ ngx_http_compile_complex_value_t ccv;
+
+ value = cf->args->elts;
+
+ ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
+
+ ccv.cf = cf;
+ ccv.value = &value[1];
+ ccv.complex_value = &cv;
+ ccv.zero = 1;
+
+ if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
+ return NGX_CONF_ERROR;
+ }
+
+ if (cv.lengths == NULL) {
+ imcf->crop_offset_x = ngx_http_image_filter_value(&value[1]);
+
+ } else {
+ imcf->oxcv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
+ if (imcf->oxcv == NULL) {
+ return NGX_CONF_ERROR;
+ }
+
+ *imcf->oxcv = cv;
+ }
+
+ ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
+
+ ccv.cf = cf;
+ ccv.value = &value[2];
+ ccv.complex_value = &cv;
+ ccv.zero = 1;
+
+ if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
+ return NGX_CONF_ERROR;
+ }
+
+ if (cv.lengths == NULL) {
+ imcf->crop_offset_y = ngx_http_image_filter_value(&value[2]);
+
+ } else {
+ imcf->oycv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
+ if (imcf->oycv == NULL) {
+ return NGX_CONF_ERROR;
+ }
+
+ *imcf->oycv = cv;
+ }
+
+ return NGX_CONF_OK;
+}
+
+
static ngx_int_t
ngx_http_image_filter_init(ngx_conf_t *cf)
{

On 3 December 2012 21:55, ivan babrou <ibobrik@gmail.com> wrote:
> Reworked again. Looks much better now! :)
>
> I renamed configuration directive to image_filter_crop_offset.
>
> diff --git a/ngx_http_image_filter_module.c b/ngx_http_image_filter_module.c
> index c853c33..4d0ce95 100644
> --- a/ngx_http_image_filter_module.c
> +++ b/ngx_http_image_filter_module.c
> @@ -32,6 +32,11 @@
> #define NGX_HTTP_IMAGE_GIF 2
> #define NGX_HTTP_IMAGE_PNG 3
>
> +#define NGX_HTTP_IMAGE_OFFSET_CENTER 0
> +#define NGX_HTTP_IMAGE_OFFSET_LEFT 1
> +#define NGX_HTTP_IMAGE_OFFSET_RIGHT 2
> +#define NGX_HTTP_IMAGE_OFFSET_TOP 3
> +#define NGX_HTTP_IMAGE_OFFSET_BOTTOM 4
>
> #define NGX_HTTP_IMAGE_BUFFERED 0x08
>
> @@ -43,11 +48,15 @@ typedef struct {
> ngx_uint_t angle;
> ngx_uint_t jpeg_quality;
> ngx_uint_t sharpen;
> + ngx_uint_t crop_offset_x;
> + ngx_uint_t crop_offset_y;
>
> ngx_flag_t transparency;
>
> ngx_http_complex_value_t *wcv;
> ngx_http_complex_value_t *hcv;
> + ngx_http_complex_value_t *oxv;
> + ngx_http_complex_value_t *oyv;
> ngx_http_complex_value_t *acv;
> ngx_http_complex_value_t *jqcv;
> ngx_http_complex_value_t *shcv;
> @@ -66,6 +75,8 @@ typedef struct {
> ngx_uint_t height;
> ngx_uint_t max_width;
> ngx_uint_t max_height;
> + ngx_uint_t crop_offset_x;
> + ngx_uint_t crop_offset_y;
> ngx_uint_t angle;
>
> ngx_uint_t phase;
> @@ -110,6 +121,8 @@ static char
> *ngx_http_image_filter_jpeg_quality(ngx_conf_t *cf,
> ngx_command_t *cmd, void *conf);
> static char *ngx_http_image_filter_sharpen(ngx_conf_t *cf, ngx_command_t *cmd,
> void *conf);
> +static char *ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,
> + void *conf);
> static ngx_int_t ngx_http_image_filter_init(ngx_conf_t *cf);
>
>
> @@ -150,6 +163,13 @@ static ngx_command_t ngx_http_image_filter_commands[] = {
> offsetof(ngx_http_image_filter_conf_t, buffer_size),
> NULL },
>
> + { ngx_string("image_filter_crop_offset"),
> + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
> + ngx_http_image_filter_offset,
> + NGX_HTTP_LOC_CONF_OFFSET,
> + 0,
> + NULL },
> +
> ngx_null_command
> };
>
> @@ -737,7 +757,8 @@ ngx_http_image_resize(ngx_http_request_t *r,
> ngx_http_image_filter_ctx_t *ctx)
> {
> int sx, sy, dx, dy, ox, oy, ax, ay, size,
> colors, palette, transparent, sharpen,
> - red, green, blue, t;
> + red, green, blue, t,
> + crop_offset_x, crop_offset_y;
> u_char *out;
> ngx_buf_t *b;
> ngx_uint_t resize;
> @@ -932,8 +953,27 @@ transparent:
> return NULL;
> }
>
> - ox /= 2;
> - oy /= 2;
> + crop_offset_x = conf->crop_offset_x;
> + crop_offset_y = conf->crop_offset_y;
> +
> + crop_offset_x = ngx_http_image_filter_get_value(r, conf->oxv,
> + crop_offset_x);
> + crop_offset_y = ngx_http_image_filter_get_value(r, conf->oyv,
> + crop_offset_y);
> +
> + if (crop_offset_x == NGX_HTTP_IMAGE_OFFSET_LEFT) {
> + ox = 0;
> +
> + } else if (crop_offset_x == NGX_HTTP_IMAGE_OFFSET_CENTER) {
> + ox /= 2;
> + }
> +
> + if (crop_offset_y == NGX_HTTP_IMAGE_OFFSET_TOP) {
> + oy = 0;
> +
> + } else if (crop_offset_y == NGX_HTTP_IMAGE_OFFSET_CENTER) {
> + oy /= 2;
> + }
>
> ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> "image crop: %d x %d @ %d x %d",
> @@ -1149,10 +1189,27 @@ ngx_http_image_filter_value(ngx_str_t *value)
> return (ngx_uint_t) -1;
> }
>
> - n = ngx_atoi(value->data, value->len);
> + if (ngx_strcmp(value->data, "center") == 0) {
> + return NGX_HTTP_IMAGE_OFFSET_CENTER;
> +
> + } else if (ngx_strcmp(value->data, "left") == 0) {
> + return NGX_HTTP_IMAGE_OFFSET_LEFT;
> +
> + } else if (ngx_strcmp(value->data, "right") == 0) {
> + return NGX_HTTP_IMAGE_OFFSET_RIGHT;
> +
> + } else if (ngx_strcmp(value->data, "top") == 0) {
> + return NGX_HTTP_IMAGE_OFFSET_TOP;
>
> - if (n > 0) {
> - return (ngx_uint_t) n;
> + } else if (ngx_strcmp(value->data, "bottom") == 0) {
> + return NGX_HTTP_IMAGE_OFFSET_BOTTOM;
> +
> + } else {
> + n = ngx_atoi(value->data, value->len);
> +
> + if (n > 0) {
> + return (ngx_uint_t) n;
> + }
> }
>
> return 0;
> @@ -1175,6 +1232,8 @@ ngx_http_image_filter_create_conf(ngx_conf_t *cf)
> conf->angle = NGX_CONF_UNSET_UINT;
> conf->transparency = NGX_CONF_UNSET;
> conf->buffer_size = NGX_CONF_UNSET_SIZE;
> + conf->crop_offset_x = NGX_CONF_UNSET_UINT;
> + conf->crop_offset_y = NGX_CONF_UNSET_UINT;
>
> return conf;
> }
> @@ -1223,6 +1282,17 @@ ngx_http_image_filter_merge_conf(ngx_conf_t
> *cf, void *parent, void *child)
> ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size,
> 1 * 1024 * 1024);
>
> + ngx_conf_merge_uint_value(conf->crop_offset_x,
> prev->crop_offset_x, NGX_HTTP_IMAGE_OFFSET_CENTER);
> + ngx_conf_merge_uint_value(conf->crop_offset_y,
> prev->crop_offset_y, NGX_HTTP_IMAGE_OFFSET_CENTER);
> +
> + if (conf->oxv == NULL) {
> + conf->oxv = prev->oxv;
> + }
> +
> + if (conf->oyv == NULL) {
> + conf->oyv = prev->oyv;
> + }
> +
> return NGX_CONF_OK;
> }
>
> @@ -1474,6 +1544,68 @@ ngx_http_image_filter_sharpen(ngx_conf_t *cf,
> ngx_command_t *cmd,
> }
>
>
> +static char *
> +ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,
> + void *conf)
> +{
> + ngx_http_image_filter_conf_t *imcf = conf;
> +
> + ngx_str_t *value;
> + ngx_http_complex_value_t cv;
> + ngx_http_compile_complex_value_t ccv;
> +
> + value = cf->args->elts;
> +
> + ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
> +
> + ccv.cf = cf;
> + ccv.value = &value[1];
> + ccv.complex_value = &cv;
> + ccv.zero = 1;
> +
> + if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
> + return NGX_CONF_ERROR;
> + }
> +
> + if (cv.lengths == NULL) {
> + imcf->crop_offset_x = ngx_http_image_filter_value(&value[1]);
> +
> + } else {
> + imcf->oxv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
> + if (imcf->oxv == NULL) {
> + return NGX_CONF_ERROR;
> + }
> +
> + *imcf->oxv = cv;
> + }
> +
> + ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
> +
> + ccv.cf = cf;
> + ccv.value = &value[2];
> + ccv.complex_value = &cv;
> + ccv.zero = 1;
> +
> + if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
> + return NGX_CONF_ERROR;
> + }
> +
> + if (cv.lengths == NULL) {
> + imcf->crop_offset_y = ngx_http_image_filter_value(&value[2]);
> +
> + } else {
> + imcf->oyv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
> + if (imcf->oyv == NULL) {
> + return NGX_CONF_ERROR;
> + }
> +
> + *imcf->oyv = cv;
> + }
> +
> + return NGX_CONF_OK;
> +}
> +
> +
> static ngx_int_t
> ngx_http_image_filter_init(ngx_conf_t *cf)
> {
>
> On 27 November 2012 20:03, Maxim Dounin <mdounin@mdounin.ru> wrote:
>> Hello!
>>
>> On Tue, Nov 27, 2012 at 12:10:13AM +0400, ivan babrou wrote:
>>
>>> I tried to rework patch. Now it supports configuration through
>>> variables. Variables ox and oy renamed to crop_offset_x and
>>> crop_offset_y.
>>>
>>> I'm confused about this line:
>>> + value.data[value.len] = 0;
>>>
>>> Probably I don't understand something about memory management in nginx.
>>
>> See below.
>>
>>>
>>>
>>> diff --git a/ngx_http_image_filter_module.c b/ngx_http_image_filter_module.c
>>> index c853c33..15f8d17 100644
>>> --- a/ngx_http_image_filter_module.c
>>> +++ b/ngx_http_image_filter_module.c
>>> @@ -32,6 +32,14 @@
>>> #define NGX_HTTP_IMAGE_GIF 2
>>> #define NGX_HTTP_IMAGE_PNG 3
>>>
>>> +#define NGX_HTTP_IMAGE_OFFSET_TYPE_HORIZONTAL 0
>>> +#define NGX_HTTP_IMAGE_OFFSET_TYPE_VERTICAL 1
>>> +
>>> +#define NGX_HTTP_IMAGE_OFFSET_CENTER 0
>>> +#define NGX_HTTP_IMAGE_OFFSET_LEFT 1
>>> +#define NGX_HTTP_IMAGE_OFFSET_RIGHT 2
>>> +#define NGX_HTTP_IMAGE_OFFSET_TOP 3
>>> +#define NGX_HTTP_IMAGE_OFFSET_BOTTOM 4
>>>
>>> #define NGX_HTTP_IMAGE_BUFFERED 0x08
>>>
>>> @@ -43,11 +51,15 @@ typedef struct {
>>> ngx_uint_t angle;
>>> ngx_uint_t jpeg_quality;
>>> ngx_uint_t sharpen;
>>> + ngx_uint_t crop_offset_x;
>>> + ngx_uint_t crop_offset_y;
>>>
>>> ngx_flag_t transparency;
>>>
>>> ngx_http_complex_value_t *wcv;
>>> ngx_http_complex_value_t *hcv;
>>> + ngx_http_complex_value_t *oxv;
>>> + ngx_http_complex_value_t *oyv;
>>
>> Probably you mean "oxcv", "oycv" - "cv" here is abbreviation of
>> "complex value".
>>
>>> ngx_http_complex_value_t *acv;
>>> ngx_http_complex_value_t *jqcv;
>>> ngx_http_complex_value_t *shcv;
>>> @@ -66,6 +78,8 @@ typedef struct {
>>> ngx_uint_t height;
>>> ngx_uint_t max_width;
>>> ngx_uint_t max_height;
>>> + ngx_uint_t crop_offset_x;
>>> + ngx_uint_t crop_offset_y;
>>> ngx_uint_t angle;
>>>
>>> ngx_uint_t phase;
>>> @@ -98,6 +112,15 @@ static u_char
>>> *ngx_http_image_out(ngx_http_request_t *r, ngx_uint_t type,
>>> static void ngx_http_image_cleanup(void *data);
>>> static ngx_uint_t ngx_http_image_filter_get_value(ngx_http_request_t *r,
>>> ngx_http_complex_value_t *cv, ngx_uint_t v);
>>> +static void
>>> +ngx_http_image_filter_crop_offset(ngx_http_request_t *r,
>>> + ngx_http_complex_value_t *cv, ngx_uint_t t, ngx_uint_t *v);
>>> +static void
>>> +ngx_http_image_filter_vertical_crop_offset(ngx_str_t *value,
>>> + ngx_uint_t *v);
>>> +static void
>>> +ngx_http_image_filter_horizontal_crop_offset(ngx_str_t *value,
>>> + ngx_uint_t *v);
>>> static ngx_uint_t ngx_http_image_filter_value(ngx_str_t *value);
>>>
>>>
>>> @@ -110,6 +133,8 @@ static char
>>> *ngx_http_image_filter_jpeg_quality(ngx_conf_t *cf,
>>> ngx_command_t *cmd, void *conf);
>>> static char *ngx_http_image_filter_sharpen(ngx_conf_t *cf, ngx_command_t *cmd,
>>> void *conf);
>>> +static char *ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,
>>> + void *conf);
>>> static ngx_int_t ngx_http_image_filter_init(ngx_conf_t *cf);
>>>
>>>
>>> @@ -150,6 +175,13 @@ static ngx_command_t ngx_http_image_filter_commands[] = {
>>> offsetof(ngx_http_image_filter_conf_t, buffer_size),
>>> NULL },
>>>
>>> + { ngx_string("image_filter_offset"),
>>> + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
>>> + ngx_http_image_filter_offset,
>>> + NGX_HTTP_LOC_CONF_OFFSET,
>>> + 0,
>>> + NULL },
>>> +
>>> ngx_null_command
>>> };
>>>
>>> @@ -529,6 +561,22 @@ ngx_http_image_process(ngx_http_request_t *r)
>>> return NULL;
>>> }
>>>
>>> + if (conf->oxv == NULL) {
>>> + ctx->crop_offset_x = conf->crop_offset_x;
>>> + } else {
>>> + ngx_http_image_filter_crop_offset(r, conf->oxv,
>>> +
>>> NGX_HTTP_IMAGE_OFFSET_TYPE_HORIZONTAL,
>>> + &ctx->crop_offset_x);
>>> + }
>>> +
>>> + if (conf->oyv == NULL) {
>>> + ctx->crop_offset_y = conf->crop_offset_y;
>>> + } else {
>>> + ngx_http_image_filter_crop_offset(r, conf->oyv,
>>> + NGX_HTTP_IMAGE_OFFSET_TYPE_VERTICAL,
>>> + &ctx->crop_offset_y);
>>> + }
>>> +
>>
>> 1) You want to follow pattern in the previous code, i.e.
>> foo = ngx_http_image_filter_get_value(r, conf->foocv, conf->foo);
>>
>> (With additional support of top/bottom/left/right literals.)
>>
>> 2) I don't think you need to calculate offsets to decide whether
>> image is ok as is or we need to resize it. Hence it's should
>> be ok to move the code to where it's needed.
>>
>> 3) With (2) fixed you probably don't need variables in ctx
>> anymore.
>>
>>> if (rc == NGX_OK
>>> && ctx->width <= ctx->max_width
>>> && ctx->height <= ctx->max_height
>>> @@ -932,8 +980,17 @@ transparent:
>>> return NULL;
>>> }
>>>
>>> - ox /= 2;
>>> - oy /= 2;
>>> + if (ctx->crop_offset_x == NGX_HTTP_IMAGE_OFFSET_LEFT) {
>>> + ox = 0;
>>> + } else if (ctx->crop_offset_x == NGX_HTTP_IMAGE_OFFSET_CENTER) {
>>> + ox /= 2;
>>> + }
>>> +
>>> + if (ctx->crop_offset_y == NGX_HTTP_IMAGE_OFFSET_TOP) {
>>> + oy = 0;
>>> + } else if (ctx->crop_offset_y == NGX_HTTP_IMAGE_OFFSET_CENTER) {
>>> + oy /= 2;
>>> + }
>>
>> Note: this needs empty lines before "} else if ..." to match
>> style. It should be changed anyway though, due to reasons
>> outlined above.
>>
>>>
>>> ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
>>> "image crop: %d x %d @ %d x %d",
>>> @@ -1139,6 +1196,52 @@ ngx_http_image_filter_get_value(ngx_http_request_t *r,
>>> return ngx_http_image_filter_value(&val);
>>> }
>>>
>>> +static void
>>> +ngx_http_image_filter_crop_offset(ngx_http_request_t *r,
>>> + ngx_http_complex_value_t *cv, ngx_uint_t t, ngx_uint_t *v)
>>> +{
>>> + ngx_str_t value;
>>> +
>>> + if (ngx_http_complex_value(r, cv, &value) != NGX_OK) {
>>> + return;
>>> + }
>>> +
>>> + value.data[value.len] = 0;
>>
>> This is incorrect and will result in memory corruption.
>>
>> If you need null-terminated string, right aproach is to set
>> ccv.zero flag while calling ngx_http_compile_complex_value(). On
>> the other hand, it might be good idea to don't assume
>> null-termination instead.
>>
>>> +
>>> + if (t == NGX_HTTP_IMAGE_OFFSET_TYPE_HORIZONTAL) {
>>> + ngx_http_image_filter_horizontal_crop_offset(&value, v);
>>> + } else if (t == NGX_HTTP_IMAGE_OFFSET_TYPE_VERTICAL) {
>>> + ngx_http_image_filter_vertical_crop_offset(&value, v);
>>> + }
>>> +}
>>> +
>>> +
>>> +static void
>>> +ngx_http_image_filter_horizontal_crop_offset(ngx_str_t *value,
>>> + ngx_uint_t *v)
>>> +{
>>> + if (ngx_strcmp(value->data, "center") == 0) {
>>> + *v = NGX_HTTP_IMAGE_OFFSET_CENTER;
>>> + } else if (ngx_strcmp(value->data, "left") == 0) {
>>> + *v = NGX_HTTP_IMAGE_OFFSET_LEFT;
>>> + } else if (ngx_strcmp(value->data, "right") == 0) {
>>> + *v = NGX_HTTP_IMAGE_OFFSET_RIGHT;
>>> + }
>>> +}
>>> +
>>> +static void
>>> +ngx_http_image_filter_vertical_crop_offset(ngx_str_t *value,
>>> + ngx_uint_t *v)
>>> +{
>>> + if (ngx_strcmp(value->data, "center") == 0) {
>>> + *v = NGX_HTTP_IMAGE_OFFSET_CENTER;
>>> + } else if (ngx_strcmp(value->data, "top") == 0) {
>>> + *v = NGX_HTTP_IMAGE_OFFSET_TOP;
>>> + } else if (ngx_strcmp(value->data, "bottom") == 0) {
>>> + *v = NGX_HTTP_IMAGE_OFFSET_BOTTOM;
>>> + }
>>> +}
>>> +
>>
>> I don't think we need so many functions to do this simple task.
>> It would be good idea simplify this.
>>
>> The NGX_HTTP_IMAGE_OFFSET_TYPE_HORIZONTAL and
>> NGX_HTTP_IMAGE_OFFSET_TYPE_VERTICAL seem to be unneeded, just a
>> boolean value would do the trick.
>>
>>> static ngx_uint_t
>>> ngx_http_image_filter_value(ngx_str_t *value)
>>> @@ -1175,6 +1278,8 @@ ngx_http_image_filter_create_conf(ngx_conf_t *cf)
>>> conf->angle = NGX_CONF_UNSET_UINT;
>>> conf->transparency = NGX_CONF_UNSET;
>>> conf->buffer_size = NGX_CONF_UNSET_SIZE;
>>> + conf->crop_offset_x = NGX_CONF_UNSET_UINT;
>>> + conf->crop_offset_y = NGX_CONF_UNSET_UINT;
>>>
>>> return conf;
>>> }
>>> @@ -1223,6 +1328,17 @@ ngx_http_image_filter_merge_conf(ngx_conf_t
>>> *cf, void *parent, void *child)
>>> ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size,
>>> 1 * 1024 * 1024);
>>>
>>> + ngx_conf_merge_uint_value(conf->crop_offset_x,
>>> prev->crop_offset_x, NGX_HTTP_IMAGE_OFFSET_CENTER);
>>> + ngx_conf_merge_uint_value(conf->crop_offset_y,
>>> prev->crop_offset_y, NGX_HTTP_IMAGE_OFFSET_CENTER);
>>> +
>>> + if (conf->oxv == NULL) {
>>> + conf->oxv = prev->oxv;
>>> + }
>>> +
>>> + if (conf->oyv == NULL) {
>>> + conf->oyv = prev->oyv;
>>> + }
>>> +
>>
>> Style: please keep lines shorter than 80 chars.
>>
>> Additionally, looking at the code I tend to think it's incorrect.
>> That is, it's pattern you've followed is incorrect, not your code.
>> E.g. the following config will result in $arg_q incorrectly used
>> for quality in /image/, instead of "80" explicitly set:
>>
>> image_filter_jpeg_quality $arg_q;
>>
>> location /image/ {
>> image_filter crop $arg_w $arg_h;
>> image_filter_jpeg_quality 80;
>> }
>>
>> This needs fixing.
>>
>>> return NGX_CONF_OK;
>>> }
>>>
>>> @@ -1474,6 +1590,64 @@ ngx_http_image_filter_sharpen(ngx_conf_t *cf,
>>> ngx_command_t *cmd,
>>> }
>>>
>>>
>>> +static char *
>>> +ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,
>>> + void *conf)
>>> +{
>>> + ngx_http_image_filter_conf_t *imcf = conf;
>>> +
>>> + ngx_str_t *value;
>>> + ngx_http_complex_value_t cv;
>>> + ngx_http_compile_complex_value_t ccv;
>>> +
>>> + value = cf->args->elts;
>>> +
>>> + ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
>>> +
>>> + ccv.cf = cf;
>>> + ccv.value = &value[1];
>>> + ccv.complex_value = &cv;
>>> +
>>> + if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
>>> + return NGX_CONF_ERROR;
>>> + }
>>> +
>>> + if (cv.lengths == NULL) {
>>> + ngx_http_image_filter_horizontal_crop_offset(&value[1],
>>> &imcf->crop_offset_x);
>>> + } else {
>>> + imcf->oxv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
>>> + if (imcf->oxv == NULL) {
>>> + return NGX_CONF_ERROR;
>>> + }
>>> +
>>> + *imcf->oxv = cv;
>>> + }
>>> +
>>> + ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
>>> +
>>> + ccv.cf = cf;
>>> + ccv.value = &value[2];
>>> + ccv.complex_value = &cv;
>>> +
>>> + if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
>>> + return NGX_CONF_ERROR;
>>> + }
>>> +
>>> + if (cv.lengths == NULL) {
>>> + ngx_http_image_filter_vertical_crop_offset(&value[2],
>>> &imcf->crop_offset_y);
>>> + } else {
>>> + imcf->oyv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
>>> + if (imcf->oyv == NULL) {
>>> + return NGX_CONF_ERROR;
>>> + }
>>> +
>>> + *imcf->oyv = cv;
>>> + }
>>> +
>>> + return NGX_CONF_OK;
>>> +}
>>> +
>>> +
>>> static ngx_int_t
>>> ngx_http_image_filter_init(ngx_conf_t *cf)
>>> {
>>>
>>>
>>> On 15 November 2012 03:47, Maxim Dounin <mdounin@mdounin.ru> wrote:
>>> > offsets via variables. On the other hand, his patch asks for
>>>
>>>
>>>
>>> --
>>> Regards, Ian Babrou
>>> http://bobrik.name http://twitter.com/ibobrik skype:i.babrou
>>>
>>> _______________________________________________
>>> nginx-devel mailing list
>>> nginx-devel@nginx.org
>>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>>
>> --
>> Maxim Dounin
>> http://nginx.com/support.html
>>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel@nginx.org
>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
>
>
> --
> Regards, Ian Babrou
> http://bobrik.name http://twitter.com/ibobrik skype:i.babrou



--
Regards, Ian Babrou
http://bobrik.name http://twitter.com/ibobrik skype:i.babrou

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel