On Sat, Jun 04, 2016 at 05:28:31PM +0200, Markus Teich wrote:
> Hiltjo Posthuma wrote:
> > previously an image file would be opened but only ffread when advancing to
> > the slide, but when the slide was not used it gave an error:
> > 
> >     /usr/local/bin/2ff: failed to convert image/png
> 
> Heyho Hiltjo,
> 
> thanks for the patch. Unfortunately it does not work if the first slide 
> contains
> an image (ffopen is only called in advance). I think it would be good to merge
> ffopen and ffread instead into a single function ffload. This makes the 
> `LOADED`
> state clearer and also enforces that the fd is closed in the same function 
> where
> it is opened. This ffload function should then be called in advance() 
> replacing
> the ffread() calls if the image is not loaded yet and once in load() for the
> first slide if it is an image.
> 

Ah yes, sorry for the oversight.

> If you want to take this new approach, go for it, otherwise I'll look into it
> myself.
>

I have attached a patch that does this, I hope you'll like it.

Also I'm not sure if we need the below code in advance(), I have removed it in
the patch:

        if (slidecount > idx + 1 && slides[idx + 1].img)
                ffread(slides[idx + 1].img);
        if (0 < idx && slides[idx - 1].img)
                ffread(slides[idx - 1].img);

That seems to preload the next and previous slide image right? A minor issue I
notice also is that images seem to flicker, it uses XPutImage directly to
xw.win. Maybe it can be replaced with a backbuffer then XCopyArea? What do you
think?

In advance() it should also not always be needed to rescale the image.

-- 
Kind regards,
Hiltjo
>From 97bebdcab4003f9acdfdd4bdf424449299ffd61d Mon Sep 17 00:00:00 2001
From: Hiltjo Posthuma <hil...@codemadness.org>
Date: Sat, 4 Jun 2016 21:34:25 +0200
Subject: [PATCH] merge ffread and ffopen into one function

---
 sent.c | 77 +++++++++++++++++++++++-------------------------------------------
 1 file changed, 27 insertions(+), 50 deletions(-)

diff --git a/sent.c b/sent.c
index 05902bf..605a7c3 100644
--- a/sent.c
+++ b/sent.c
@@ -89,9 +89,8 @@ typedef struct {
        const Arg arg;
 } Shortcut;
 
-static Image *ffopen(char *filename);
 static void fffree(Image *img);
-static void ffread(Image *img);
+static Image *ffread(char *filename);
 static void ffprepare(Image *img);
 static void ffscale(Image *img);
 static void ffdraw(Image *img);
@@ -159,14 +158,27 @@ filter(int fd, const char *cmd)
        return fds[0];
 }
 
+void
+fffree(Image *img)
+{
+       free(img->buf);
+       if (img->ximg)
+               XDestroyImage(img->ximg);
+       free(img);
+}
+
 Image *
-ffopen(char *filename)
+ffread(char *filename)
 {
+       uint32_t y, x;
+       uint16_t *row;
+       uint8_t opac, fg_r, fg_g, fg_b, bg_r, bg_g, bg_b;
+       size_t rowlen, off, nbytes, i;
+       ssize_t count;
        unsigned char hdr[16];
        char *bin = NULL;
        regex_t regex;
        Image *img;
-       size_t i;
        int tmpfd, fd;
 
        for (i = 0; i < LEN(filters); i++) {
@@ -178,13 +190,11 @@ ffopen(char *filename)
                        break;
                }
        }
-
        if (!bin)
                return NULL;
 
-       if ((fd = open(filename, O_RDONLY)) < 0) {
+       if ((fd = open(filename, O_RDONLY)) < 0)
                die("sent: Unable to open file %s:", filename);
-       }
 
        tmpfd = fd;
        fd = filter(fd, bin);
@@ -192,10 +202,7 @@ ffopen(char *filename)
                die("sent: Unable to filter %s:", filename);
        close(tmpfd);
 
-       if (read(fd, hdr, 16) != 16)
-               return NULL;
-
-       if (memcmp("farbfeld", hdr, 8))
+       if (read(fd, hdr, 16) != 16 || memcmp("farbfeld", hdr, 8))
                return NULL;
 
        img = calloc(1, sizeof(Image));
@@ -203,31 +210,6 @@ ffopen(char *filename)
        img->bufwidth = ntohl(*(uint32_t *)&hdr[8]);
        img->bufheight = ntohl(*(uint32_t *)&hdr[12]);
 
-       return img;
-}
-
-void
-fffree(Image *img)
-{
-       free(img->buf);
-       if (img->ximg)
-               XDestroyImage(img->ximg);
-       free(img);
-}
-
-void
-ffread(Image *img)
-{
-       uint32_t y, x;
-       uint16_t *row;
-       uint8_t opac;
-       uint8_t fg_r, fg_g, fg_b, bg_r, bg_g, bg_b;
-       size_t rowlen, off, nbytes;
-       ssize_t count;
-
-       if (img->state & LOADED)
-               return;
-
        if (img->buf)
                free(img->buf);
        /* internally the image is stored in 888 format */
@@ -237,9 +219,8 @@ ffread(Image *img)
        /* scratch buffer to read row by row */
        rowlen = img->bufwidth * 2 * strlen("RGBA");
        row = malloc(rowlen);
-       if (!row) {
+       if (!row)
                die("sent: Unable to malloc buffer for image row.\n");
-       }
 
        /* extract window background color channels for transparency */
        bg_r = (sc[ColBg].pixel >> 16) % 256;
@@ -270,6 +251,8 @@ ffread(Image *img)
        free(row);
        close(img->fd);
        img->state |= LOADED;
+
+       return img;
 }
 
 void
@@ -444,13 +427,11 @@ load(FILE *fp)
                                memmove(s->lines[s->linecount], 
&s->lines[s->linecount][1], blen);
                        s->linecount++;
                } while ((p = fgets(buf, sizeof(buf), fp)) && strcmp(buf, "\n") 
!= 0);
+
                slidecount++;
                if (!p)
                        break;
        }
-
-       if (slidecount && slides[0].embed && slides[0].embed[0])
-               slides[0].img = ffopen(slides[0].embed);
 }
 
 void
@@ -462,13 +443,7 @@ advance(const Arg *arg)
                if (slides[idx].img)
                        slides[idx].img->state &= ~(DRAWN | SCALED);
                idx = new_idx;
-               if (!slides[idx].img && slides[idx].embed && 
slides[idx].embed[0])
-                       slides[idx].img = ffopen(slides[idx].embed);
                xdraw();
-               if (slidecount > idx + 1 && slides[idx + 1].img)
-                       ffread(slides[idx + 1].img);
-               if (0 < idx && slides[idx - 1].img)
-                       ffread(slides[idx - 1].img);
        }
 }
 
@@ -514,7 +489,11 @@ void
 xdraw()
 {
        unsigned int height, width, i;
-       Image *im = slides[idx].img;
+       Image *im;
+
+       if (!slides[idx].img && slides[idx].embed && slides[idx].embed[0])
+               slides[idx].img = ffread(slides[idx].embed);
+       im = slides[idx].img;
 
        getfontsize(&slides[idx], &width, &height);
        XClearWindow(xw.dpy, xw.win);
@@ -532,8 +511,6 @@ xdraw()
                                 0);
                drw_map(d, xw.win, 0, 0, xw.w, xw.h);
        } else {
-               if (!(im->state & LOADED))
-                       ffread(im);
                if (!(im->state & SCALED))
                        ffprepare(im);
                if (!(im->state & DRAWN))
-- 
2.8.1

Reply via email to