Looks good.  I've tested it on big- and little-endian images, and both
load fine.  Some comments:

  1) This was missing from my original commit, but the pfm export code
     at src/imageio/format/pfm.c also needs a similar treatment.  It
     always exports as little-endian, which is fine, but it needs to
     ensure that the exported data is actually little-endian.

  2) You use a variable 'cols' to describe the number of channels in the
     image.  Given that the number of columns in the image is actually
     stored in img->width, it may be better to change the name of 'cols'
     to something like 'chans'.

  3) The branches for 3 channels and one channel are needlessly
     different.  If you pulled out the fread call, you could write it
     like this and it would suffice for both branches:

     ret = fread(buf, chans*sizeof(float), (size_t)(img->width * img->height), 
f);
     if(ret != img->width * img->height)
       goto error_corrupt;

     This also has the advantage of reporting truncated files as
     corrupt, whereas the current code will return an image with the
     truncated region uninitialized.

  4) There is no need for the loop on line 67 to run backwards, nor for
     there to be two separate loops on lines 77 and 78.

Thanks for getting this support landed.

Taahir

Attachment: signature.asc
Description: This is a digitally signed message part.

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
darktable-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/darktable-devel

Reply via email to