davidgriffith / xv Goto Github PK

Pressing 'C' should automatically crop out surrounding whitespace. It works fine in 8-bit mode, but not 24-bit mode. A fix was presented in xv-20140327-mark-brader-24bit-autocrop-bug.msg which partially fixes the problem and the provided test image (a locomotive drawing) crops more or less correctly. I have another one of a old mobile phone that is cropped completely incorrectly by default, which is 24-bit mode. If I shift into 8-bit mode and crop there, the crop happens flawlessly.

Debian and several applications (imagemagick) have switched from JasPer to OpenJPEG. JasPer (libjasper) has been removed from the Debian repositories as of Stretch. I think it would therefore be a good idea to rewrite https://github.com/DavidGriffith/xv/blob/master/xvjp2k.c to use OpenJPEG instead.

After the last official release of xv, Greg Roelofs has maintained the xv "Jumbo Patch" which incorporates all sorts of patches that he has collected. The last time the Jumbo Patch was updated was 2007. He recently sent me many individual patches that he hasn't assembled into a new Jumbo Patch. This is a listing of what was in the archive.

• xv-20040426-joe-zbiciak-rh7.3-bad-libpng.msg
• xv-20040525-jean-pierre-demailly-startgrab-imake-hips.patch
• xv-20040602-walter-harms-suse-rpm-specfile.msg
• xv-20041004-bruno-rohee-bmp-16bit-openbsd.patch.msg
• xv-20050422-joe-zonker-brockmeier-response-from-john-bradley.msg
• xv-20050601-jim-r-kirkpatrick-xvmisc.c-statement-not-reached.msg
• xv-20070625-thomas-klausner-bsd-compilation-issues.msg
• xv-20070711-fabian-greffrath-binaries-in-patch.msg
• xv-20071013-ross-combs-resize-ppos-paste.msg
• xv-20071013-ross-combs-resize-ppos-paste.patch
• xv-20071016-lukacs-arpad-gcc-4.x-lvalues-fix.patch
• xv-20071211-l-gabriel-somlo-response-from-john-bradley.msg
• xv-20080118-ian-collier-crash-fixes.msg
• xv-20080506-joe-peterson-wait-CLK_TCK-fix.dif
• xv-20080616-joe-peterson-makefile-LDFLAGS-to-linker.msg
• xv-20080901-david-bath-empty-filename-save-crash-fix.dif
• xv-20081204-fabian-greffrath-icon.xpm
• xv-20081204-fabian-greffrath-pixmap-manpage-bugs.mime
• xv-20081205-niklas-edmundsson-filename-buffersize-fix.patch
• xv-20081205-niklas-edmundsson-filename-buffersize-fix.patch.mime
• xv-20081211.klaus-ethgen-msg.pgp.txt
• xv-20081213-anthony-thyssen-slideshow-pause.dif
• xv-20081215-fabian-greffrath-xv.desktop.mime
• xv-20081216-elmar-plischke-xviff-crash-fix.dif
• xv-20081218-fabian-greffrath-01-cflags.patch
• xv-20081218-fabian-greffrath-02-shared-libjasper.patch
• xv-20081218-fabian-greffrath-03-install-pixmap.patch
• xv-20081218-fabian-greffrath-04-install-desktop.patch
• xv-20081218-fabian-greffrath-05-install-README.patch
• xv-20081218-fabian-greffrath-06-hyphen-used-as-minus-sign.patch
• xv-20081218-fabian-greffrath-07-manpages-CFLAGS-etc.mime
• xv-20081218-fabian-greffrath-patches-01-06.mime
• xv-20081227-klaus-ethgen-own-jumbo-patches-in-git.msg
• xv-20090126-ignatios-souvatzis-xxx.msg.pgp.asc
• xv-20090131-grr-C++-comments.where
• xv-20090322-jan-fuchs-CLOCKS_PER_SEC-vs-USE_TICKS.msg
• xv-20090426-lluis-perez-vidal-c++-comments-and-makefile.patch.mime
• xv-20090826-paul-howarth-warnings-and-JP-FLmask.patch.mime
• xv-20090907-patrick-keshishian-CMYK-JPEG-crash-fix.patch
• xv-20090907-patrick-keshishian-CMYK-JPEG-crash-fix.patch.mime
• xv-20090912-patrick-keshishian-CMYK-JPEG-crash-test-case.jpg
• xv-20090912-patrick-keshishian-CMYK-JPEG-crash-test-case.jpg.mime
• xv-20091212-patrick-keshishian-bmp-negative-height.patch.mime
• xv-20091215-werner-fink-multiple-fixes.patch.mime
• xv-20091217-patrick-keshishian-bmp-negative-height.patch.mime
• xv-20100206-niklas-edmundsson-filename-buffersize-fix2.patch
• xv-20100206-niklas-edmundsson-filename-buffersize-fix2.patch.mime
• xv-20100212-patrick-keshishian-bmp-negative-height.patch.mime
• xv-20100225-jpeg-segfault-bug-test-case.notes
• xv-20100228-anthony-thyssen-window-geometry-pos-drift.patch
• xv-20100405-patrick-keshishian-negative-height-sample.bmp.mime
• xv-20100523-markku-savela-wheelmouse-request.msg
• xv-20100523-ville-saari-clipboard-colormap-fix.dif
• xv-20100811-eeri-kask-nopos-window-placement-fix.patch
• xv-20110523-patrick-keshishian-openbsd-segfault-fix.msg
• xv-20110615-fabian-greffrath-libpng15-compile.msg
• xv-20120505-ross-combs-middle-button-paste.mime
• xv-20120507-ross-combs-middle-button-paste.mime
• xv-20130213-hints-for-macosx-snow-leopard.msgs
• xv-20130314-gabriel-somlo-NAME_MAX-buffer-overrun-fix.dif
• xv-20130328-gabriel-somlo-NAME_MAX-buffer-overrun-fix2.mark-brader-cut-paste-fix.dif
• xv-20140327-mark-brader-24bit-autocrop-bug.msg
• xv-20140506-yann-ricquebourg--exitcode.patch
• xv-20140506-yann-ricquebourg--newpng.patch
• xv-20140603-fabian-greffrath--updated-Debian-redistrib-permission.msg
• xv-20150913-rainer-m-canavan--xrandr-support--webp-support.dif
• xv--adobe-XMP--test-image--inspections_table_2013.png
• xv--adobe-XMP--test-image--inspections_table_2013.where
• xv--adobe-XMP--test-image--tumblr_mlqyn0H7A01qb3mfao1_250.png
• xv--adobe-XMP--test-image--tumblr_mlqyn0H7A01qb3mfao1_250.where
• xv-erling-jacobsen-20080225-smoothing-crash-fix.diff
• xv-fabian-greffrath-20070215-debian-stuff.msgs
• xv-fabian-greffrath-20070814-cygwin-patch.msg
• xv-fabian-greffrath-CFLAGS-fix2.diff
• xv-fabian-greffrath-CFLAGS-fix.diff
• xv-greg-kriehn-20071214-fedora8-packaging.msg
• xv-grr.dif.20070318.xvinfo-security-fix
• xv-jerome-ibanes-jpeg2000-2005-07-02T16-14-47-quality-100-truncate-eighth.jp2
• xv-jerome-ibanes-jpeg2000-output.gif
• xv-jerome-ibanes-jpeg2000-patches3.diff
• xv-jerome-ibanes-jpeg2000-patches.diff
• xv-jerome-ibanes-jpeg2000-patches.msg
• xv-jerome-ibanes-jpeg2000-patches.sh
• xv-joe-zbiciak-20070621-gif-decoder-bugfix.dif
• xv-jpdemailly-20070904-g3-fax-patch.mime
• xv-lukacs-arpad-gcc422-fix.dif
• xv-michal-maruska-no-flicker-bkgd.patch
• xv-rcombs-resize-ppos-paste.diff
• xv-rcombs-resize-ppos-paste.where
• xv-scott-marovich-20070214-jpeg2000-stuff.msgs
• xv-thomas-loimer-20070810-ubuntu-fixes.msg

