patchGNU Octave - Patches: patch #9729, [octave forge] (image) rgb2xyz...

 
 

patch #9729: [octave forge] (image) rgb2xyz conversion with adapted whitepoint

Submitter:  Pascal Dupuis <cdemills>
Submitted:  Thu 29 Nov 2018 10:45:42 AM UTC
   
 
Category:  Forge : new feature Priority:  5 - Normal
Status:  In Progress Privacy:  Public
Assigned to:  None Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 24 Dec 2018 08:16:10 PM UTC, comment #4: 

retitling

 Avinoam Kalma <avinoam>
Group Member
Thu 29 Nov 2018 06:59:20 PM UTC, comment #3: 

I've had a look at the code of this patch. My comments:

  • I would suggest to make wp_adapt.m a private function. Currently it is then only called by rgb2xyz (and makes its performance closer to Matlab performance). But I could (lateron?) also be used in more (Matlab compatible) functions, like the ones Carne mentions in comment #2.
  • The changes to "NEWS", "DESCRIPTION" and "configure.ac" in the patch file seem to be unintentional and wrong. Those should be removed and by substituted by a useful comment in NEWS only.
  • The code changes in rgb2xyz:
    • There should remain a check for the number of input arguments. Maybe it could be "nargin == 1 or ==3".
    • There are some remaining (debug) comments in the code. Those should be removed.
    • The return value MA of wp_adapt seems to be unused. What is this for?
    • Is the call to wp_adapt the wrong way around? The call is wp_adapt(xyz,source_wp,dest_wp) but the names in the function wp_adapt seem to be the other way around (source <-> dest).
    • If input and output wp are the same, then rgb2xyz should NOT call wp_adapt at all. To not introduce unnecessary rounding errors.
  • The code in wp_adapt.m:
    • You could put some more information into the help string (but this information should also go to rgb2xyz if wp_adapt becomes a private function). E.g.: The usage of icc profiles is not supported (Matlab does this). There are some Octave-Only whitepoints supported: B, D75, F2, F7, F11.
    • There are too much spaces in the definition of tri_base
    • The test code in the patch contains nothing new. Those tests are currently already part of rgb2xyz and should probably stay there.
    • Some (new) tests for the performance of wp_adapt would be very good. They can maybe go to rgb2xyz as well. They should for example test the new rgb2xy(..., wp, 'A') functionality. You also mentioned (in a private email) some other tests (humhum, to the last digits), please add them if possible. If you have test cases (maybe from OpenCV), then please add them.


If you need Matlab outputs, you can send my the test case code, and I can then run them in Matlab and let you know the output values.

 Hartmut <hardy>
Thu 29 Nov 2018 03:06:56 PM UTC, comment #2: 

The feature would be nice indeed but we should do this in a Matlab compatible manner instead of adding this wp_adapt.

I believe this would mean implementing the functions whitepoint, makecform (AdaptedWhitePoint option), and applycform.

 Carnë Draug <carandraug>
Group Member
Thu 29 Nov 2018 01:53:45 PM UTC, comment #1: 

(adding people to cc)

 Hartmut <hardy>
Thu 29 Nov 2018 10:45:42 AM UTC, original submission:  

Hello,

the enclosed patch implements White Point adaptation as explained at http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html

The idea is to take some input in RGB with some color point, and convert it to XYZ which by default use a "flat" (equispectral) reference.

I need this in order to extract luminances from picture. Our lab disposes of a luminance calibrator with an incandescent ('A") ref; I would like to implement luminance extraction whatever the WP of the camera.

Regards

Pascal

 Pascal Dupuis <cdemills>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #45551:  wp_adapt.pat added by cdemills (9KiB - image/x-coreldrawpattern)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by hardy
  • -email is unavailable- added by hardy
  • -email is unavailable- added by cdemills (Submitted the item)

    •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-12-24 avinoam Summary [octave forge] (image) package -- rgb2xyz conversion with adapted whitepoint [octave forge] (image) rgb2xyz conversion with adapted whitepoint
    2018-12-24 avinoam Summaryimage package -- rgb2xyz conversion with adapted whitepoint [octave forge] (image) package -- rgb2xyz conversion with adapted whitepoint
    2018-11-29 carandraug Summaryimage package -- RGB2XYZ function image package -- rgb2xyz conversion with adapted whitepoint
    2018-11-29 hardy Carbon-Copy- Added avinoam
        Carbon-Copy- Added carandraug
    2018-11-29 cdemills Attached File- Added wp_adapt.pat, #45551

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.13-df36.
    Corresponding source code