Skip to content

Commit

Permalink
set autoreload timeout based on the latest event (#459)
Browse files Browse the repository at this point in the history
currently the autoreload feature of nsxiv is a bit unreliable because we
try to load at the very first event we received. however, the writer
might not be done writing and so we might try to load a truncated image
(and fail).

in the following ascii diagram, function S represents sleep and `+` sign
represents writes by the writer. because we set the sleep (of 10ms) at
the first event, subsequent writes by the writer doesn't influence our
reload logic:

       S(10)                   load()
nsxiv        |                       |
writer       +           +           +           + (done)
time(ms):   00          05          10          15

after this patch, (assuming function T (re)sets a timeout), we will keep
(re)setting a timeout on new events giving the writer more time to
finish:

       T(10)       T(10)       T(10)       T(10)                   load()
nsxiv        |           |           |           |                       |
writer       +           +           +           + (done)
time(ms):   00          05          10          15          20          25

while this patch makes things significantly more robust, the problem
here is inherently unsolvable since there's no way to tell whether the
writer is done writing or not.

for example, if user does something like `curl 'some.png' > test.png`
then curl might stop for a second or two in the middle of writing due to
internet issues - which will make nsxiv drop the image.

this patch also increases the autoreload delay from 10ms to now 128ms
instead to decrease chances of false failures.

ref: 0ion9/sxiv@6ae2df6
partially-fixes: https://codeberg.org/nsxiv/nsxiv/issues/456
commit-message-by: NRK
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/459
Reviewed-by: eylles <eylles@noreply.codeberg.org>
  • Loading branch information
0ion9 authored and N-R-K committed Aug 28, 2023
1 parent cc132dd commit 10a6228
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 10 deletions.
1 change: 1 addition & 0 deletions image.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ void img_init(img_t *img, win_t *win)
img->dirty = false;
img->anti_alias = options->anti_alias;
img->alpha_layer = options->alpha_layer;
img->autoreload_pending = false;
img->multi.cap = img->multi.cnt = 0;
img->multi.animate = options->animate;
img->multi.framedelay = options->framerate > 0 ? 1000 / options->framerate : 0;
Expand Down
33 changes: 23 additions & 10 deletions main.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "commands.h"
#include "config.h"

#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <locale.h>
Expand Down Expand Up @@ -72,6 +73,8 @@ int prefix;
bool title_dirty;
const XButtonEvent *xbutton_ev;

static void autoreload(void);

static bool extprefix;
static bool resized = false;

Expand All @@ -91,6 +94,7 @@ static struct {
struct timeval when;
bool active;
} timeouts[] = {
{ autoreload },
{ redraw },
{ reset_cursor },
{ slideshow },
Expand Down Expand Up @@ -269,6 +273,18 @@ static bool check_timeouts(int *t)
return tmin > 0;
}

static void autoreload(void)
{
if (img.autoreload_pending) {
img_close(&img, true);
/* load_image() sets autoreload_pending to false */
load_image(fileidx);
redraw();
} else {
assert(!"unreachable");
}
}

static void kill_close(pid_t pid, int *fd)
{
if (fd != NULL && *fd != -1) {
Expand Down Expand Up @@ -367,10 +383,13 @@ void load_image(int new)

if (win.xwin != None)
win_set_cursor(&win, CURSOR_WATCH);
reset_timeout(autoreload);
reset_timeout(slideshow);

if (new != current)
if (new != current) {
alternate = current;
img.autoreload_pending = false;
}

img_close(&img, false);
while (!img_load(&img, &files[new])) {
Expand Down Expand Up @@ -735,7 +754,6 @@ static void run(void)
enum { FD_X, FD_INFO, FD_TITLE, FD_ARL, FD_CNT };
struct pollfd pfd[FD_CNT];
int timeout = 0;
const struct timespec ten_ms = { 0, 10000000 };
bool discard, init_thumb, load_thumb, to_set;
XEvent ev, nextev;

Expand Down Expand Up @@ -775,14 +793,9 @@ static void run(void)
read_info();
if (pfd[FD_TITLE].revents & POLLIN)
read_title();
if (pfd[FD_ARL].revents & POLLIN) {
if (arl_handle(&arl)) {
/* when too fast, imlib2 can't load the image */
nanosleep(&ten_ms, NULL);
img_close(&img, true);
load_image(fileidx);
redraw();
}
if ((pfd[FD_ARL].revents & POLLIN) && arl_handle(&arl)) {
img.autoreload_pending = true;
set_timeout(autoreload, TO_AUTORELOAD, true);
}
}
continue;
Expand Down
2 changes: 2 additions & 0 deletions nsxiv.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ typedef struct {

/* timeouts in milliseconds: */
enum {
TO_AUTORELOAD = 128,
TO_REDRAW_RESIZE = 75,
TO_REDRAW_THUMBS = 200,
TO_CURSOR_HIDE = 1200,
Expand Down Expand Up @@ -204,6 +205,7 @@ struct img {
bool dirty;
bool anti_alias;
bool alpha_layer;
bool autoreload_pending;

struct {
bool on;
Expand Down

0 comments on commit 10a6228

Please sign in to comment.