The file xv-20091215-werner-fink-multiple-fixes.patch.mime addressed, among other things, switching away from using CLK_TCK to sysconf(_SC_CLK_TCK).

The patch number four uses the correct tick definition of the system instead of a hard defined value. In fact CLK_TCK isn't defined anymore on newer Linux systems as it now becomes variable at boot time.

Later on, I want to have the Makefile (or Autoconf) check to see if we should use sysconf(_SC_CLK_TCK) and if not, use the constant CLK_TCK.

xv was written with ibpng version 1.2 in mind. Since version 1.5, it has forbidden accessing png_ptr and info_ptr directly. Now everything must go through png_get_xxx() and png_set_xxx() functions.

Looking at http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/graphics/xv/patches/, I see some patches that were written after the 2007 Jumbo Patch that have merit.

xv-20091212-patrick-keshishian-bmp-negative-height.patch.mime
xv-20091217-patrick-keshishian-bmp-negative-height.patch.mime
xv-20100405-patrick-keshishian-negative-height-sample.bmp.mime

From http://en.wikipedia.org/wiki/BMP_file_format

Uncompressed Windows bitmaps can also be stored from the top row to the bottom, if the image > height value is negative.

The patch works on the 30k test file provided. Here is the header:

00000000   42 4D 68 75  00 00 00 00  00 00 36 00  00 00 28 00  BMhu......6...(.
00000010   00 00 64 00  00 00 9C FF  FF FF 01 00  18 00 00 00  ..d.............
00000020   00 00 32 75  00 00 12 0B  00 00 12 0B  00 00 00 00  ..2u............
00000030   00 00 00 00  00 00 FF FF  FF FF FF FF  FF FF FF FF  ................
00000040   FF FF FF FF  FF FF FF FF  FF FF FF FF  FF FF FF FF  ................
00000050   FF FF FF FF  FF FF FF FF  FF FF FF FF  FF FF FF FF  ................
00000060   FF FF FF FF  FF FF FF FF  FF FF FF FF  FF FF FF FF  ................
00000070   FF FF FF FF  FF FF FF FF  FF FF FF FF  FF FF FF FF  ................
00000080   FF FF FF FF  FF FF FF FF  FF FF FF FF  FF FF FF FF  ................
00000090   FF FF FF FF  FF FF FF FF  FF FF FF FF  FF FF FF FF  ................
000000A0   FF FF FF FF  FF FF FF FF  FF FF FF FF  FF FF FF FF  ................

$ file negheight_test_cs3.bmp 
PC bitmap, Windows 3.x format, 100 x -100 x 24

I found another, much smaller, test image at https://issues.apache.org/jira/browse/IMAGING-162. Even with the patch, xv doesn't like it.

Here's the whole file (cannot attach bmp files to Github issues):

begin 644 monochrome-negative-height.bmp
M0DVB`````````((```!L````!````/C___\!``$``````"`````3"P``$PL`
M``(````"````0D=2<P``````````````````````````````````````````
M``````````````````````(```````````````````#___\`````````````
;```````````````0````(````$````"`````
`
end

This is the entire hexdump of the file:

00000000   42 4D A2 00  00 00 00 00  00 00 82 00  00 00 6C 00  BM............l.
00000010   00 00 04 00  00 00 F8 FF  FF FF 01 00  01 00 00 00  ................
00000020   00 00 20 00  00 00 13 0B  00 00 13 0B  00 00 02 00  .. .............
00000030   00 00 02 00  00 00 42 47  52 73 00 00  00 00 00 00  ......BGRs......
00000040   00 00 00 00  00 00 00 00  00 00 00 00  00 00 00 00  ................
00000050   00 00 00 00  00 00 00 00  00 00 00 00  00 00 00 00  ................
00000060   00 00 00 00  00 00 00 00  00 00 02 00  00 00 00 00  ................
00000070   00 00 00 00  00 00 00 00  00 00 FF FF  FF 00 00 00  ................
00000080   00 00 00 00  00 00 00 00  00 00 00 00  00 00 00 00  ................
00000090   00 00 10 00  00 00 20 00  00 00 40 00  00 00 80 00  ...... ...@.....
000000A0   00 00                                               ..

file monochrome-negative-height.bmp
monochrome-negative-height.bmp: PC bitmap, Windows 95/NT4 and newer format, 4 x -8 x 1

So... it appears that the patch is deficient in handling newer-format .bmp files.

Patch was originally applied in 564e566. More work is necessary.

For a long time (always?) the xv image window scoots down and to the right when another image is loaded. Depending on who you are, this can be a major or minor annoyance. It's particularly disturbing when using xv to display a slideshow. The patch xv-20100228-anthony-thyssen-window-geometry-pos-drift.patch claims to fix this in a limited fashion. In practice I found that one to be more irritating than simply leaving the problem alone. A solid fix is needed.

I applied the FLmask patch by hand and placed the results in the flmask branch. Two masks cause segfaults. These are MEKO and CPMASK.

Patch file was found at https://github.com/pld-linux/xv.git

xvjp2k.c is incapable of loading a Part 2 JP2 file. It's not clear to me at this point if there's an endian problem or something else.

This might be subordinate to Issue #2.

This is a large patch presented to Greg by Ross Combs in xv-20120505-ross-combs-middle-button-paste.mime and xv-20120507-ross-combs-middle-button-paste.mime. The second patch is an update of the first patch.

From the first patch:

I don't remember which state of the patch you have, and I just did some testing after fixing the paste portion so I rediffed it.

The major parts are:

• resize support
• middle-click pasting
• option to make window geometries pretend to be user-specified

From the second patch:

I couldn't stand it. I broke down an implemented all the common select/paste editing operations and added notes to the keyboard help window. New patch attached.

I've tested it out manually but you should probably too since I may be blind to my own bugs after messing with it so long.

I took the liberty of adding a few things I found annoying to the wishlist:

• tooltips for buttons with icons
• fix meta/alt/mod1 confusion in event handling and docs
• fix strange non-ICCCM positioning which causes windows to drift
  when operations are performed and other strangeness

I guess I could also add having two-mode selection support (so you can see if the highlighted text is no longer the active PRIMARY item) -- I didn't want to mess with colors because of B&W support.

By the way, it looks like there are a lot of signed arithmetic operation overflows which GCC is warning about with -Wall. Many appear to be in buffer length checks and other security-related code.

I found a bunch of .tga files lying around that xv won't load.

I don't think xv has any sort of future on VMS, so I'll be removing all references to it in the code. If someone wants to resurrect VMS support, they're free to go digging in the repo for the deleted bits.

There are a lot of places where there are potential buffer overflows. Here are some to start with (from xv-20090131-grr-C++-comments.where)

xv.c:    strcpy(fullname, namelist[filenum]);   // 1 of 2 places fullname != const
xv.c:    *tmp = '\0';                      // 2 of 2 places fullname != const


xvevent.c:      //fprintf(stderr, "RAC: orig window pos %d,%d\n", xwa.x, xwa.y);
xvevent.c:      //fprintf(stderr, "RAC: image size now %d,%d\n", xwa.width, xwa.height);
xvevent.c:      //fprintf(stderr, "RAC: moving window to %d,%d\n", xwa.x, xwa.y);


xvgif.c:          xv_mktemp(pinfo->pagebname, "xvpgXXXXXX"); // a.k.a. close(mkstemp())


xvhips.c://extern char *calloc();
xvhips.c:  pic = pinfo->pic = (byte *) malloc(h.rows * h.cols);   // GRR POSSIBLE OVERFLOW / FIXME


xvmag.c:    *pic0  = mag_malloc((size_t) mi->width * mi->height, "mag_expand_body#2");  // GRR POSSIBLE OVERFLOW / FIXME
xvmag.c:    pixel0 = mag_malloc((size_t) 2 * mi->p_width * 17, "mag_expand_body#3");  // GRR POSSIBLE OVERFLOW / FIXME
xvmag.c:    mag.a_size = (mag.p_width * mag.p_height + 15) / 16;        /* x/2/8 */   // GRR POSSIBLE OVERFLOW / FIXME
xvmag.c:    pixel0 = mag_malloc((size_t) 2 * mi->p_width * mi->p_height,  // GRR POSSIBLE OVERFLOW / FIXME
xvmag.c:    flag0 = mag_malloc((size_t) mi->p_width * mi->p_height,  // GRR POSSIBLE OVERFLOW / FIXME
xvmag.c:    mi->a = mag_malloc((size_t) mi->a_size, "mag_compress_data#4");   // GRR POSSIBLE OVERFLOW / FIXME


xvmaki.c:    mi->vs = maki_malloc((size_t) bpl * mi->height,  // GRR POSSIBLE OVERFLOW / FIXME
xvmaki.c:    *pic = maki_malloc((size_t) mi->width * mi->height,  // GRR POSSIBLE OVERFLOW / FIXME
xvmaki.c:    mi->forma = maki_malloc((size_t) mi->width / 2 * mi->height,  // GRR POSSIBLE OVERFLOW / FIXME
xvmaki.c:    mi->formb = maki_malloc((size_t) mi->width / 2 * mi->height,  // GRR POSSIBLE OVERFLOW / FIXME
xvmaki.c:    mi->vs = maki_malloc((size_t) bpl * mi->height,  // GRR POSSIBLE OVERFLOW / FIXME
xvmaki.c:    mi->fa = maki_malloc((size_t) bpl * mi->height, "maki_make_flags#1");  // GRR POSSIBLE OVERFLOW / FIXME


xvpi.c:    *pic = pi_malloc((size_t) max_cnt, "pi_expand");   // GRR POSSIBLE OVERFLOW / FIXME


xvpic.c:    pi->data = pic_malloc(sizeof(data32) * pi->width * pi->height,   // GRR POSSIBLE OVERFLOW / FIXME
xvpic.c:            *xp = pic_malloc((size_t) pi->width * pi->height * 2,   // GRR POSSIBLE OVERFLOW / FIXME
xvpic.c:            *xp = pic_malloc((size_t) pi->width * pi->height,   // GRR POSSIBLE OVERFLOW / FIXME
xvpic.c:        *xp = pic_malloc((size_t) pi->width * pi->height * 3,   // GRR POSSIBLE OVERFLOW / FIXME
xvpic.c:    pi->data = pic_malloc(sizeof(data32) * pi->width * pi->height,   // GRR POSSIBLE OVERFLOW / FIXME


xvpic2.c:       *xp = pic2_new((size_t) pi->x_max * pi->y_max * 3, "pic2_make_xvpic");   // GRR POSSIBLE OVERFLOW / FIXME
xvpic2.c:    p = pi->buf = (byte *) pic2_new((wid + 8) * sizeof(pixel) * 3   // GRR POSSIBLE OVERFLOW / FIXME


xvpng.c:// GRR FIXME:  add .Xdefaults option to omit writing gamma (size, cumulative errors when editing)--alternatively, modify save box to include "omit" checkbox
xvpng.c:      //fprintf(stderr, "Disabling MMX read support for combining rows.\n");
xvpng.c:      //fprintf(stderr, "Disabling MMX read support for expanding interlacing.\n");
xvpng.c:      //fprintf(stderr, "Disabling MMX read support for decoding row-filters.\n");


xvtiff.c:#endif // USE_LIBJPEG_FOR_TIFF_YCbCr_RGB_CONVERSION

xv--adobe-XMP--test-image--inspections_table_2013.where
xv--adobe-XMP--test-image--tumblr_mlqyn0H7A01qb3mfao1_250.png
xv--adobe-XMP--test-image--tumblr_mlqyn0H7A01qb3mfao1_250.where

URLs where to find some test images in PNG format and the test images themselves. These two files will cause xv to crash. Somehow I was able to load the first one.

Ross Combs submitted to Greg some rather nice-looking patches. In the first set, a file xvselect.c was missing. I found it later in the second pair of files (see below) MIME-encoded.

Files involved are:
xv-20071013-ross-combs-resize-ppos-paste.msg
xv-20071013-ross-combs-resize-ppos-paste.patch
xv-20120505-ross-combs-middle-button-paste.mime
xv-20120507-ross-combs-middle-button-paste.mime

I'm documenting that stuff here so I can close Issue #4.