Some bitmaps fail in GoVips but work with Vips

[jhford-scout24]

I am bringing our image scaling application up to date using govips v2. Thanks for the huge effort to bring the library up to date, it's great!

I have one small issue right now around Bitmap support. I believe that there's something in the bindings which has changed around Bitmap handling. Bitmaps generated in the OS/2 format do not load when they are loaded by govips, instead returning the error bmp: unsupported BMP image.

$ file ../testdata/koala.bmp*
../testdata/koala.bmp:  PC bitmap, Windows 98/2000 and newer format, 1024 x 768 x 24, cbSize 2359434, bits offset 138
../testdata/koala.bmp2: PC bitmap, OS/2 1.x format, 1024 x 768 x 24, cbSize 2359322, bits offset 26
../testdata/koala.bmp3: PC bitmap, Windows 3.x format, 1024 x 768 x 24, image size 2359296, resolution 2834 x 2834 px/m, cbSize 2359350, bits offset 54

These files are definitely loadable by the Vips CLI program for the installation that matches the one used by govips:

$ vips rot ../testdata/koala.bmp2 out.jpg d90
$ file out.jpg
out.jpg: JPEG image data, <snip> 768x1024 <snip>

I know this is an edge case, but I suspect it's because BMP handling is being done using the standard library? I haven't dug into it yet.

Here are the files in question. This picture was originally obtained through wikipedia I believe, and reencoded to these bitmap formats using ImageMagick Archive.zip

[jhford-scout24]

Ah, I see that BMP files are handled in a special way by using the bmp go package which is known to not support all versions of BMP

foreign.go: vipsLoadFromBuffer is the relevant function.

I'm curious why this approach was taken?

[cshum]

I also wonder why. If I need such conversion I could have call the function myself. Since this is advertised as a libvips binding, it should be as close to libvips as possible.

To make things worse, the recent changes seems to put yet another hacky layer on top of this: https://github.com/davidbyttow/govips/pull/299/files

I presume the Go BMP to PNG conversion was made for backward compatibility? I would much rather deprecate it though.