I've send patch for configuration semantics in separate thread.<div><br></div><div>Here's lastest version of my patch that may be applied after. Is there something else that I should fix?</div><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..9bb6e12 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 crop_offset_x;</div><div>+ ngx_uint_t crop_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 crop_offset_x;</div><div>+ ngx_uint_t crop_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->crop_offset_x);</div><div>+ offset_y = ngx_http_image_filter_get_value(r, conf->oycv,</div>
<div>+ conf->crop_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>@@ -1151,7 +1188,24 @@ ngx_http_image_filter_value(ngx_str_t *value)</div><div> </div><div> n = ngx_atoi(value->data, value->len);</div>
<div> </div><div>- if (n > 0) {</div><div>+ if (n == NGX_ERROR) {</div><div>+ if (ngx_strncmp(value->data, "left", value->len) == 0) {</div><div>+ return NGX_HTTP_IMAGE_OFFSET_LEFT;</div>
<div>+</div><div>+ } else if (ngx_strncmp(value->data, "right", value->len) == 0) {</div><div>+ return NGX_HTTP_IMAGE_OFFSET_RIGHT;</div><div>+</div><div>+ } else if (ngx_strncmp(value->data, "top", value->len) == 0) {</div>
<div>+ return NGX_HTTP_IMAGE_OFFSET_TOP;</div><div>+</div><div>+ } else if (ngx_strncmp(value->data, "bottom", value->len) == 0) {</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 +1229,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->crop_offset_x = NGX_CONF_UNSET_UINT;</div><div>+ conf->crop_offset_y = NGX_CONF_UNSET_UINT;</div><div> </div><div> return conf;</div><div> }</div><div>@@ -1230,6 +1286,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->crop_offset_x == NGX_CONF_UNSET_UINT) {</div>
<div>+ ngx_conf_merge_uint_value(conf->crop_offset_x, prev->crop_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->crop_offset_y == NGX_CONF_UNSET_UINT) {</div><div>+ ngx_conf_merge_uint_value(conf->crop_offset_y, prev->crop_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 +1555,68 @@ 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>+ ccv.zero = 1;</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->crop_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>+ ccv.zero = 1;</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->crop_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 class="gmail_extra"><br><br><div class="gmail_quote">On 18 December 2012 14:53, 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 Sun, Dec 16, 2012 at 07:55:39PM +0400, ivan babrou wrote:<br>
<br>
> On 5 December 2012 18:52, Maxim Dounin <<a href="mailto:mdounin@mdounin.ru">mdounin@mdounin.ru</a>> wrote:<br>
><br>
> > Hello!<br>
> ><br>
> > On Tue, Dec 04, 2012 at 06:58:17PM +0400, ivan babrou wrote:<br>
> ><br>
> > > ><br>
> > > > Additionally, looking at the code I tend to think it's incorrect.<br>
> > > > That is, it's pattern you've followed is incorrect, not your code.<br>
> > > > E.g. the following config will result in $arg_q incorrectly used<br>
> > > > for quality in /image/, instead of "80" explicitly set:<br>
> > > > image_filter_jpeg_quality $arg_q;<br>
> > > > location /image/ {<br>
> > > > image_filter crop $arg_w $arg_h;<br>
> > > > image_filter_jpeg_quality 80;<br>
> > > > }<br>
> > > > This needs fixing.<br>
> > ><br>
> > ><br>
> > > As I see other config variables are buggy too. Should I fix them too?<br>
> ><br>
> > It would be fine to produce a patch which fixes existing config<br>
> > variables, and the patch to add crop offsets on top of it.<br>
><br>
><br>
> Can you give me some good examples to see "good practices" in nginx<br>
> configuration handling?<br>
<br>
</div>I would recommend looking into standard ngx_conf_merge_*_value()<br>
macros from src/core/ngx_conf_file.h, and rolling your code based<br>
on them with needed modifications and/or code reuse. Something<br>
like this should work:<br>
<br>
if (conf->jpeg_quality == NGX_CONF_UNSET_UINT) {<br>
ngx_conf_merge_uint_value(conf->jpeg_quality, prev->jpeg_quality, 75);<br>
<br>
if (conf->jqcv == NULL) {<br>
conf->jqcv = prev->jqcv;<br>
<div class="HOEnZb"><div class="h5"> }<br>
}<br>
<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<br>
</div>