Hi Kieran, Thank you for the patch.
On Friday 10 Feb 2017 13:30:06 Kieran Bingham wrote: > From: Kieran Bingham <[email protected]> > > Allow the user to specify an input crop in the form (X,Y)/WxH > > Signed-off-by: Kieran Bingham <[email protected]> > --- > src/gen-image.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 132 insertions(+) > > diff --git a/src/gen-image.c b/src/gen-image.c > index 31d42e0211db..088e8b26f648 100644 > --- a/src/gen-image.c > +++ b/src/gen-image.c > @@ -95,6 +95,13 @@ struct format_info { > struct format_yuv_info yuv; > }; > > +struct image_rect { > + int left; > + int top; > + unsigned int width; > + unsigned int height; > +}; > + > struct image { > const struct format_info *format; > unsigned int width; > @@ -127,6 +134,8 @@ struct options { > bool rotate; > unsigned int compose; > struct params params; > + bool crop; > + struct image_rect inputcrop; > }; > > /* ------------------------------------------------------------------------ > @@ -1076,6 +1085,25 @@ static void image_flip(const struct image *input, > struct image *output, } > > /* ------------------------------------------------------------------------ > + * Image Cropping > + */ > + > +static void image_crop(const struct image *input, const struct image > *output, > + const struct image_rect *crop) > +{ > + unsigned int offset = (crop->top * input->width + crop->left) * 3; > + const uint8_t *idata = input->data + offset; > + uint8_t *odata = output->data; > + unsigned int y; > + > + for (y = 0; y < output->height; ++y) { > + memcpy(odata, idata, output->width * 3); > + odata += output->width * 3; > + idata += input->width * 3; > + } > +} > + > +/* ------------------------------------------------------------------------ > * Look Up Table > */ > > @@ -1363,6 +1391,21 @@ static int process(const struct options *options) > input = rgb; > } > > + if (options->crop) { > + struct image *cropped; > + > + cropped = image_new(input->format, options->inputcrop.width, > + options->inputcrop.height); > + if (!cropped) { > + ret = -ENOMEM; > + goto done; > + } > + > + image_crop(input, cropped, &options->inputcrop); > + image_delete(input); > + input = cropped; > + } > + > /* Scale */ > if (options->output_width && options->output_height) { > output_width = options->output_width; > @@ -1596,6 +1639,7 @@ static void usage(const char *argv0) > printf(" or percentages ([0%% - 100%%]). Defaults to 1.0\n"); > printf("-c, --compose n Compose n copies of the image offset > by (50,50) over a black background\n"); > printf("-C, --no-chroma-average Disable chroma averaging for odd > pixels on output\n"); > + printf(" --crop (X,Y)/WxH Crop the input image\n"); The alignment is incorrect. > printf("-e, --encoding enc Set the YCbCr encoding method. Valid > values are\n"); > printf(" BT.601, REC.709, BT.2020 and > SMPTE240M\n"); > printf("-f, --format format Set the output image format\n"); > @@ -1628,11 +1672,13 @@ static void list_formats(void) > > #define OPT_HFLIP 256 > #define OPT_VFLIP 257 > +#define OPT_CROP 260 > > static struct option opts[] = { > {"alpha", 1, 0, 'a'}, > {"clu", 1, 0, 'L'}, > {"compose", 1, 0, 'c'}, > + {"crop", 1, 0, OPT_CROP}, > {"encoding", 1, 0, 'e'}, > {"format", 1, 0, 'f'}, > {"help", 0, 0, 'h'}, > @@ -1649,6 +1695,84 @@ static struct option opts[] = { > {0, 0, 0, 0} > }; > > +static int parse_crop(struct image_rect *crop, char *optarg, char **endp) The optarg argument should be made const. I would also rename it to value as it might not come from an option. > +{ > + /* (X,Y)/WxH */ > + char *endptr = optarg; > + > + if (*endptr != '(') { > + printf("Invalid crop argument\n"); How about "Invalid crop format, expected '('\n" to clearly indicate what's wrong ? > + *endp = endptr; > + return 1; > + } > + > + crop->left = strtol(endptr + 1, &endptr, 10); > + if (*endptr != ',') { > + printf("Invalid crop position\n"); > + *endp = endptr; > + return 1; > + } > + > + if (crop->left < 0) { > + printf("Invalid negative crop\n"); > + *endp = endptr - 1; > + return 1; > + } > + > + crop->top = strtol(endptr + 1, &endptr, 10); > + if (*endptr != ')') { > + printf("Invalid crop position\n"); > + *endp = endptr; > + return 1; > + } > + > + if (crop->top < 0) { > + printf("Invalid negative crop\n"); > + *endp = endptr - 1; > + return 1; > + } > + > + endptr++; > + > + if (*endptr != '/') { > + printf("Invalid crop argument\n"); > + *endp = endptr; > + return 1; > + } > + > + crop->width = strtol(endptr + 1, &endptr, 10); > + if (*endptr != 'x') { > + printf("Invalid crop size\n"); > + *endp = endptr; > + return 1; > + } > + > + crop->height = strtol(endptr + 1, &endptr, 10); > + if (*endptr != 0) { > + printf("Invalid crop size\n"); > + *endp = endptr; > + return 1; > + } > + > + return 0; > +} > + > +void print_streampos(const char *p, const char *end) This function should be static. I would rename it to parser_print_error(), "streampos" not being very explicit. > +{ > + int pos; > + > + pos = end - p + 1; > + > + if (pos < 0) > + pos = 0; > + if (pos > (int) strlen(p)) > + pos = strlen(p); > + > + printf("\n"); > + printf(" %s\n", p); > + printf(" %*s\n", pos, "^"); > +} > + > static int parse_args(struct options *options, int argc, char *argv[]) > { > char *endptr; > @@ -1809,6 +1933,14 @@ static int parse_args(struct options *options, int > argc, char *argv[]) options->vflip = true; > break; > > + case OPT_CROP: > + if (parse_crop(&options->inputcrop, optarg, &endptr)) { > + print_streampos(optarg, endptr); I would move this inside parse_crop(). I'll fix while applying, no need to resend. > + return 1; > + } > + options->crop = true; > + break; > + > default: > printf("Invalid option -%c\n", c); > printf("Run %s -h for help.\n", argv[0]); -- Regards, Laurent Pinchart
