sxiv - Simple image viewer written in C

gorilla · 2011-02-10 19:13:33

ber_t wrote:

Dieter@be wrote:
so, if i open a gif file with sxiv-git, notice it doesn't work, i have to close it and manually type another program name?
I'm currently thinking about some new mechanisms, which would make such interactions with other programs much simpler. I will shortly describe them here, and would like to ask you all to comment on which ones I should implement, if any:
1. New option flag -e to specify a command line, which is run on the currently visible image after pressing a certain key. Could also be extended to options -1 to -0 and corresponding keys F1-F10. Should the image always be reloaded after triggering such an external action?
2. Key mapping to print the filename of the currently visible image to stdout, or mark/unmark the currently visible image and then print the filenames of all marked images to stdout when quitting sxiv. This would be useful in scripts, for instance to open a bunch of images and decide which ones to delete.

The first option sounds promisng for me and if I understand you right, it can include the second. I mean you could bind a command to print the filename to stdout or somewhere else... I think reloading the image should be done manual, if necessary...

Last edited by gorilla (2011-02-10 19:14:29)

Dieter@be · 2011-02-10 20:25:15

gorilla wrote:

ber_t wrote:
Dieter@be wrote:
so, if i open a gif file with sxiv-git, notice it doesn't work, i have to close it and manually type another program name?
I'm currently thinking about some new mechanisms, which would make such interactions with other programs much simpler. I will shortly describe them here, and would like to ask you all to comment on which ones I should implement, if any:
1. New option flag -e to specify a command line, which is run on the currently visible image after pressing a certain key. Could also be extended to options -1 to -0 and corresponding keys F1-F10. Should the image always be reloaded after triggering such an external action?
2. Key mapping to print the filename of the currently visible image to stdout, or mark/unmark the currently visible image and then print the filenames of all marked images to stdout when quitting sxiv. This would be useful in scripts, for instance to open a bunch of images and decide which ones to delete.
The first option sounds promisng for me and if I understand you right, it can include the second. I mean you could bind a command to print the filename to stdout or somewhere else... I think reloading the image should be done manual, if necessary...

i agree with gorilla. except: i guess there could be use cases where you might want to automatically reload without forcing a keypress on the user, maybe you could check the mtime of the image when the child process is done and reload if it changed?

ber_t · 2011-02-10 22:10:23

Okay. I was hoping that you would choose number 2, because it's a lot easier to implement. Anyway, I will do the first one, but before I start, let me ask just one more question: Should triggering such an action cause sxiv to wait for the child or just fork it and give a damn about it? Because checking the mtime and reloading the image, if it has been changed by the child, requires the blocking behaviour...

gorilla · 2011-02-10 23:09:24

ber_t wrote:

Okay. I was hoping that you would choose number 2, because it's a lot easier to implement. Anyway, I will do the first one, but before I start, let me ask just one more question: Should triggering such an action cause sxiv to wait for the child or just fork it and give a damn about it? Because checking the mtime and reloading the image, if it has been changed by the child, requires the blocking behaviour...

You asked for it

But it's a good Idea to ask the people...

Well the next question...
Since all I would use it for is deleting/moving, tagging and cropping, I would choose the second option this time... I think it would also be the better choice if you need to edit an image in gimp...
What should be the advantages of waiting for the child? Maybe I'll change my oppinion...

Honestly, I like sxiv as it is right now. Maybe keep sxiv 0.5 and put a secondone (with the additional features) like "sxiv+" or so, to the AUR.

Last edited by gorilla (2011-02-10 23:10:57)

Dieter@be · 2011-02-11 12:58:15

consider extending sxiv with a lua api.

yes, that probably goes beyond your goals, but once you try to provide features that help users in scripting and combining multiple tools, you should do it correctly from the start. Look at the awesome window manager or the luakit browser, the api allows users to create their own bindings to invoke other tools, and trigger behaviors from the lua config.
You will only limit yourself, your app, and cause frustration, by providing limited scripting features. There will always be things that don't work quite right, or at all.
I learned a lot from the uzbl project. Imho you should either do scripting support properly, or not at all.

ber_t · 2011-02-11 13:36:33

Dieter@be wrote:

consider extending sxiv with a lua api.

I think that would be overkill, but you nail it with your next point:

Dieter@be wrote:

I learned a lot from the uzbl project. Imho you should either do scripting support properly, or not at all.

The more I think about this scripting or external action support, the more I come to the conclusion that I should not do it.
I mean, all I was talking about is the definition of some commands (at compile- or runtime) like "gimp" or "rm", maybe with a small number of arguments, which would get called upon a key press. This limits the possible commands down to the ones I've mentioned; even a `mv filename target' wouldn't be possible, because the name of the current file isn't at the end.
So, I kind of decided to abandon this idea and keep sxiv as minimal as it is right now, like gorilla already said:

