[GRASS-dev] Modified i.rgb.his

Attached here-in a slightly updated diff (from the last diff attached in
#774) for i.rgb.his.

I need time to learn/understand things (in C). Yet, I tested the "new"
version and it works for me.

I would like to push this version into SVN myself. Thus, I'd like to ask
(in the grass-psc list) for commit access. However, I can just submit
another, updated, diff, in ticket #774 for someone to take over.

Modifying i.his.rgb, accordingly, is halfway done. I mostly learn how to
use pre-existing C functions, and this is a bit time-consuming.

These modifications would free-up also subsequent modules who use
i.[rgb|his].[his|rgb] to compute maps of more than 8-bit. For example,
i.pansharpen [see tickets #774, #2048]

Thanks for any attention.
Nikos

(attachments)

i.rgb.his.diff (15.2 KB)

On 27/08/16 15:54, Nikos Alexandris wrote:
> Attached here-in a slightly updated diff (from the last diff attached in
> #774) for i.rgb.his.
>
> I need time to learn/understand things (in C). Yet, I tested the "new"
> version and it works for me.
>
> I would like to push this version into SVN myself. Thus, I'd like to ask
> (in the grass-psc list) for commit access. However, I can just submit
> another, updated, diff, in ticket #774 for someone to take over.

Please do that. Core svn access takes time, and in any case it is better to have everything documented in the trac ticket.

When I apply this patch I get some compiler warning that need to be fixed first.

Also, could you please document the bit_depth parameter in i.rgb.his.html ?

One thing I don't understand about the results:

Using the NC landsat images and running

i.rgb.his r=lsat7_2002_30 g=lsat7_2002_20 bl=lsat7_2002_10 h=h8 i=i8 s=s8 bit_depth=8

and

i.rgb.his r=lsat7_2002_30 g=lsat7_2002_20 bl=lsat7_2002_10 h=h16 i=i16 s=s16 bit_depth=16

I then get for intensity:

> r.info -r i8
min=0.08431373
max=1

> r.info -r i16
min=0.000328069
max=0.003891051

Is this expected, or is there an issue with scaling the 16-bit version between 0 and 1 ?

Moritz

Nikos Alexandris wrote:

> Attached here-in a slightly updated diff (from the last diff attached in
> #774) for i.rgb.his.
>
> I need time to learn/understand things (in C). Yet, I tested the "new"
> version and it works for me.
>
> I would like to push this version into SVN myself. Thus, I'd like to ask
> (in the grass-psc list) for commit access. However, I can just submit
> another, updated, diff, in ticket #774 for someone to take over.

Moritz Lennert:

Please do that. Core svn access takes time, and in any case it is better
to have everything documented in the trac ticket.

Absolutely.

When I apply this patch I get some compiler warning that need to be
fixed first.

Also, could you please document the bit_depth parameter in i.rgb.his.html ?

One thing I don't understand about the results:

Using the NC landsat images and running

i.rgb.his r=lsat7_2002_30 g=lsat7_2002_20 bl=lsat7_2002_10 h=h8 i=i8 s=s8 bit_depth=8

and

i.rgb.his r=lsat7_2002_30 g=lsat7_2002_20 bl=lsat7_2002_10 h=h16 i=i16 s=s16 bit_depth=16

I then get for intensity:

> r.info -r i8
min=0.08431373
max=1

> r.info -r i16
min=0.000328069
max=0.003891051

Is this expected, or is there an issue with scaling the 16-bit version
between 0 and 1 ?

Yes, expected. The correct way to compare is:

# raw input is 8-bit
for BAND in 10 20 30; do echo "Band ${BAND}:" `r.info -r lsat7_2002_${BAND}` ;done

Band 10: min=42 max=255
Band 20: min=28 max=255
Band 30: min=1 max=255

# thus, rescale to 16-bit
for BAND in 10 20 30; do r.rescale lsat7_2002_$BAND from=0,255 output=lsat7_2002_${BAND}_16 to=0,65535 ;done

# then convert
i.rgb.his r=lsat7_2002_30_16 g=lsat7_2002_20_16 bl=lsat7_2002_10_16 h=h16 i=i16 s=s16 bit_depth=16

# now "input" will be
for BAND in 10 20 30; do echo "Band ${BAND}:" `r.info -r lsat7_2002_${BAND}_16` ;done

Band 10: min=0 max=65536
Band 20: min=0 max=65535
Band 30: min=0 max=65535

# maybe wrong the above -- should use real "min" instead of 0?

# then compare
for DIMENSION in h s i; do echo `echo "${DIMENSION} 8:" && r.info -r ${DIMENSION}8` && echo `echo -e "${DIMENSION} 16:" && r.info -r ${DIMENSION}16` ;done

h 8: min=0 max=359.434
h 16: min=0 max=359.434
s 8: min=0 max=1
s 16: min=0 max=1
i 8: min=0.08431373 max=1
i 16: min=0.08431373 max=1

Several points to discuss/answer:

- Autodetect input's bitness?

- Still enable the user to control input bitness -- We can imagine to
  even instruct `bit_depth=11` or `bit_depth=12`. Makes sense?

- 8-bit (or any bitness!) images should fail to convert, and report, in
  case the user tries to "fake" them as being 16-bit (or higher than
  what the input really is)

- A test for an open half ended range, ie [0, 2^bit-depth),
  is required too. That is inputs ranging in [0, 2^bit-depth] should
  fail. Makes sense?

- Still to do: set hue to NULL if chroma = 0

- More?

Nikos

Nikos Alexandris:

[..]

> I would like to push this version into SVN myself. Thus, I'd like to ask
> (in the grass-psc list) for commit access. However, I can just submit
> another, updated, diff, in ticket #774 for someone to take over.

Moritz Lennert:

[..]

When I apply this patch I get some compiler warning that need to be
fixed first.

Also, could you please document the bit_depth parameter in i.rgb.his.html ?

Will take care about the above observations too. Would it make sense to
upload in my sandbox (svn in grass-addons)? I want to avoid
re-submitting multiple patches as I am experimenting.

Nikos

On 31/08/16 08:44, Nikos Alexandris wrote:

Nikos Alexandris:

[..]

I would like to push this version into SVN myself. Thus, I'd like to ask
(in the grass-psc list) for commit access. However, I can just submit
another, updated, diff, in ticket #774 for someone to take over.

Moritz Lennert:

[..]

When I apply this patch I get some compiler warning that need to be
fixed first.

Also, could you please document the bit_depth parameter in i.rgb.his.html ?

Will take care about the above observations too. Would it make sense to
upload in my sandbox (svn in grass-addons)? I want to avoid
re-submitting multiple patches as I am experimenting.

Yes, good idea. Put a new patch into trac once you feel it is closer to final.

Moritz

Updated i.rgb.his in sandbox.

As part of my learning process, I chose to not use mapcalc (as suggested in
ticket #774 by Glynn). Will do so later on, if mapcalc is better.

I seek for assistance regarding the following:

1) on opening files

The following code works:

globals.h:

void openfiles(char *, char *, char *,
               char *, char *, char *,
               int[3], int[3],
               DCELL *[3]);

openfiles.c:

void openfiles(char *red, char *green, char *blue,
               char *hue, char *saturation, char *lightness,
               int fd_input[3], int fd_output[3], DCELL * rowbuffer[3])
{

    /* open input files */
    fd_input[0] = Rast_open_old(red, "");
    fd_input[1] = Rast_open_old(green, "");
    fd_input[2] = Rast_open_old(blue, "");

    /* open output files */
    fd_output[0] = Rast_open_fp_new(hue);
    fd_output[1] = Rast_open_fp_new(saturation);
    fd_output[2] = Rast_open_fp_new(lightness);

    /* allocate row buffers */
    rowbuffer[0] = Rast_allocate_d_buf();
    rowbuffer[1] = Rast_allocate_d_buf();
    rowbuffer[2] = Rast_allocate_d_buf();

}

I want to let `Rast_map_type()` retrieve the data type (see r.example) and use `Rast_allocate_buf(data_type)` instead.
Trying something like:

    const char *mapset;
    RASTER_MAP_TYPE red_data_type;

    /* get mapset (NULL if map not found in any mapset) */
    mapset = (char *) G_find_raster2(red, "");
    if (mapset == NULL)
        G_fatal_error(_("Raster map <%s> not found"), red);

    /* determine input raster map type */
    red_data_type = Rast_map_type(red, mapset);

    /* allocate the cell row buffer */
    rowbuffer[0] = Rast_allocate_buf(red_data_type);

This segfaults because the returned data_type is NULL.

2) Much like in `r.example`, I unsuccessfully try to define, in `globals.h` and
subsequently in `openfiles.c` the following:

void openfiles(char *, char *, char *,
               char *, char *, char *,
               int[3], int[3],
               void *[3]);

and

void openfiles(char *red, char *green, char *blue,
               char *hue, char *saturation, char *intensity,
               int fd_input[3], int fd_output[3],
               void *rowbuffer[3])

and whatever follows up in `main.c` (and elsewhere).

3) How to I print a DATA_TYPE?, ie:

    G_debug(1, "Red input raster map is of type: %?", red_data_type);

   Is data_type, as returned by `Rast_map_type()` a string?

4) Again, like in `r.example`, I would like to split `rowbuffer` in main(), in
`input_rowbuffer` and `output_rowbuffer`.

Thank you, Nikos

Dear all,

instead of modifying i.rgb.his and i.his.rgb, which, in my humble view, would
take time to be accepted, even in trunk, I decided to derive to new modules:
i.rgb.hsl (https://trac.osgeo.org/grass/browser/sandbox/alexandris/i.rgb.hsl)
and i.hsl.rgb (https://trac.osgeo.org/grass/browser/sandbox/alexandris/i.hsl.rgb).

Obviously, they convert values between the RGB and HSL color spaces. The `bits`
option allows for arbitrary color depths from 2 up to 16. Maybe there is need
for even higher number of colors per image?

It is possible to make a roundtrip from rgb to hsl and back to rgb landing to
the exact same ranges (not 1:1 identical values). The output from i.hsl.rgb is
FCELL though I can imagine that it could be well desired to be, also, possible
to get simply CELLs.

This is a call for testers and assistance in any way that would improve them.
I would like to ask for access to svn, but maybe it’s too early.

Wish to see ticketis #774 and #2^11 in trac resolved :slight_smile:

Thanks, Nikos

On Fri, Sep 9, 2016 at 10:13 AM, Nikos Alexandris <nik@nikosalexandris.net>
wrote:

instead of modifying i.rgb.his and i.his.rgb, which, in my humble view,
would
take time to be accepted, even in trunk, I decided to derive to new
modules:
i.rgb.hsl (https://trac.osgeo.org/grass/browser/sandbox/alexandris/i.r
gb.hsl)
and i.hsl.rgb (https://trac.osgeo.org/grass/browser/sandbox/alexandris/i.h
sl.rgb).

This is a very good idea. If it is a separate functionality, it is better
to have a separate module. They can go just to the Addons.

* Vaclav Petras <wenzeslaus@gmail.com> [2016-09-10 18:56:22 -0400]:

On Fri, Sep 9, 2016 at 10:13 AM, Nikos Alexandris <nik@nikosalexandris.net>
wrote:

instead of modifying i.rgb.his and i.his.rgb, which, in my humble view,
would
take time to be accepted, even in trunk, I decided to derive to new
modules:
i.rgb.hsl (https://trac.osgeo.org/grass/browser/sandbox/alexandris/i.r
gb.hsl)
and i.hsl.rgb (https://trac.osgeo.org/grass/browser/sandbox/alexandris/i.h
sl.rgb).

This is a very good idea. If it is a separate functionality, it is better
to have a separate module. They can go just to the Addons.

We still need to make HIS capable for >8-bit though. I did some tests, based on
HSL to replicate the pan-sharpening process, but cannot arrive to
visually expected images.

N

Hi

for easier testing, I have enabled g.extension support in the "sandbox":

GRASS 7.2.svn (nc_spm_08_grass7):~ > g.extension i.rgb.hsl
url=trac.osgeo.org/grass/browser/sandbox/alexandris/i.rgb.hsl
Fetching <i.rgb.hsl> from
<https://trac.osgeo.org/grass/browser/sandbox/alexandris/i.rgb.hsl?format=zip&gt;
(be patient)...
Compiling...
main.c: In function ‘main’:
main.c:127:9: warning: this ‘for’ clause does not guard...
[-Wmisleading-indentation]
         for (band = 0; band < 3; band++)
         ^~~
main.c:131:13: note: ...this statement, but the latter is misleadingly
indented as if it is guarded by the ‘for’
             rgb_to_hsl(rowbuffer, cols, max_colors);
             ^~~~~~~~~~
Installing...
Installation of <i.rgb.hsl> successfully finished

Likewise for
g.extension i.rgb.hsl
url=trac.osgeo.org/grass/browser/sandbox/alexandris/i.hsl.rgb

cheers
Markus