Subject: Re: [boost] [Review][GIL] Boost.GIL IO and Toolbox extensions have been ACCEPTED into Boost
From: Christian Henning (chhenning_at_[hidden])
Date: 2011-01-27 15:47:04
thanks for the being such a perfect review manager. I'll address the
complains as soon as possible. Some have been already fixed.
On Thu, Jan 27, 2011 at 8:35 AM, Mateusz Loskot <mateusz_at_[hidden]> wrote:
> Dear All,
> The formal review of IO and Toolbox extensions for Boost.GIL 
> ended on December 17th, 2010 
> The review subject was not a standalone library but a part of whole
> - an extension to existing Boost.GIL.
> The review result: ACCEPTED
> Number of received discussion and feedback posts is ~140.
> I consider most of them in-topic, significant and also insightful.
> Reviewers pointed issues already known from previous discussions on the
> mailing list as well as new topics regarding the design and
> implementation of the extensions. Also, issues related to the library
> itself (Boost.GIL) were addressed. Some of them could be considered
> ideas for future works and improvements.
> It is remarkable, most (if not all) of the discussions were focused
> on the IO extension. None of the submitted reviews include
> comments to the Toolbox extension.
> Formally, the review concluded with 1 NO and 7 YES votes.
> Some of the YES votes are considered as conditional and
> number of changes were requested by the reviewers.
> The following changes should be addressed before the extensions
> appear in the Boost mainstream:
> 1. Documentation is adequate, but requires clean-up and fixing of
> lots of typos.
> 2. Additional tests are added to exercise currently untested error
> cases that result in a longjmp should be added. They should pass
> on all platforms officially supported by Boost.
> 3. Make sure the test images are license-free. If the license
> status is unknown, change them to other license-free images.
> 4. The requirements for adding new file formats / backends to be made
> more explicit. This means defining concept for a file format
> reader/writer, etc. and adding new file formats would constitute
> providing models for these concepts.
> 5. Support for reading images whose file format is unknown at compile
> time. That involves the ability to register file formats / backends,
> associate them with given extensions and then call the appropriate
> reader based on the extension of the file name.
> 6. Support for different backends for the same image formats.
> It appears that this can be done simply the same way a file format
> can be extended, i.e. have "png_libpng" and "png_lodepng" tags.
> 7. Support for efficient reading/writing of parts of images that are
> too large to fit in memory. There are two ways to proceed:
> one is to have an input/output iterator that enforces sequential
> access, and another is to allow for user-driven access at the cost
> of sometimes severe performance.
> The reviewers also posted a few suggestions that could improve
> quality of the two extensions,
> 1. Name of functions like read_int8, read_int16, read_int32, write_int8
> in Device models suggest they return and accept signed integers,
> but they return and accept unsigned integers. The naming could
> be improved..
> 2. Do some profiling and performance optimization of the most common
> formats. Add some test cases to measure speed of common scenarios.
> 3. Make sure the test cases provide complete coverage. For example,
> set a breakpoint in every method, every switch statement and branch.
> Run your tests and remove any breakpoints that are encountered.
> Look at the breakpoints that were not hit and add tests for them.
> 4. It would be great if Christian and Domagoj could join their
> efforts to take the best of their libraries (IO extension
> implementations) to the IO extension included in Boost.GIL
> and collaborate for the advanced features discussed in reviews.
> I would like to thank Edd Dawson, Zach Laine, Domagoj Saric, Roland
> Bock, Lubomir Bourdev, Tom Brinkman, Phil Endecott, Kenny Riddile for
> reviews and others who participated in discussions.
> Thanks also go to Christian Henning for great work.
> I would like conclude recalling short comment Lubomir Bourdev
> included in his review. In my opinion, it is a good summary of the
> Boost.GIL IO and Toolbox review and generally all discussions
> on this subject:
> The problem with I/O is that you can never declare success.
> All we can hope for is push the boundary as much as we can,
> and leave the rest for the next update.
>  http://lists.boost.org/boost-announce/2010/11/0273.php
>  http://lists.boost.org/boost-announce/2010/12/0279.php
> Best regards,
> Mateusz Loskot, http://mateusz.loskot.net
> Charter Member of OSGeo, http://osgeo.org
> Member of ACCU, http://accu.org
> Unsubscribe & other changes:
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk