[Octave-patch-tracker] [patch #9729] image package -- rgb2xyz conversion with adapted whitepoint

[Hartmut]

Follow-up Comment #3, patch #9729 (project octave):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?9729>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/