Hi Mateusz, thanks for the being such a perfect review manager. I'll address the complains as soon as possible. Some have been already fixed. Regards, Christian On Thu, Jan 27, 2011 at 8:35 AM, Mateusz Loskot <mateusz@loskot.net> wrote:
Dear All,
The formal review of IO and Toolbox extensions for Boost.GIL [1] ended on December 17th, 2010 [2] 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. ***
[1] http://lists.boost.org/boost-announce/2010/11/0273.php [2] 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: http://lists.boost.org/mailman/listinfo.cgi/boost