Back in 2006 I had some code that used ImageMagick's Magick++ library to read and write images in various formats and do some basic processing. That library works well and is very feature-full, but is not what you would call "Boost like", so I was excited when GIL came along. I looked at it at the time of its review, and I thought at first that I must have missed a big chunk of the documentation where all the algorithms were described! Of course, it has no image processing algorithms; it's just a layer; if you want to scale or rotate or merge images, you have to do that yourself. I don't think I wrote a review in the end; my feeling was - in comparison with ImageMagick - "come back when you've finished" - but I didn't want to say that in public. So it's great that now, 4 years later!, we are finally getting some GIL extensions. Thank you Christian for - I hope! - getting the ball rolling. I hope we'll see more. Having said that, I suspect that even with lots of extensions GIL might not really be what I need. Here's a typical problem that I will have to address in the next few weeks: I have a 1e12 pixel image - a map - which is supplied as a few thousand 5000x5000 TIFF tiles; I have to join them together, tweak the colours, draw some lines on top, and re-chop it into a few million 256x256 PNG tiles. The issue of course is that that image can't be held in RAM. I will do this with my own wrappers around libtiff and libpng, in rows or chunks. So, do the image concepts (and concrete classes) that GIL defines accommodate that sort of problem? Even if they do, is a "virtual image" of some sort a good way of doing it? I suspect not. But even if the image concept is not useful, presumably other GIL concepts like pixel formats and algorithms in future extensions (pixel format conversion, blending) could still be useful. So, along comes Christian's IO extension. Wrapping the "legacy" image libraries is something that I have done a few times now and it can be painful, not least because of the need to read a lot of documentation; the libraries all differ and it would be great to get a unified set of wrappers that provide a sane C++ interface. But as you can see, I don't really want something that reads and writes complete GIL images, and that is what Christian's extension does. Ideally, this extension would have separated out the wrapping of the libraries from the actual GIL image integration, with the GIL image integration just being a layer on top of the image library wrappers. Since it doesn't do that, it's not much use to me. It is, however, better than what GIL currently has. So for that reason I suggest that it should be accepted.
- What is your evaluation of the design?
In addition to my comments above, I note that it's necessary to jump through hoops (using Boost.IOStreams) just to read an image from memory. Surely this should be a fundamental operation; it would be much better to have a ctor that takes begin and end pointers for the encoded image data.
- What is your evaluation of the implementation?
When I first looked at the library I noticed that the error handling was clearly broken. Although that seems to have been resolved, it has given me a poor impression of the quality (i.e. likely correctness) of the implementation. It seems that this error handling has received almost no testing. Thorough testing will be difficult for this library because as well as all the usual permutations of platform features, compilers etc. the variations in the image libraries must also be considered. The library version and compiler settings could both affect the correct operation of this code. I have some concerns about the likely performance of the code due to excessive copying of the data both in and out of the image library. Although I've not measured the performance impact, I feel it would be very desirable (and is certainly possible) for this library to have zero copying.
- What is your evaluation of the documentation?
Satisfactory. (As a meta-comment - I think libraries being reviewed really should have their docs available online somewhere, and preferably also their source code in a source browser of some sort. In fact, I thought this was already a requirement. Can it be added to the review managers' checklist for future reviews?)
- What is your evaluation of the potential usefulness of the extensions?
It's better than what was there before.
- Did you try to use the extensions? With what compiler? Did you have any problems?
No, I've not compiled anything.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
Probably half a day during the review, and a similar amount of time a few months ago.
- Are you knowledgeable about the problem domain?
I've written a few wrappers for libjpeg, libpng and libtiff. I've never used GIL though I've read extensively about it.
And finally, every review should answer this question: - Do you think the extensions should be accepted as a part of Boost.GIL library?
On the grounds that this is better than what is currently there, and on the assumption that Domagoj Saric is not imminently going to post something else that would supersede this, I believe that it should be accepted. (I have not looked at the "toolbox" at all, and I've not seen it mentioned in any other reviews yet. My "yes" is just for the io_new functionality.) Regards, Phil.