bugGNU Octave - Bugs: bug #30784, imwrite fails when FMT is set...

 
 

bug #30784: imwrite fails when FMT is set explicitly

Submitter:  Rik <rik5>
Submitted:  Mon 16 Aug 2010 03:36:55 AM UTC
   
 
Category:  Interpreter Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 16 Sep 2010 06:07:53 PM UTC, comment #8: 

"Actually, because we now prepend the filename for every call to GraphicMagick's WriteImages function, and because WriteImages only looks at the start of the filename up to the first colon, everything works as expected."

D'oh, good point.

I applied your latest change.

Thanks.

John W. Eaton <jwe>
Group administrator
Thu 16 Sep 2010 02:25:01 PM UTC, comment #7: 

I see that you didn't remove the for loop.  Here is a patch to do just that.  Sorry this took so many iterations to solve. I hate cluttering up the repository with so many patchsets.



(file #21473)

John Swensen <jpswensen>
Thu 16 Sep 2010 02:06:15 PM UTC, comment #6: 

Actually, because we now prepend the filename for every call to GraphicMagick's WriteImages function, and because WriteImages only looks at the start of the filename up to the first colon, everything works as expected.

octave:9> imwrite(img,'tiff:test_img.png','bmp');
octave:10> system('file tiff:test_img.png')
tiff:test_img.png: PC bitmap, Windows 3.x format, 640 x 480 x 24
ans = 0
octave:11> imwrite(img,'tiff:test_img.png');
octave:12> system('file tiff:test_img.png')
tiff:test_img.png: PNG image, 640 x 480, 8-bit grayscale, non-interlaced
ans = 0

Did you actually remove the for loop, or do I need to whip up another changeset?  I agree that it isn't necessary.

John Swensen <jpswensen>
Thu 16 Sep 2010 06:57:14 AM UTC, comment #5: 

OK, I checked in the change.

It looks to me like the loop

      for (size_t i = 0; i < imvec.size (); i++)
        imvec[i].magick (fmt);

is not needed if you use the filename encoding trick.

But, ugh.  This means that you can't write a file called

  jpg:foo.tif

right?  I mean, again, not that I would really want to do this, but these kind of design decisions have some unintended consequences that would have been really easy to avoid...

John W. Eaton <jwe>
Group administrator
Thu 16 Sep 2010 04:28:41 AM UTC, comment #4: 

Despite you having closed this bug, I have one more changeset associated with this patch. After looking at the GraphicsMagick source, I confirmed that the library (and not just the gm utility) supports the ability to write the filename as 'format:filename' and it will parse the format out and use this for the format.  The function was written strangely such that setting the 'magick' struct member directly does not work when a file extension is given (the file extension always overwrites 'magick' at the start of WriteImage), but using the colon syntax causes the output type to be set after the point where the output filename extension overwrites 'magick'.

I also looked at the ML documentation to see what file types they support and added all those that are supported by GraphicsMagick to the list of allowable file types.  I tested each of them to make sure they wrote properly.

I did notice, however, that imwrite could use a lot of work to allow users to set parameters for the plethora of different allowable formats. I think this is an easy enough task for one of the people who sometimes comes to the list looking for something to do. It would be a matter of modifying just imwrite.m and _image_read_.cc (which needs a name change for clarity) and would be a great "get your feet wet" project.

(file #21470)

John Swensen <jpswensen>
Wed 15 Sep 2010 06:23:25 PM UTC, comment #3: 

I think we need to use a vector because an Octave image can contain multiple frames.  As I understand it each frame is considered a separate image in the vector that is passed to the writeImages function.

Your change seems to solve the reported problem so I checked it in.

Unfortunately, if you try

  imwrite (img, "foo.jpg", "tif")

the file is written in JPEG format, not TIFF even though we are setting the type to "tif" before calling writeImages.  Ugh.  This "feature" is a bit annoying, as I think the contents of a file should determine what is in it, not the filename extension. I don't suppose this is a big problem because not many people are likely to want to mix things up this way.  But what if I don't know about all the naming conventions and I have some other reason to use .jpg, like maybe it stands for "John's Perfect Graph".  Who cares if I want to write TIFF data to a file with a .jpg extension?

Also, in the future, it would be helpful if you sent Mercurial changesets instead of plain diffs.

Thanks.

John W. Eaton <jwe>
Group administrator
Tue 14 Sep 2010 08:36:57 PM UTC, comment #2: 

I think something like the following should solve your problem (BTW, I am not an ImageMagick expert, but did a simple test using one of their examples).

In the write_image function of _magick_read_.cc, we use the STL compatible Magick::WriteImages () function.  I don't think this is necessary.  Correct me if I am wrong, but doesn't the variable 'img' always get passed in a single image?  If this is the case, then I think we can do the following:

1) Fix the start of the function so that the octave_value 'img' is encoded properly and placed in an instance of Magick::Image called 'image' (i.e. get rid of the STL::vector).
2) Execute the following commands:
         image.magick(fmt);    // fmt as passed in as string to the write_image function
         image.fileName(filename);   // filename as passed in as string to the write_image function
         WriteImage( image.imageInfo(), image.image() );  // This is a call to the C ImageMagick library with calls to the proper ImageMagick++ member functions of ImageMagick::Image to retrieve the C structures necessary to satisfy the parameter list

Hope this helps.  I don't currently have a working build on my OSX system (still fighting with gfortran issues) or I would make and test a patch myself.

John Swensen <jpswensen>
Tue 14 Sep 2010 05:05:09 PM UTC, comment #1: 

The problem appears to be that Octave is calling

  Magick::writeImages (imvec.begin (), imvec.end (), filename);

to write the image, and this function does not include an explicit image storage type so it depends on the filename to determine the type.  If I knew which Magick++ function(s) to call to write an image to an arbitrary filename with a specified imagee type, I could probably fix this.  But I have no clue.  Are there any GraphicsMagick++ experts out there?

I'm also asking for help on the maintainers list.

John W. Eaton <jwe>
Group administrator
Mon 16 Aug 2010 03:36:55 AM UTC, original submission:  

imwrite can be called with a format, FMT, defined implicitly by the filename extension or explicitly by the argument FMT.


imwrite (img,"test_img.tif")      # works
imwrite (img,"test_img","tif")    # fails
error: Magick++ exception: Magick: No encode delegate for this image format (test_img) reported by magick/constitute.c:9099 (WriteImage)


This is with a recent tip (10892:20ce631f0f12) and GraphicsMagick-1.3.5

Rik <rik5>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #21473:  imwrite_rmvfor.diff added by jpswensen (1KiB - text/x-patch)
file #21470:  imwrite_morefmt.diff added by jpswensen (3KiB - text/x-patch)
file #21466:  magick_fmt.diff added by jpswensen (855B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jpswensen (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (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 group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-09-16 jpswensen Attached File- Added imwrite_rmvfor.diff, #21473
    2010-09-16 jpswensen Attached File- Added imwrite_morefmt.diff, #21470
    2010-09-15 jwe StatusNone Fixed
        Open/ClosedOpen Closed
    2010-09-15 jpswensen Attached File- Added magick_fmt.diff, #21466
    2010-08-16 rik5 Summaryimwrite fails when FMT is set imwrite fails when FMT is set explicitly

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code