gorilla wrote:

Honestly, I like sxiv as it is right now. Maybe keep sxiv 0.5 and put a secondone (with the additional features) like "sxiv+" or so, to the AUR.

But thanks to both of you, for taking the time to discuss this idea...

Last edited by ber_t (2011-02-11 13:38:27)

0mark · 2011-02-13 23:10:34

Great stuff

ber_t wrote:

The more I think about this scripting or external action support, the more I come to the conclusion that I should not do it.

But i just needed the ability to execute custom scripts on the current image. Patchtime!
*edit* Just a quick hack, no sanity checking, error handling and stuff!

diff --git a/main.c b/main.c
index f449df9..4da1774 100644
--- a/main.c
+++ b/main.c
@@ -22,6 +22,7 @@
 #include <dirent.h>
 #include <sys/select.h>
 #include <sys/stat.h>
+#include <unistd.h>
 
 #include <X11/Xlib.h>
 #include <X11/Xutil.h>
@@ -232,6 +233,8 @@ void on_keypress(XKeyEvent *kev) {
     char key;
     KeySym ksym;
     int changed;
+    command *cmd;
+    char *buf, *buf2;
 
     if (!kev)
         return;
@@ -357,6 +360,23 @@ void on_keypress(XKeyEvent *kev) {
             break;
     }
 
+    for(cmd = options->commands; cmd; cmd = cmd->next) {
+        if(key == cmd->key) {
+            if(fork()==0) {
+                setsid(); /* Please do this a less uglu way! */
+                buf = calloc(1, sizeof(char)*(strlen(cmd->string)+1));
+                strcpy(buf, cmd->string);
+                buf2 = calloc(1, sizeof(char)*(strlen(buf)+strlen(filenames[fileidx])+1));
+                sprintf(buf2, buf, filenames[fileidx]);
+                execlp("/bin/sh", "sh", "-c", buf2);
+                printf("ERROR:\n");
+                printf(cmd->string, filenames[fileidx]);
+                printf("\n");
+                exit(0);
+            }
+        }
+    }
+
     if (changed) {
         img_render(&img, &win);
         update_title();
diff --git a/options.c b/options.c
index 3e519f2..556d5c5 100644
--- a/options.c
+++ b/options.c
@@ -25,8 +25,10 @@
 #include "config.h"
 #include "options.h"
 
-options_t _options;
+/*options_t _options;
 const options_t *options = (const options_t*) &_options;
+static options_t options;*/
+options_t *options;
 
 void print_usage() {
     printf("usage: sxiv [-dFfhpqrsvZ] [-g GEOMETRY] [-z ZOOM] FILES...\n");
@@ -40,70 +42,95 @@ void print_version() {
 void parse_options(int argc, char **argv) {
     float z;
     int opt;
+    char k;
+    char *s;
+    command *cmd;
 
-    _options.scalemode = SCALE_MODE;
-    _options.zoom = 1.0;
-    _options.aa = 1;
+    options = (options_t *)calloc(1, sizeof(options_t));
 
-    _options.fixed = 0;
-    _options.fullscreen = 0;
-    _options.geometry = NULL;
+    options->scalemode = SCALE_MODE;
+    options->zoom = 1.0;
+    options->aa = 1;
 
-    _options.quiet = 0;
-    _options.recursive = 0;
+    options->fixed = 0;
+    options->fullscreen = 0;
+    options->geometry = NULL;
 
-    while ((opt = getopt(argc, argv, "dFfg:hpqrsvZz:")) != -1) {
+    options->quiet = 0;
+    options->recursive = 0;
+    
+    options->commands = NULL;
+
+    while ((opt = getopt(argc, argv, "dFfg:hpqrsvZze:")) != -1) {
         switch (opt) {
             case '?':
                 print_usage();
                 exit(1);
             case 'd':
-                _options.scalemode = SCALE_DOWN;
+                options->scalemode = SCALE_DOWN;
                 break;
             case 'F':
-                _options.fixed = 1;
+                options->fixed = 1;
                 break;
             case 'f':
-                _options.fullscreen = 1;
+                options->fullscreen = 1;
                 break;
             case 'g':
-                _options.geometry = optarg;
+                options->geometry = optarg;
                 break;
             case 'h':
                 print_usage();
                 exit(0);
             case 'p':
-                _options.aa = 0;
+                options->aa = 0;
                 break;
             case 'q':
-                _options.quiet = 1;
+                options->quiet = 1;
                 break;
             case 'r':
-                _options.recursive = 1;
+                options->recursive = 1;
                 break;
             case 's':
-                _options.scalemode = SCALE_FIT;
+                options->scalemode = SCALE_FIT;
                 break;
             case 'v':
                 print_version();
                 exit(0);
             case 'Z':
-                _options.scalemode = SCALE_ZOOM;
-                _options.zoom = 1.0;
+                options->scalemode = SCALE_ZOOM;
+                options->zoom = 1.0;
                 break;
             case 'z':
-                _options.scalemode = SCALE_ZOOM;
+                z = 0;
+                options->scalemode = SCALE_ZOOM;
                 if (!sscanf(optarg, "%f", &z) || z < 0) {
                     fprintf(stderr, "sxiv: invalid argument for option -z: %s\n",
                             optarg);
                     exit(1);
                 } else {
-                    _options.zoom = z / 100.0;
+                    options->zoom = z / 100.0;
+                }
+                break;
+            case 'e':
+                s  = calloc(1, 256*sizeof(char));
+                if (sscanf(optarg, "%c=%254c", &k, s)!=2) {
+                    fprintf(stderr, "sxiv: invalid command definition: %s\n", optarg);
+                    exit(1);
+                } else {
+                    if(!options->commands) {
+                        cmd = options->commands = (command *)calloc(1, sizeof(command));
+                    } else {
+                        for(cmd = options->commands; cmd->next; cmd = cmd->next);
+                        cmd->next = (command *)calloc(1, sizeof(command));
+                        cmd = cmd->next;
+                    }
+                    cmd->key = k;
+                    cmd->string = s;
+                    cmd->next = NULL;
                 }
                 break;
         }
     }
-
-    _options.filenames = (const char**) argv + optind;
-    _options.filecnt = argc - optind;
+    options->filenames = (const char**) argv + optind;
+    options->filecnt = argc - optind;
 }
diff --git a/options.h b/options.h
index 3cb4b38..dcc0d80 100644
--- a/options.h
+++ b/options.h
@@ -21,7 +21,16 @@
 
 #include "image.h"
 
-typedef struct options_s {
+typedef struct command command;
+typedef struct options_s options_t;
+
+struct command {
+    char key;
+    char *string;
+    command *next;
+};
+
+struct options_s {
     const char **filenames;
     int filecnt;
 
@@ -35,9 +44,11 @@ typedef struct options_s {
 
     unsigned char quiet;
     unsigned char recursive;
-} options_t;
 
-extern const options_t *options;
+    command *commands;
+};
+
+extern options_t *options;
 
 void print_usage();
 void print_version();

You can simply bind a single key to a command (%s will be replaced with current image path and filename). For example:

./sxiv -e 1='x=`ls /path/2/where/the/images/are|dmenu`; test "x" != "x$x" && mv %s /path/2/where/the/images/are/$x/' -e 3=bla -F ../tet/*

Binds to key '1' and runs dmenu to select a subdir in the gallery dir and moves the image.

I plan a second, non-forking version wich uses return codes to determine action (reload image, show next image, remove image from list, etc..)

The other idea mentioned above, sending image names to stdout, sounds also very interesting. Maybe i will try that too.

Dieter@be wrote:

I learned a lot from the uzbl project. Imho you should either do scripting support properly, or not at all.

Dude, i loved uzbl as it was before the external event loop. Actually, im trying to backport a recent uzbl to old school uzbl

Last edited by 0mark (2011-02-14 12:18:19)

ber_t · 2011-02-14 09:59:00

@0mark: While I appreciate your work, I must say that I'm not happy with it, because it doesn't handle any calloc errors; there are already some secure wrapper functions to malloc and realloc in util.c.
The use of the sprintf aprroach with "%s" is very risky. The least you should do is using snprintf. What if the command string has more than one "%s" in it, or other format directives?
Why did you change the whole "options_t _options" and "const options_t *options" thing in options.c? It has the purpose to only allow write access to the options struct from inside options.c.
It's also possible to assign a command to an already mapped key, which I believe results in triggering the default and the command action inside the on_keypress handler. Why not using the function or number keys for triggering the actions and option flags -0/-1 to -9 to assign the external actions to them?

0mark · 2011-02-14 10:42:07

ber_t wrote:

@0mark: While I appreciate your work, I must say that I'm not happy with it, because it doesn't handle any calloc errors; there are already some secure wrapper functions to malloc and realloc in util.c.
The use of the sprintf aprroach with "%s" is very risky. The least you should do is using snprintf. What if the command string has more than one "%s" in it, or other format directives?

Its just a quick hack. I should have mentioned that.

ber_t wrote:

Why did you change the whole "options_t _options" and "const options_t *options" thing in options.c? It has the purpose to only allow write access to the options struct from inside options.c.

I did not get it to work otherwise. Never seen such a construct before. Im practically a C noob
Actually, i also cant see why only options.c should write to the options string.

ber_t wrote:

It's also possible to assign a command to an already mapped key, which I believe results in triggering the default and the command action inside the on_keypress handler. Why not using the function or number keys for triggering the actions and option flags -0/-1 to -9 to assign the external actions to them?

I know. For now, its up to the user. Double used keys might even become usefull. But i think later i will only test for user defined key when no other key fits. Only allowing some keys is not what i want.

ber_t · 2011-02-16 20:57:56

Version 0.6 is out..

New features:

    - Read filenames from stdin
    - Check if files got removed/renamed while running
    - Handle UTF-8 titles correctly (thanks to fungt)
    - Handle WM_DELETE_WINDOW messages (thanks to fungt)

sironitomas · 2011-02-21 10:10:45

This image viewer is awesome. Feh has been replaced a few seconds ago!

Oh, I almost forget. In the last git version, fullscreen is not working.

Regards!

Army · 2011-02-21 10:46:19

Tiny bug:
When I open an image and the windows is being resized according to the layout of the tiling wm, the zoom factor in the window title isn't being adjusted. Not a big one, but it should be being fixed.

Last edited by Army (2011-02-21 10:46:41)

ber_t · 2011-02-21 14:12:22

sironitomas wrote:

Oh, I almost forget. In the last git version, fullscreen is not working.

It's working for me. Has it ever worked for you? If not: does your window manager support EWMH?

Army wrote:

Tiny bug:
When I open an image and the windows is being resized according to the layout of the tiling wm, the zoom factor in the window title isn't being adjusted. Not a big one, but it should be being fixed.

It's already fixed (by accident) in the thumbs branch, which soon will become the new head...
So yes, I've been working on thumbnail support recently and must admit that they can be extremely useful.

EDIT: Merged the thumbs branch, so it's fixed in head.

Last edited by ber_t (2011-02-21 16:08:54)

karol · 2011-02-21 14:40:47

qiv hides the mouse cursor if I'm not moving it while viewing the pictures, sxiv keeps the cursor visible at all times. Is it the intended behaviour? Do you plan to change it? ;P Is there an option I can use to hide the cursor (other than moving it to the bottom right corner of the screen)?

ber_t · 2011-02-21 19:16:07

karol wrote:

qiv hides the mouse cursor if I'm not moving it while viewing the pictures, sxiv keeps the cursor visible at all times. Is it the intended behaviour? Do you plan to change it? ;P Is there an option I can use to hide the cursor (other than moving it to the bottom right corner of the screen)?

Never thought of that, but implemented it right away. Can be found in the current head...

karol · 2011-02-21 19:28:51

ber_t wrote:

karol wrote:
qiv hides the mouse cursor if I'm not moving it while viewing the pictures, sxiv keeps the cursor visible at all times. Is it the intended behaviour? Do you plan to change it? ;P Is there an option I can use to hide the cursor (other than moving it to the bottom right corner of the screen)?
Never thought of that, but implemented it right away. Can be found in the current head...

Just tested it - works great!
I <3 you ... in a forumish way, of course :-D

vanvalium · 2011-02-22 00:30:58

nice viewer,
I will check tomorrow if it's enough to replace feh
Haven't tried most of the features yet, but I dig the minimal approach

The more I think about this scripting or external action support, the more I come to the conclusion that I should not do it.

Hope you don't, keep it small

sironitomas · 2011-02-22 01:09:21

ber_t wrote:

It's working for me. Has it ever worked for you? If not: does your window manager support EWMH?

Sory, I haven't noticed I needed a EWMH compliant WM to use fullscreen. So I moved from openbox to wmfs (impressive BTW) and It is working very fine.

Just tried the new thumbs option and it works, but a little bit slow. Nothing you can do though (caching?), except not to combine 14MP photos with a atom processor

Regards!

Last edited by sironitomas (2011-02-22 01:12:18)

ber_t · 2011-02-23 18:51:06

sironitomas wrote:

Just tried the new thumbs option and it works, but a little bit slow. Nothing you can do though (caching?), except not to combine 14MP photos with a atom processor

I know it's slow, but caching them in a file per folder or in separate files in a subdir of home seems to be too much for a <2000 sloc project. Thumbnails on their own may be already too much... otherwise I think the thumbnail support is quite nice compared to feh.

alterecco · 2011-02-24 22:56:39

Thumbnail support! Awesome!

It works good so far. Thanks a lot!

tzervo · 2011-02-25 06:51:09

Excellent viewer, congratulations. One question though. Would you consider an option to save rotated images? It is not uncommon to skim through some newly-taken photos, and then notice that you want to rotate some of them and have them remain so.

ber_t · 2011-02-25 08:26:51

tzervo wrote:

Excellent viewer, congratulations. One question though. Would you consider an option to save rotated images? It is not uncommon to skim through some newly-taken photos, and then notice that you want to rotate some of them and have them remain so.

I'll add this to the todo list and will look into it.

Dieter@be · 2011-02-25 13:03:35

ber_t wrote:

tzervo wrote:
Excellent viewer, congratulations. One question though. Would you consider an option to save rotated images? It is not uncommon to skim through some newly-taken photos, and then notice that you want to rotate some of them and have them remain so.
I'll add this to the todo list and will look into it.

or better: update the EXIF data that defines the rotation.
assume sxiv supports exif

ber_t · 2011-02-25 15:23:39

Dieter@be wrote:

ber_t wrote:
tzervo wrote:
Excellent viewer, congratulations. One question though. Would you consider an option to save rotated images? It is not uncommon to skim through some newly-taken photos, and then notice that you want to rotate some of them and have them remain so.
I'll add this to the todo list and will look into it.
or better: update the EXIF data that defines the rotation.
assume sxiv supports exif

Using the EXIF tag seems to be the only reasonable way. There's no easy possibility to tell Imlib2 what quality to use when writing a file, so it would be a really bad idea to use it's functionality to write files. I also looked into jpegtran, but it didn't work very well for lots of my test images.
So I'll have a look at libexif. Maybe I'll use it to automatically rotate/flip images based on the current tag and to update the tag whenever a jpeg file is rotated.

Ledti · 2011-02-25 15:42:33

The -Z option is nearly ideal for manga/comics, but it defaults to centering images. Would it be possible to have a method of setting the default viewing position? Perhaps consider having it default to the top for images that require panning (if it's possible).

I've also noticed that -r will scramble the order of files, while using a glob (instead of -r) will work correctly, for example with:

000.jpg
001.jpg
002.jpg

sxiv is great, your work is appreciated.

Last edited by Ledti (2011-02-25 15:43:25)

Arch Linux

#51 2011-02-10 19:13:33

Re: sxiv - Simple image viewer written in C

#52 2011-02-10 20:25:15

Re: sxiv - Simple image viewer written in C

#53 2011-02-10 22:10:23

Re: sxiv - Simple image viewer written in C

#54 2011-02-10 23:09:24

Re: sxiv - Simple image viewer written in C

#55 2011-02-11 12:58:15

Re: sxiv - Simple image viewer written in C

#56 2011-02-11 13:36:33

Re: sxiv - Simple image viewer written in C

#57 2011-02-13 23:10:34

Re: sxiv - Simple image viewer written in C

#58 2011-02-14 09:59:00

Re: sxiv - Simple image viewer written in C

#59 2011-02-14 10:42:07

Re: sxiv - Simple image viewer written in C

#60 2011-02-16 20:57:56

Re: sxiv - Simple image viewer written in C

#61 2011-02-21 10:10:45

Re: sxiv - Simple image viewer written in C

#62 2011-02-21 10:46:19

Re: sxiv - Simple image viewer written in C

#63 2011-02-21 14:12:22

Re: sxiv - Simple image viewer written in C

#64 2011-02-21 14:40:47

Re: sxiv - Simple image viewer written in C

#65 2011-02-21 19:16:07

Re: sxiv - Simple image viewer written in C

#66 2011-02-21 19:28:51

Re: sxiv - Simple image viewer written in C

#67 2011-02-22 00:30:58

Re: sxiv - Simple image viewer written in C

#68 2011-02-22 01:09:21

Re: sxiv - Simple image viewer written in C

#69 2011-02-23 18:51:06

Re: sxiv - Simple image viewer written in C

#70 2011-02-24 22:56:39

Re: sxiv - Simple image viewer written in C

#71 2011-02-25 06:51:09

Re: sxiv - Simple image viewer written in C

#72 2011-02-25 08:26:51

Re: sxiv - Simple image viewer written in C

#73 2011-02-25 13:03:35

Re: sxiv - Simple image viewer written in C

#74 2011-02-25 15:23:39

Re: sxiv - Simple image viewer written in C

#75 2011-02-25 15:42:33

Re: sxiv - Simple image viewer written in C

Board footer