Re: [boost] [gil] Ideas to make my extension more generic?

From: Lubomir Bourdev (lbourdev_at_[hidden])
Date: 2007-04-24 01:38:16

Christian Henning wrote:
> Hi there, in the course of getting more familiar with gil 2.0 I
started
> writing a new extension for downsampling (don't if that's the right
> term) an source image to its unsigned 8 bit counterpart. 

Hi Christian,

It is great to see someone take a stab at providing more GIL algorithms!
You are right that downsample is not the best name for this operation.
This term is used for a different operation. You are scaling up the
histogram to the full range, so maybe scale_histogram or
spread_histogram is better.

> You would use
> such an extension when trying to save an image that is using more than
> 8 bit per channel, e.g. rgb32_image_t. Some file formats only take
> unsigned 8bit per channel.

While this algorithm is useful for increasing the contrast of the image,
I wouldn't use it as the default when saving higher bit depth images to
8-bit formats. If you want to preserve as much as possible the original
look of the image I suggest using color_converted_view to a form
supported by the I/O format.
 
> 1. How do I figure out if  the src channel type is signed? If it is I
> would like to add half the value range to all channels and pixels.

As suggested, you may want to use channel_convert. It linearly maps the
range of the source into the destination. It will add half the range
when going from signed to the corresponding unsigned integral channel.

> 2. When dealing with heterogeneous pixel types how can I initializes
> them generically?

It is hard to write code that supports heterogeneous pixels. Since each
channel may be of a different type you cannot create explicit loop. The
only way to loop is via static recursion (see static_for_each as an
example). You may use GIL's static_xxx algorithms or write your own
compile-time recursion.

> 
> 3. How do I compute the unsigned 8bit image type of the src image
type?
> I'm looking for xxx8_image_t.

If image_in_t is your input image type:

typedef typename color_space_type<image_in_t>::type cs_t;
typedef typename channel_mapping_type<image_in_t>::type cnm_t;

typedef pixel<bits8, layout<cs_t,cnm_t> > pixel_out_t;
typedef image<pixel_out_t, is_planar<image_in_t>::type::value>
image_out_t;

But in your case you probably want to operate on views, not images. Also
you may want to leave the out channel type as a template parameter
instead of hard-coding it to 8-bit unsigned.

Other suggestions regarding your code:

1. A better histogram spread computes the image histogram and uses the
top/bottom N% value as the min/max value, instead of the absolute
minimum and maximum. This is more robust to noise.

2. Consider taking out the "min pixel value" and "max pixel value" as
separate stand-alone algorithms. They could be used in other contexts. 

3. You might be able to use std::min_element, std::max_element,
boost::minmax_element coupled with nth_channel_view to find the min/max
value for each channel in the image. It is simpler though it might be
slower for interleaved images because we will be revisiting the pixels
of the image K times (K == number of channels).

4. Never take a non-constant reference to a view. Image views are always
constant. Mutability is part of their type.(line 65)

5. Don't assume that the reference type is value_type& (lines
91,120,121). In fact, this doesn't hold true for planar images. Use
View::reference or View::const_reference as the return type of
dereferencing the view iterators.

6. I would suggest making the transformation dst_max * (src - min) /
(max - min) as a per-channel function object and using static_for_each
to apply it to each channel. This is not only faster but will work for
heterogeneous channels. Another advantage is that performance
specializations can be provided for pairs of source/destination
channels. 

You can then wrap it into a function object per pixel and use
gil::for_each_pixel

Lubomir