Re: [hackers][farbfeld][patch] ffcrop tool.

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Fri, 8 Apr 2016 16:08:07 +0200

On Fri, Apr 08, 2016 at 01:13:09PM +0000, rain1_AT_openmailbox.org 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 <rain1_AT_openmailbox.org>
> 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
> _AT_@ -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
> _AT_@ -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
Received on Fri Apr 08 2016 - 16:08:07 CEST

This archive was generated by hypermail 2.3.0 : Fri Apr 08 2016 - 16:12:14 CEST