bugGNU Octave - Bugs: bug #40669, rgb2ind: uses system ("gm...

 
 

bug #40669: rgb2ind: uses system ("gm convert....") for dither option

Submitter:  Carnë Draug <carandraug>
Submitted:  Fri 22 Nov 2013 01:30:05 PM UTC
 
Category:  Octave Function Severity:  4 - Important
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Sun 24 Nov 2013 02:27:09 AM UTC, comment #7: 
Carnë Draug <carandraug>
Project Member
Fri 22 Nov 2013 06:37:43 PM UTC, comment #6: 

The original patch that made this change is here, if it helps.

https://savannah.gnu.org/patch/?func=detailitem&item_id=8001#options

Adam H Aitkenhead <adama>
Fri 22 Nov 2013 05:16:51 PM UTC, comment #5: 

We're out of time for the 3.8 release so I would revert the changesets as the fastest possible solution.

Rik <rik5>
Project Administrator
Fri 22 Nov 2013 03:24:58 PM UTC, comment #4: 

As I mentioned, the missing options should not be very difficult to implement but I won't have the time to do it. If anyone wants to do this, it's basically finding the closest value in the colormap, and then distribute the difference (error) to the pixels ahead. To note that different algorithms exist, the only difference being how the error is distributed so a very interesting thing would be to have the internal function accept some sort of mask specifying how to perform the distribution.

Anyway, for the upcoming release, my opinion would be to revert those changes and not implement this options (unless someone picks up this and implements it on time).

Note that if we keep the current implementation, when we implement this again properly with Matlab compatibility, the results will be different.

I'm adding Adam H Aitkenhead to the mailing list of this bug since he was the author of the change and of bug #40664.

Carnë Draug <carandraug>
Project Member
Fri 22 Nov 2013 03:09:21 PM UTC, comment #3: 

What do you think we can do about this for the imminent 3.8 release? I don't think the current situation is ideal to release with, right?

Mike Miller <mtmiller>
Project Administrator
Fri 22 Nov 2013 02:23:05 PM UTC, comment #2: 

I would not recommend to make use of GM for the following reasons:

  • because of other issues when reading and writing images, I'm guessing we'll eventually have to replace GM with something else;
  • the Floyd and Steinberg algorithm is quite easy to implement, no need to add a dependency because of that;
  • the algorithm can be done in different ways. The diffusion of the error is usually done from left to right, and top to bottom but other variants do it from bottom to top. My previous attempt at it found that Matlab does not implement the standard one so depending on GM for this will not allow us to be Matlab compatible;
  • GM only does this for 8bit images;

Finally, dithering should not be implemented in rgb2ind, there's a missing function "dither" that rgb2ind should be calling. My guess (from trying to do this last year) is that for the complete rgb2ind to be implemented, the functions imapprox and dither need to be implemented first (each of the functions implementing different methods to perform the conversion).

Carnë Draug <carandraug>
Project Member
Fri 22 Nov 2013 02:02:35 PM UTC, comment #1: 

Confirmed. I also don't have the gm executable on my system. But apparently "make check" doesn't exercise this code path because I haven't seen an error yet.

Yes it would be much preferable to reimplement with magick library calls, could be added to _magick_read_.cc. Otherwise this adds an extra runtime dependency that our distributors need to know about, and this is fairly hidden.

If we have to keep the current approach, it at least needs a runtime check for the executable and an appropriate error message if "gm" is not found in PATH. It also doesn't look to me like the temporary files being created are deleted at all.

Mike Miller <mtmiller>
Project Administrator
Fri 22 Nov 2013 01:30:05 PM UTC, original submission:  

With cset 5d6243c2acbf, the option dither and others related to it were implemented. However, this is done by making a system call to "gm convert", one of the applications that may get installed with GraphicsMagick. But Octave is not dependent on this application, it's dependent on the Magick++ library and some users use ImageMagick instead.

The current implementation performs the following:
- saving image as a tiff file
- making a system call to "gm convert"
- read back the converted file

If a dependency on GraphicsMagick for this is desirable, then there's a map method http://www.graphicsmagick.org/Magick++/Image.html#map though if I remember correctly it was limited to 8bit images.

But the algorithm is very simple so that should not be necessary (although it will most likely be needed to be implemented as oct file as I don't see a way to vectorize it).

This is slightly related to bug #40664.

Carnë Draug <carandraug>
Project Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by adama (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by carandraug (author of the cset in question)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by carandraug (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 project members can vote.

     

     

     

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-11-24 carandraug Open/ClosedOpen Closed
    2013-11-24 carandraug StatusConfirmed Fixed
    2013-11-22 carandraug Carbon-Copy- Added adama
    2013-11-22 mtmiller Severity3 - Normal 4 - Important
        Item GroupNone Incorrect Result
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.10