Re: [boost] [gil] New IO release

Subject: Re: [boost] [gil] New IO release
From: Christian Henning (chhenning_at_[hidden])
Date: 2010-10-18 10:53:04

Hi there, good to hear from you.

>
> Your interface uses free (global) functions (like
> xxx_read_and_convert_image()...) while my interface uses objects/classes and
> corresponding member functions...

OK, now I understand. The new interfaces don't have the xxx_ anymore.
Now we have:

read_image( filename, gil_view, xxx_tag() );

The reason I choose this unified interface is first of all historical.
The current gil::io uses free functions as well, which are by the way
not global but inside a namespace. I also provide header files which
supports the old styles to avoid breaking existing code. The second
reason is the ease of use. I believe using these free functions is
easy for users to understand since they all have their own distinct
behavior and meaning.

>
>> Why not use std::streams
>
> Streams are _evil_ ...
> http://lists.boost.org/Archives/boost/2010/01/160911.php ...
>
>
>> and what are the alternatives?
>
> In my book anything is an alternative to screams :D
>
> OTOH, only providing a streams based interface as an additional option is
> usually just fine (as opposed to forcing a streams only based interface)
>

Don't wanna get into a "streaming" fight here. Point is noted. Did you
know there are several fast streams implementation available on the
net? They might change some arguments.

>
>> WIC?
>
> http://msdn.microsoft.com/en-us/library/ee719902(VS.85).aspx
>

Looks interesting.

>
>>> Unlike io and io_new it uses objects that represent on-disk images
>>> ("formatted images" in io2-speak). This has several advantages:
>>
>> Could you go into more details? What are "formatted images"?
>
> Well, images in different 'formats' like JPEG, PNG, GIF...obviously not a
> very clever name that I just came up with to somehow distinguish them from
> 'raw'/in-memory images (like gil::image<>)...you are more than welcome to
> come up with a more intuitive name ;)

How about in-memory vs. of-file? ;-)

>
> (AFAICT) You can reuse the low level file object but not the backend library
> object, which is what I was talking about and what is prerequisite for, for
> example, efficient 'moving ROI'/sequential-in-blocks image reading...

Are you talking about FILE* only or does that include std::ifstream as well?

>
>>>    libtiff_image my_tiff( "my_tiff.tiff" );
>>>    ::TIFFSetDirectory( &my_tiff.lib_object(), <a directory number> );
>>>    my_tiff.copy_to(....);
>>
>> That's a neat feature that I like to have too.
>
> But I don't see how you can have that with free functions...

Maybe I should give out access to my reader< ..., xxx_tag, ... > objects.

> No jamfile yet (Boost build/bjam are definitely not one of my favourite
> subjects or skills for that matter :)
> OTOH, I'm not quite sure (with my limited bjam knowledge) why/how would
> different backends influence the library's jamfile (atleast as long as it is
> header only)..?

Check out Mateusz's blog:

http://mateusz.loskot.net/2010/10/17/notes-on-boost-build-for-boost-gil/

>> I would love to know how you seek through a image using a 3rd party
>> lib, for instance with libjpeg. Right now I'm just reading and
>> discarding unwanted regions. Not the most ideal solution to say the
>> least.
>
> With LibJPEG you cannot really/literally skip unwanted data but you can make
> the library read the unwanted data using faster/less precise methods which
> is what I currently do (and admittedly use a bit of LibJPEG's internal
> implementation detail knowledge for that)...

Cool, I would love to know more. Can you point where in your code you do that?

>
>> Not sure what synchronize_dimensions and assert_formats_match stands
>> for.
>
> synchronize_dimensions is a policy that causes the target image to be
> resized to fit the source image (it is obviously not valid for views),
> alternatively assert_dimensions_match and ensure_dimensions_match (throws in
> case of mismatch) can be used...
> For formats there are the corresponding assert_formats_match,
> ensure_formats_match and synchronize_formats (with the latter performing a
> builtin conversion if supported by the backend otherwise it uses GIL's
> default colour converter) with the addition that a user specified colour
> converter can be passed instead of the three mentioned predefined
> policies...

For synchronize_dimensions I have read_and_convert_image() which takes
a custom color converter as on of the parameters. In case the users
doesn't want to convert he/she would you read_image() but then the io
extension would check of the user supplied image is compatible to the
on-file image. Compatible images means both color spaces are
compatible and all channel's are pairwise compatible.

