Re: Move rgb2ntsc and ntsc2rgb to image package

[Rik]

On 01/17/2018 07:20 AM, Carnë Draug wrote:
> On 17 January 2018 at 05:25, Rik <address@hidden> wrote:
>> On 01/16/2018 10:04 AM, Carnë Draug wrote:
>>> On 16 January 2018 at 16:06, Rik <address@hidden> wrote:
>>>> Carnë,
>>>>
>>>> As you are maintainer of the image package, I'm writing to coordinate on
>>>> some of of the lesser used image functions in core Octave that should move
>>>> to the image package.  This sort of clean-up has already been done for the
>>>> statistics functions.
>>>>
>>>> It seems like rgb2ntsc and ntsc2rgb are pretty specific and should be moved
>>>> to the image package.  However, the general functions rgb2gray and gray2rgb
>>>> which use the luminance channel of rgb2ntsc should be written.  I don't
>>>> mind doing that if you can take care of incorporating the other two into
>>>> the image package.
>>>>
>>>> Cheers,
>>>> Rik
>>> ok. I think there's more functions like ind2gray and gray2ind so let
>>> me know.  I have a patch ready for the functions you mentioned.
>>>
>>> Carnë
>> I added rgb2gray.m here
>> (http://hg.savannah.gnu.org/hgweb/octave/rev/3ad53e4793fc).  I removed
>> rgb2ntsc.m and ntsc2rgb.m here
>> (http://hg.savannah.gnu.org/hgweb/octave/rev/afbef2f579c9).  It should be
>> okay to push your patch for the image package now.
> Unlike the version of rgb2ntsc, rgb2gray should return the same data
> type as the input image.  On the image package, that happens with the
> imcast/im2X functions but they are not in core [1].  Not sure how to
> best handle this in core without having those functions duplicated.
If that is the required behavior then I don't think there's any way to
avoid it re-coding some of imcast in to rgb2gray.  I will take a look at
that later.

>
> Also, despite what Matlab says on the documentation, they don't seem
> to use that transformation matrix with only 3 decimal cases:
>
>     >> T = rgb2gray ([1 0 0; 0 1 0; 0 0 1]);
>     >> T(:,1)
>     ans =
>        0.298936021293776
>        0.587043074451121
>        0.114020904255103
>
> so might be better to had a few more decimal cases.  At least so it's
> compatible with the existing function in the image package.
Sure, I'll change it to keep 6 significant figures.  Most LCD displays are
going to have, at best, 1/255 precision in display or .004.  Assuming some
display technology that we regularly use eventually has uint16 precision
that would be 1 part in 2^16 or just 1.5e-5 so keeping the matrix rounded
to 1e-6 should be good enough.

>
> Finally, the reshaping back to the original shape can be done with the
> existing private function colorspace_conversion_revert [2].
I tried using that function at first, but had to give up because I couldn't
get ND-images to work correctly.  The problem is that, unlike rgb2ntsc, the
third dimension is removed.  I can't mimic that by just deleting the
dimension from the sz vector because colorspace_conversion_revert expects 4
dimensions.  The code in question is

    if (is_nd)
      rv = reshape (rv, [sz(1:2) sz(4) sz(3)]);
      rv = permute (rv, [1 2 4 3]);

We could recode this to be more general, or we could just make rgb2gray a
one-off function.  I'm inclined to do the latter, but if you see a strong
reason for modifying colorspace_conversion_revert we could do that.

--Rik