Re: [boost] GIL io_new review

Subject: Re: [boost] GIL io_new review
From: Christian Henning (chhenning_at_[hidden])
Date: 2010-12-07 18:13:45

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_at_[hidden]> 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