Re: imread

[Thomas L. Scofield]

On Jul 14, 2008, at 4:14 PM, John W. Eaton wrote:

On 12-Jul-2008, Thomas L. Scofield wrote:

| I hope this is what is needed.  I'm sending two changesets (or  

| whatever is produced by "export tip"), lamely giving them the  

| extension .changeset.  (I'm not sure what the usual extension is.  I  

| think I sometimes see .ksh files in the archives.)

I don't care what you call the file, but it is slightly easier for me

if you send text/plain attachments instead of binary attachments.

I'm not exactly sure what to do differently.  The files on my machine are text files.  I included them as attachments (email program is the Mac Mail program).  At what stage would they have become binary files?

I applied your patches and then made some additional changes.

I moved imread.m to scripts/image and also added it to the list of

files in scripts/image/Makefile.in.

Please take a look at the changeset below to see the additional

changes I made, including:

  check for graphics magick config script in Octave's configure script

  use graphics magick configure script to set 

  note in octave_config_info

  add __magick_read__.cc to list of files in src/Makefile.in

  add rules in src/Makefile.in so proper flags are used to compile and

  link __magick_read__.cc

  eliminate "using namespace ..." directives

  include config.h, not octave/oct.h

What's the reason behind these last two?  Am I correct, in the case of the last one, that inclusion of oct.h pulls in config.h as well?  If so, is the need for only a smaller set of declarations something that, as the principle architect of the package, you (and perhaps only a handful of others) would be able to determine efficiently, or is there some process a novice like me could use?  The only documentation I've seen (Appendix A of the manual) suggests that you generally just go ahead and include oct.h when you write dynamically linked functions.

  other style fixes

| There is more to  

| do, I know.  I'm starting to learn the Magick++ API for the purpose  

| of removing all system calls (assuming that is possible), so that  

| imread() works without having either ImageMagick or GraphicsMagick  

| installed on one's local machine.  That is why I see no point in  

| changing the current system calls to "identify" and  

| "convert" (ImageMagick routines) to "gm identify", etc.

I don't understand why any of that is present in imread.  Shouldn't it

just be a simple wrapper around __magick_read__?  And if that is all

it is, then I think we could just rename __magick_read__ to be imread,

and eliminate imread.m.

I think the same thing, but I wasn't the code's original author.  To date, I've changed only about a line or so of the code.  I would like to write a modified __magick_read__.cc routine for all of the handling and information-gathering of the image files themselves.  I'm sure the more mundane things can be wrapped there as well, but given where I am in the knowledge of C++ these days, I'd rather not have to deal with the appropriateness of arguments to imread there.  There's already a bit of argument checking in the existing imread routine, and my recollection of C++ file handling in general is that it is harder to check if files exist, etc. than the commands provided in Octave (not to mention that the commands provided in Octave are, I presume, platform independent).

If you update your sources and run

  ./autogen.sh

  configure ...

  make ...

then __magick_read__ should be built now.  When I use it, it fails to

read jpg files correctly.  When I do

  x = __magick_read__ ("foo.jpg");

  imshow (x);

(for any jpg file) the displayed image appears washed out (too

bright).  If I change 

  unsigned int

  scale_quantum_to_depth (const Magick::Quantum& quantum, unsigned int depth)

  {

    return (static_cast<unsigned int> (static_cast<double> (quantum)

      / MaxRGB * ((1 << depth) - 1)));

  }

to

  unsigned int

  scale_quantum_to_depth (const Magick::Quantum& quantum, unsigned int depth)

  {

    return (static_cast<unsigned int> (static_cast<double> (quantum)

      / 1024 * ((1 << depth) - 1)));

  }

my test image looks OK, but I don't think this is a proper fix.  What

is the purpose of this function?

I'll dig into it to see if I can determine what its purpose is.  Is the issue you mention the same one discussed between Soren Hauberg and me soon after I submitted the files for review (a couple of weeks ago)?  I get washed-out pictures not only with jpegs (that type of file, in the one instance I've tried, has not been particularly egregious), but also with png files (some really bad examples).  But the problem only exists when I am doing things on my linux machine. Assuming nothing I've done has changed the behavior of imread drastically, I find the pictures appear like they ought when displayed using imread on my mac (there I am using the original imread from octave-forge).  To understand that comment fully, I guess you need to know that the problem does not arise on either machine when I display these images with either "display" (ImageMagick command) or "gm display" (GraphicsMagick).

---

Just got your message about it being imshow.  Can it be the source of the problem I describe here, too?

Thomas L. Scofield

--------------------------------------------------------

Associate Professor

Department of Mathematics and Statistics

Calvin College

--------------------------------------------------------