Hi Phil, thanks for your review. It's very appreciated. On Tue, Dec 7, 2010 at 2:48 PM, Phil Endecott <spam_from_boost_dev@chezphil.org> wrote:
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.
I guess, what you're hoping for is a full-fledged image processing tool chain. That's a huge undertaking if you ask me. Not sure if the boost community could ever agree on such a huge library. Like a boost user interface library or a boost XML lib. It's just very hard to suite all use cases.
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.
Quite a monumental task, I must admit. For use cases like this I'm afraid you always have to resort to some homegrown code. I mean someone has to manage your memory and gil is not a memory manager.
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.
Mhmm, what would the first wrapper do, apart from error handling and providing a c++ interface for callbacks? What kind of buffer type would you use? I'm intrigued by your idea. But need more information.
- 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.
Do you think io_new should provide in-memory streams, as well? So many other libs do that already? I mean there is std::stringstream, boost::iostreams, Fast Format ( http://www.fastformat.org/ ), and more.
- 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.
The fix you mention was added over a year ago. Yes, like every software piece, even io_new has some bugs. ( sorry for the sarcasm ) One reason to bring io_new into boost mainstream is to have some more people to help out.
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.
Yes it's pretty complicated and finicky. I'm actually quite happy the test suite runs on Linux, as well. I don't use Linux or gcc.
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.
You're right there are cases when zero copying is possible but there might be fewer cases than you think. For instance, reading a rgb8_image_t from a bmp still requires some channel shuffling since bmp stores the image as bgr8_image_t. Regards, Christian