>>> - LibJPEG was additionally compiled with the NO_GETENV macro and LibPNG
>>> with
>>> the PNG_NO_CONSOLE_IO (additionaly for io2 it was built with
>>> PNG_NO_STDIO,
>>> PNG_NO_WARNINGS, PNG_NO_ERROR_TEXT macros, io and io_new would not link
>>> with
>>> those macros)
>>
>> When using these compiler symbols do you experience significant speed ups?
>
> Not (direct) speedups but smaller binaries (when coupled with custom error
> and IO handling routines) because of removal of stdio code and error message
> tables from the binary...

Good to know.

>>> - I don't know if I missed something or did something seriously wrong but
>>> the io_new TIFF reader seems broken in the sense that it does not do any
>>> pixel/image format conversion and thus works properly only when the
>>> destination image/view is in the same format as the image on disk....?
>>
>> I'll investigate.
>
> Have you perhaps had the chance?

Yep, working on a fix. Tiff is very flexible and reading files without
conversion works fine. But when reading with conversion I get into a
problem with template member overloading. Basically I have a switch
statement that would overload the read_data member with the on-file
pixel format as template parameter. Now this pixel parameter can be
anything for gray1 to rgb23_55_12, etc. Which makes the switch
statement approach not feasible. I'm inclined to only allow
read_and_convert_image/view for the most common channel sizes.

>
>> That sounds great. I have to look at how you deal with certain
>> functionalities, like reading ROI in a jpeg image.
>
> Actually, I forgot to mention, with the LibX (JPEG, PNG, TIFF) backends I do
> not provide 'full'/2D/rectangular ROI capability but only 'vertical' ROIs
> (i.e. you can specify from which to which row/scanline to read but whole
> rows/scanlines are always read...) because the backends do not support
> reading partial rows/scanlines so adding full/proper ROIs for those backends
> would require emulation which in turn would cause code bloat/complication
> and (possible) redundant data copying...For now it seems to me that the user
> should handle 'full' ROI support in such cases (if required)...

I try to allow all sizes of ROI. It can be anything from a single
pixel to a scanline or a vertical line to a rectangle. This was  bit**
to implement for tiff's tiled images. ;-)

The basic algorithm is to read scanline by scanline and decided if
there are portions that are needed or not. Same for tiles.

>
> Place a breakpoint in the std::string constructor...it is mostly due to the
> change you made to the io_error() helper function (to take a std::string
> const & instead of char const *)...
>

You're absolutely right. Are you recommending a string table?

> For example not performing a full check-and-throw on every library call but
> accumulating multiple results and throwing in one place/out of a loop...
>
> Error reporting 'level'/complexity should ideally be configurable by the
> user (which is something I'd like to see on a global Boost level) in the
> sense that the user can choose whether errors should be reported with a
> simple std::exception with a single/simple hardcoded message (like "GIL IO
> failure") or a with a full std::ios_base::failure filled with detailed and
> formatted error messages provided by the backend library or with something
> inbetween...

Sounds interesting. I might add that after the review depending on the
reviewers input.

>> Any idea of avoiding setjmp in a portable manner?
>
> It cannot be avoided using only standard C and C++ because/if the libraries
> use setjmp. My code also uses/works with setjmp unless
> BOOST_GIL_THROW_THROUGH_C_SUPPORTED is defined (which is automatically
> defined for MSVC++)...

Are you suggesting your lib is not header-only?

>> In conclusion, I must say I'm impressed with your io2. I think there
>> are a lot of things we both can reuse and add into our projects. Maybe
>> a merge could be possible. One of my highest goals is to support as
>> many image formats as possible. For instance, a lot of work went into
>> the tiff extension since it's to most flexible. Here, all bit image
>> types are supported, for instance rgb7_14_12 in a tiled or strip
>> manner.
>
> Well yes, your proposal goes further than just IO with for example the
> toolbox extension...although I had an idea, since the beginning of my
> dealing with GIL IO, that should probably go to the toolbox extension and
> that is support/adapters for image classes from various GUI frameworks that
> would make them conform the the GIL image concept. One more thing I forgot
> to mention is that I already made some steps in that direction with the GDI+
> backend that make a gp_image behave directly like a gil::any_image<>
> (without the need to call copy_to...(...) on a gp_image object to read its
> contents)...
> The massive work you did with the wrapping of all the image/reading/writing
> properties that the individual backends provide can also prove to be
> useful...
> However a (relatively simple) merge does not seem possible because of the
> radically different interfaces (as well as implementations)...

You're more than welcome to add stuff into the toolbox extension. Let me know.

Thanks again for your insightful comments,

Christian