image_filter enhancement

ivan babrou ibobrik at gmail.com
Tue Dec 4 05:35:07 UTC 2012


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 at 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 at 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 at 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 at nginx.org
>>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>>
>> --
>> Maxim Dounin
>> http://nginx.com/support.html
>>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at 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



More information about the nginx-devel mailing list