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.
|
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)
|
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.
|
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...
|
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)
|
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.
|
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.
|
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.
|
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.
This is with a recent tip (10892:20ce631f0f12) and GraphicsMagick-1.3.5
|