<div dir="ltr">Btw, here's latest version with zero-termination and other fixes:<div><br></div><div><div>diff --git a/ngx_http_image_filter_module.c b/ngx_http_image_filter_module.c</div><div>index 3aee1a4..b086e3c 100644</div>
<div>--- a/ngx_http_image_filter_module.c</div><div>+++ b/ngx_http_image_filter_module.c</div><div>@@ -32,6 +32,11 @@</div><div> #define NGX_HTTP_IMAGE_GIF 2</div><div> #define NGX_HTTP_IMAGE_PNG 3</div><div>
</div>
<div>+#define NGX_HTTP_IMAGE_OFFSET_CENTER 0</div><div>+#define NGX_HTTP_IMAGE_OFFSET_LEFT 1</div><div>+#define NGX_HTTP_IMAGE_OFFSET_RIGHT 2</div><div>+#define NGX_HTTP_IMAGE_OFFSET_TOP 3</div><div>+#define NGX_HTTP_IMAGE_OFFSET_BOTTOM 4</div>
<div> </div><div> #define NGX_HTTP_IMAGE_BUFFERED 0x08</div><div> </div><div>@@ -43,11 +48,15 @@ typedef struct {</div><div> ngx_uint_t angle;</div><div> ngx_uint_t jpeg_quality;</div>
<div> ngx_uint_t sharpen;</div><div>+ ngx_uint_t offset_x;</div><div>+ ngx_uint_t offset_y;</div><div> </div><div> ngx_flag_t transparency;</div>
<div> </div><div> ngx_http_complex_value_t *wcv;</div><div> ngx_http_complex_value_t *hcv;</div><div>+ ngx_http_complex_value_t *oxcv;</div><div>+ ngx_http_complex_value_t *oycv;</div><div> ngx_http_complex_value_t *acv;</div>
<div> ngx_http_complex_value_t *jqcv;</div><div> ngx_http_complex_value_t *shcv;</div><div>@@ -66,6 +75,8 @@ typedef struct {</div><div> ngx_uint_t height;</div><div> ngx_uint_t max_width;</div>
<div> ngx_uint_t max_height;</div><div>+ ngx_uint_t offset_x;</div><div>+ ngx_uint_t offset_y;</div><div> ngx_uint_t angle;</div><div>
</div>
<div> ngx_uint_t phase;</div><div>@@ -110,6 +121,8 @@ static char *ngx_http_image_filter_jpeg_quality(ngx_conf_t *cf,</div><div> ngx_command_t *cmd, void *conf);</div><div> static char *ngx_http_image_filter_sharpen(ngx_conf_t *cf, ngx_command_t *cmd,</div>
<div> void *conf);</div><div>+static char *ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,</div><div>+ void *conf);</div><div> static ngx_int_t ngx_http_image_filter_init(ngx_conf_t *cf);</div><div>
</div><div> </div><div>@@ -150,6 +163,13 @@ static ngx_command_t ngx_http_image_filter_commands[] = {</div><div> offsetof(ngx_http_image_filter_conf_t, buffer_size),</div><div> NULL },</div><div> </div><div>
+ { ngx_string("image_filter_crop_offset"),</div><div>+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,</div><div>+ ngx_http_image_filter_offset,</div><div>+ NGX_HTTP_LOC_CONF_OFFSET,</div>
<div>+ 0,</div><div>+ NULL },</div><div>+</div><div> ngx_null_command</div><div> };</div><div> </div><div>@@ -737,7 +757,8 @@ ngx_http_image_resize(ngx_http_request_t *r, ngx_http_image_filter_ctx_t *ctx)</div>
<div> {</div><div> int sx, sy, dx, dy, ox, oy, ax, ay, size,</div><div> colors, palette, transparent, sharpen,</div><div>- red, green, blue, t;</div>
<div>+ red, green, blue, t,</div><div>+ offset_x, offset_y;</div><div> u_char *out;</div><div> ngx_buf_t *b;</div>
<div> ngx_uint_t resize;</div><div>@@ -932,8 +953,24 @@ transparent:</div><div> return NULL;</div><div> }</div><div> </div><div>- ox /= 2;</div><div>- oy /= 2;</div>
<div>+ offset_x = ngx_http_image_filter_get_value(r, conf->oxcv,</div><div>+ conf->offset_x);</div><div>+ offset_y = ngx_http_image_filter_get_value(r, conf->oycv,</div>
<div>+ conf->offset_y);</div><div>+</div><div>+ if (offset_x == NGX_HTTP_IMAGE_OFFSET_LEFT) {</div><div>+ ox = 0;</div><div>+</div><div>+ } else if (offset_x == NGX_HTTP_IMAGE_OFFSET_CENTER) {</div>
<div>+ ox /= 2;</div><div>+ }</div><div>+</div><div>+ if (offset_y == NGX_HTTP_IMAGE_OFFSET_TOP) {</div><div>+ oy = 0;</div><div>+</div><div>+ } else if (offset_y == NGX_HTTP_IMAGE_OFFSET_CENTER) {</div>
<div>+ oy /= 2;</div><div>+ }</div><div> </div><div> ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,</div><div> "image crop: %d x %d @ %d x %d",</div>
<div>@@ -1141,17 +1178,43 @@ ngx_http_image_filter_get_value(ngx_http_request_t *r,</div><div> </div><div> </div><div> static ngx_uint_t</div><div>-ngx_http_image_filter_value(ngx_str_t *value)</div><div>+ngx_http_image_filter_value(ngx_str_t *v)</div>
<div> {</div><div> ngx_int_t n;</div><div> </div><div>- if (value->len == 1 && value->data[0] == '-') {</div><div>+ if (v->len == 1 && v->data[0] == '-') {</div><div>
return (ngx_uint_t) -1;</div><div> }</div><div> </div><div>- n = ngx_atoi(value->data, value->len);</div><div>+ n = ngx_atoi(v->data, v->len);</div><div>+</div><div>+ if (n == NGX_ERROR) {</div>
<div>+</div><div>+ if (v->len == sizeof("left") - 1</div><div>+ && ngx_strncmp(v->data, "left", v->len) == 0)</div><div>+ {</div><div>+ return NGX_HTTP_IMAGE_OFFSET_LEFT;</div>
<div>+</div><div>+ } else if (v->len == sizeof("right") - 1</div><div>+ && ngx_strncmp(v->data, "right", sizeof("right") - 1) == 0)</div><div>+ {</div>
<div>+ return NGX_HTTP_IMAGE_OFFSET_RIGHT;</div><div>+</div><div>+ } else if (v->len == sizeof("top") - 1</div><div>+ && ngx_strncmp(v->data, "top", sizeof("top") - 1) == 0)</div>
<div>+ {</div><div>+ return NGX_HTTP_IMAGE_OFFSET_TOP;</div><div> </div><div>- if (n > 0) {</div><div>+ } else if (v->len == sizeof("bottom") - 1</div><div>+ && ngx_strncmp(v->data, "bottom", sizeof("bottom") - 1) == 0)</div>
<div>+ {</div><div>+ return NGX_HTTP_IMAGE_OFFSET_BOTTOM;</div><div>+</div><div>+ } else {</div><div>+ return NGX_HTTP_IMAGE_OFFSET_CENTER;</div><div>+ }</div><div>+</div><div>+ } else if (n > 0) {</div>
<div> return (ngx_uint_t) n;</div><div> }</div><div> </div><div>@@ -1175,6 +1238,8 @@ ngx_http_image_filter_create_conf(ngx_conf_t *cf)</div><div> conf->angle = NGX_CONF_UNSET_UINT;</div><div> conf->transparency = NGX_CONF_UNSET;</div>
<div> conf->buffer_size = NGX_CONF_UNSET_SIZE;</div><div>+ conf->offset_x = NGX_CONF_UNSET_UINT;</div><div>+ conf->offset_y = NGX_CONF_UNSET_UINT;</div><div> </div><div> return conf;</div><div> }</div>
<div>@@ -1230,6 +1295,24 @@ ngx_http_image_filter_merge_conf(ngx_conf_t *cf, void *parent, void *child)</div><div> ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size,</div><div> 1 * 1024 * 1024);</div>
<div> </div><div>+ if (conf->offset_x == NGX_CONF_UNSET_UINT) {</div><div>+ ngx_conf_merge_uint_value(conf->offset_x, prev->offset_x,</div><div>+ NGX_HTTP_IMAGE_OFFSET_CENTER);</div>
<div>+</div><div>+ if (conf->oxcv == NULL) {</div><div>+ conf->oxcv = prev->oxcv;</div><div>+ }</div><div>+ }</div><div>+</div><div>+ if (conf->offset_y == NGX_CONF_UNSET_UINT) {</div>
<div>+ ngx_conf_merge_uint_value(conf->offset_y, prev->offset_y,</div><div>+ NGX_HTTP_IMAGE_OFFSET_CENTER);</div><div>+</div><div>+ if (conf->oycv == NULL) {</div><div>
+ conf->oycv = prev->oycv;</div><div>+ }</div><div>+ }</div><div>+</div><div> return NGX_CONF_OK;</div><div> }</div><div> </div><div>@@ -1481,6 +1564,66 @@ ngx_http_image_filter_sharpen(ngx_conf_t *cf, ngx_command_t *cmd,</div>
<div> }</div><div> </div><div> </div><div>+static char *</div><div>+ngx_http_image_filter_offset(ngx_conf_t *cf, ngx_command_t *cmd,</div><div>+ void *conf)</div><div>+{</div><div>+ ngx_http_image_filter_conf_t *imcf = conf;</div>
<div>+</div><div>+ ngx_str_t *value;</div><div>+ ngx_http_complex_value_t cv;</div><div>+ ngx_http_compile_complex_value_t ccv;</div><div>+</div><div>+ value = cf->args->elts;</div>
<div>+</div><div>+ ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));</div><div>+</div><div>+ <a href="http://ccv.cf">ccv.cf</a> = cf;</div><div>+ ccv.value = &value[1];</div><div>+ ccv.complex_value = &cv;</div>
<div>+</div><div>+ if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {</div><div>+ return NGX_CONF_ERROR;</div><div>+ }</div><div>+</div><div>+ if (cv.lengths == NULL) {</div><div>+ imcf->offset_x = ngx_http_image_filter_value(&value[1]);</div>
<div>+</div><div>+ } else {</div><div>+ imcf->oxcv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));</div><div>+ if (imcf->oxcv == NULL) {</div><div>+ return NGX_CONF_ERROR;</div>
<div>+ }</div><div>+</div><div>+ *imcf->oxcv = cv;</div><div>+ }</div><div>+</div><div>+ ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));</div><div>+</div><div>+ <a href="http://ccv.cf">ccv.cf</a> = cf;</div>
<div>+ ccv.value = &value[2];</div><div>+ ccv.complex_value = &cv;</div><div>+</div><div>+ if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {</div><div>+ return NGX_CONF_ERROR;</div><div>+ }</div>
<div>+</div><div>+ if (cv.lengths == NULL) {</div><div>+ imcf->offset_y = ngx_http_image_filter_value(&value[2]);</div><div>+</div><div>+ } else {</div><div>+ imcf->oycv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));</div>
<div>+ if (imcf->oycv == NULL) {</div><div>+ return NGX_CONF_ERROR;</div><div>+ }</div><div>+</div><div>+ *imcf->oycv = cv;</div><div>+ }</div><div>+</div><div>+ return NGX_CONF_OK;</div>
<div>+}</div><div>+</div><div>+</div><div> static ngx_int_t</div><div> ngx_http_image_filter_init(ngx_conf_t *cf)</div><div> {</div></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 25 December 2012 20:05, Maxim Dounin <span dir="ltr"><<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello!<br>
<div class="im"><br>
On Thu, Dec 20, 2012 at 09:01:38PM +0400, ivan babrou wrote:<br>
<br>
> I've send patch for configuration semantics in separate thread.<br>
><br>
> Here's lastest version of my patch that may be applied after. Is there<br>
> something else that I should fix?<br>
<br>
</div>I would like to see clarification on how are you going to handle<br>
just bare numbers specified. As of now the code seems to don't do<br>
any special processing and seems to assume all values are correct,<br>
which might not be true and will result in some mostly arbitray<br>
crop offset selection. It might make sense to actually support<br>
percents as in Maxim Bublis's patch[1].<br>
<br>
[1] <a href="http://mailman.nginx.org/pipermail/nginx-devel/2012-April/002156.html" target="_blank">http://mailman.nginx.org/pipermail/nginx-devel/2012-April/002156.html</a><br>
<br>
[...]<br>
<div class="im"><br>
> @@ -1151,7 +1188,24 @@ ngx_http_image_filter_value(ngx_str_t *value)<br>
><br>
> n = ngx_atoi(value->data, value->len);<br>
><br>
> - if (n > 0) {<br>
> + if (n == NGX_ERROR) {<br>
> + if (ngx_strncmp(value->data, "left", value->len) == 0) {<br>
> + return NGX_HTTP_IMAGE_OFFSET_LEFT;<br>
> +<br>
> + } else if (ngx_strncmp(value->data, "right", value->len) == 0) {<br>
> + return NGX_HTTP_IMAGE_OFFSET_RIGHT;<br>
> +<br>
> + } else if (ngx_strncmp(value->data, "top", value->len) == 0) {<br>
> + return NGX_HTTP_IMAGE_OFFSET_TOP;<br>
> +<br>
> + } else if (ngx_strncmp(value->data, "bottom", value->len) == 0) {<br>
> + return NGX_HTTP_IMAGE_OFFSET_BOTTOM;<br>
> +<br>
> + } else {<br>
> + return NGX_HTTP_IMAGE_OFFSET_CENTER;<br>
> + }<br>
> +<br>
> + } else if (n > 0) {<br>
> return (ngx_uint_t) n;<br>
> }<br>
><br>
<br>
</div>The ngx_strncmp() checks are incorrect, see here:<br>
<a href="http://mailman.nginx.org/pipermail/nginx-devel/2012-December/003065.html" target="_blank">http://mailman.nginx.org/pipermail/nginx-devel/2012-December/003065.html</a><br>
<br>
You may also move them below "if (n > 0)" the check for better<br>
readability.<br>
<div class="im"><br>
> @@ -1175,6 +1229,8 @@ ngx_http_image_filter_create_conf(ngx_conf_t *cf)<br>
> conf->angle = NGX_CONF_UNSET_UINT;<br>
> conf->transparency = NGX_CONF_UNSET;<br>
> conf->buffer_size = NGX_CONF_UNSET_SIZE;<br>
> + conf->crop_offset_x = NGX_CONF_UNSET_UINT;<br>
> + conf->crop_offset_y = NGX_CONF_UNSET_UINT;<br>
<br>
</div>The "crop_" prefix probably should be dropped here for better<br>
readability (and to match other names in the code like "oxcv").<br>
<br>
[...]<br>
<div class="HOEnZb"><div class="h5"><br>
--<br>
Maxim Dounin<br>
<a href="http://nginx.com/support.html" target="_blank">http://nginx.com/support.html</a><br>
<br>
_______________________________________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>Regards, Ian Babrou<br><a href="http://bobrik.name" target="_blank">http://bobrik.name</a> <a href="http://twitter.com/ibobrik" target="_blank">http://twitter.com/ibobrik</a> skype:i.babrou
</div>