On Fri, Apr 08, 2016 at 01:13:09PM +0000, [email protected] wrote:
> This adds a crop tool to farbfeld, the use is: ffcrop x y w h.
> 
> What do you think?

Cool idea, I think farbfeld should have some useful tools included, maybe
in a separate tools/ directory or something.

Another tool I like is farbfeld-resize:
https://github.com/ender672/farbfeld-resize

Some notes inline in the patch:

> From fc0479232da65ca60119b6131a11e6aca4273d11 Mon Sep 17 00:00:00 2001
> From: rain1 <[email protected]>
> Date: Fri, 8 Apr 2016 13:08:47 +0000
> Subject: [PATCH] Added crop tool.
> 
> ---
>  Makefile |   2 +-
>  ffcrop.c | 107 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 108 insertions(+), 1 deletion(-)
>  create mode 100644 ffcrop.c
> 
> diff --git a/Makefile b/Makefile
> index 21b82d9..bf5b40f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2,7 +2,7 @@
>  # See LICENSE file for copyright and license details
>  include config.mk
>  
> -BIN = png2ff ff2png jpg2ff ff2jpg ff2ppm
> +BIN = png2ff ff2png jpg2ff ff2jpg ff2ppm ffcrop
>  SCRIPTS = 2ff
>  SRC = ${BIN:=.c}
>  HDR = arg.h
> diff --git a/ffcrop.c b/ffcrop.c
> new file mode 100644
> index 0000000..03e162c
> --- /dev/null
> +++ b/ffcrop.c
> @@ -0,0 +1,107 @@
> +/* See LICENSE file for copyright and license details. */
> +#include <arpa/inet.h>
> +
> +#include <errno.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <png.h>
> +
> +static char *argv0;
> +
> +int
> +main(int argc, char *argv[])
> +{
> +     size_t rowlen;
> +     uint32_t hdr[4], width, height, i, tmp32;
> +     uint16_t *row;
> +     uint32_t crop_x, crop_y, crop_w, crop_h;
> +     size_t crop_skiplen, crop_rowlen;
> +
> +     argv0 = argv[0], argc--, argv++;
> +
> +     if (argc != 4) {
> +             fprintf(stderr, "usage: %s x y w h\n", argv0);
> +             return 1;
> +     }
> +
> +     crop_x = strtol(argv[0], NULL, 10);
> +     crop_y = strtol(argv[1], NULL, 10);
> +     crop_w = strtol(argv[2], NULL, 10);
> +     crop_h = strtol(argv[3], NULL, 10);
> +

I think these need to be checked (errno and whole string is a number).
Also width and height should be > 0. Note also that strtol() parses signed
numbers and it will be cast to uint32_t.

> +     /* header */
> +     if (fread(hdr, sizeof(*hdr), 4, stdin) != 4) {
> +             goto readerr;
> +     }
> +     if (memcmp("farbfeld", hdr, sizeof("farbfeld") - 1)) {
> +             fprintf(stderr, "%s: invalid magic value\n", argv0);
> +             return 1;
> +     }
> +     width = ntohl(hdr[2]);
> +     height = ntohl(hdr[3]);
> +
> +     /* sanitize the input sizes */
> +     if (crop_x > width) {
> +             crop_x = width;
> +     }
> +        if (crop_y > height) {
> +                crop_y = height;
> +        }
> +     if (crop_w > width - crop_x) {
> +             crop_w = width - crop_x;
> +     }
> +     if (crop_h > height - crop_y) {
> +             crop_h = height - crop_y;
> +     }
> +

Some style issues, please use TABS for indentation.

> +        /* write header */
> +        fputs("farbfeld", stdout);
> +        tmp32 = htonl(crop_w);
> +        if (fwrite(&tmp32, sizeof(uint32_t), 1, stdout) != 1)
> +                goto writerr;
> +        tmp32 = htonl(crop_h);
> +        if (fwrite(&tmp32, sizeof(uint32_t), 1, stdout) != 1)
> +                goto writerr;
> +
> +     if (width > SIZE_MAX / ((sizeof("RGBA") - 1) * sizeof(uint16_t))) {
> +             fprintf(stderr, "%s: row length integer overflow\n", argv0);
> +             return 1;
> +     }
> +     rowlen = width * (sizeof("RGBA") - 1);
> +     if (!(row = malloc(rowlen * sizeof(uint16_t)))) {
> +             fprintf(stderr, "%s: malloc: out of memory\n", argv0);
> +             return 1;
> +     }
> +     crop_skiplen = crop_x * (sizeof("RGBA") - 1);
> +     crop_rowlen = crop_w * (sizeof("RGBA") - 1);
> +
> +     /* skip the first rows */
> +     for (i = 0; i < crop_y; ++i) {
> +             if (fread(row, sizeof(uint16_t), rowlen, stdin) != rowlen) {
> +                     goto readerr;
> +             }
> +     }
> +     /* write rows */
> +     for (i = 0; i < crop_h; ++i) {
> +             if (fread(row, sizeof(uint16_t), rowlen, stdin) != rowlen) {
> +                     goto readerr;
> +             }
> +             fwrite(row + crop_skiplen, sizeof(uint16_t), crop_rowlen, 
> stdout);
> +     }
> +
> +     return 0;
> +readerr:
> +     fprintf(stderr, "%s: fread: ", argv0);
> +     perror(NULL);
> +
> +     return 1;
> +
> +writerr:
> +     fprintf(stderr, "%s: fwrite: ", argv0);
> +     perror(NULL);
> +
> +     return 1;
> +}
> -- 
> 2.8.1
> 

Kind regards,
Hiltjo

Reply via email to