bugGNU Octave - Bugs: bug #50708, [octave forge] (image) add...

 
 

bug #50708: [octave forge] (image) add demosaic.m

Submitter:  Timothy Parys <tparys>
Submitted:  Sun 02 Apr 2017 10:46:52 PM UTC
   
 
Category:  Octave Package Severity:  1 - Wish
Priority:  3 - Low Item Group:  Feature Request
Status:  Patch Submitted Assigned to:  None
Originator Name:  Timothy Parys Open/Closed:  * Open
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 14 Aug 2020 06:23:01 AM UTC, comment #15: 

@Timothy Parys: I can understand that you are disappointed. It was not my intention to insult you in any way, and I am sorry that you feel that way.

Please keep this bug report open as it contains a lot of work of other people trying to find a feasible way to get a demosaic function for Octave.

Hartmut <hardy>
Wed 12 Aug 2020 12:42:56 AM UTC, comment #14: 

Not so much awkward or demotivating as outright insulting, and proof positive that beggars can be choosers.

It may be that Pei-Tam algorithm may be only slightly more computationally expensive, but the fact remains that nobody has lifted a finger to do this in the 3 years this bug has been open.

But it may work out better this way. I hereby revoke all license to the Octave project to use this code.

So, enjoy writing it yourself. I'm sure it will meet your coding standards much more than my attempt.

Feel free to close this bug, as it's no longer relevant.

Thanks.

Timothy Parys <tparys>
Mon 10 Aug 2020 07:14:49 PM UTC, comment #13: 

I have reviewed the patch mentioned in comment #8. And I have started reading some scientific literature about different demosaicing algorithms.

As a result, I would tend to NOT include this patch into the image package, yet. I would very much prefer to use a better algorithm instead (but one that is not patent encumbered), e.g. the Pei-Tam algorithm, as mentioned in commment #11. For completeness I will also add my comments on the current patch file near the bottom of this post.

1. Why not the bilinear algorithm?

The current patch (from comment #8) uses the bilinear interpolation method to do the deomosaicing. When reading the literature, it seems to me that this algorithm is the first and easiest thing one can do to achieve some kind of demosaicing. But is seems to also have major drawbacks, since it generates comparably strong artefacts. When we include this algorithm into the Octave image package, and users use it to run Matlab code they found somewhere, then they will get visibly worse results and probably complain about this in a bug report sooner or later. Yes, having the current demosaic.m included into the image package is a bit better than having no function for this at all. But it is also a bit worse than having no function for this job (and honestly saying this) because people excpect the function to work at least in a similar quality than in Matlab. So in the long run we would just exchange one bug report with another. And people using the (bilinear) demosaic function in the meantime are also dissapointed because we need to break the compatibility of our Octave code at some future time to introduce a better algorithm (as Avinoam also seems to forsee in comment #9).

2. Why not the Malvar-He-Cutler algorithm?

Because it is somehow patent encoumbered. And the Octave team as not at all the means to effectivly deal with this law topic.

3. Why the Pei-Tam algorithm (for example)?

This algorithm was suggested (by "anonymous") in comment #11. And after reading the literature, it seems to me that it is

  • not much more computational difficult than the simple bilinear method
  • produces much better results that the simple bilinear method
  • produces almost as good results as the Malvar-He-Cutler method that Matlab claims to use. In the original Halvar-He-Cutler paper those authors also say "Compared to Pei-Tam linear filters, our method produces slightly less visible color fringing distortion, as well as a lower PSNR, at a slightly reduced computational complexity." So it seems to do 90% of the job.
  • I have (so far) not found any hint that it would also be patent encoumbered.


The original paper of Pei-Tam can for example be found here: https://pdfs.semanticscholar.org/8d68/4a67ae27a177a592baf96008d919b16d867b.pdf

4. Sorry

It must feel akward to only get this feedback more than 3 years after the original submission of the patch. I fear this is also demotivating for Timothy as original author. It is due to chronic shortage of manpower in our voluntary project.

5. My comments on the current patch (from comment #8)

Here is nevertheless my detailed feedback on the current patch file:

  • The copyright text should probably be adapted. The first line normally mentions the author and his email address. The other three lines (after the GPL text) with the author information could then be deleted.
  • line 34: the use of size(size(I)) could be substituted by ndims(I) for better readibility.
  • There are a couple of places, where the (coding style) space sign after a comma is missing. E.g. line 38.
  • line 41: Couldn't we use isstring() instead of class=='str' ?
  • All strings should be in double quotes (") and not single quotes ('), if I remember the coding style right.
  • The help string should also tell which algorithm is used (it currently does this), that it is different from Matlab (it currently does this) but comparably good in outcome (if it really is). And it should probably tell the minimum size of the input image (for Pei-Tam this would also be 5x5, as in Matlab, then).


6. How do we proceed?

Can anyone follow my argumentation? It feels sad to me to not use a decent piece of code (as given in the current patch file), but I think it wouldn't be good in the long run.

Is there a chance that someone (Avinoam? Timothy?) adapts the current patch file, to properly use the Pei-Tam algorithm (or some other algorithm with comparably performance)? I gave a link to the paper above. Literature says its computational burden is similar to the bilinear algorithm. But it would still need a major re-write of the algorithm part of this patch.

If anyone takes up this task, I would then be willing to again review the code, and hopefully then include a demosaic function (with a decent algorithm) into the image package.

Hartmut <hardy>
Mon 27 Jul 2020 06:40:29 PM UTC, comment #12: 

I recommend to insert this function to image package.
Any objection?

Avinoam Kalma <avinoam>
Group Member
Tue 26 Sep 2017 01:21:27 PM UTC, comment #11: 

An alternative for the patent encumbered approach might be the one from which it is derived from, should still be better than bilinear:

S.-C. Pei and I.-K. Tam, “Effective color interpolation in CCD color filter arrays using signal correlation,” IEEE Trans. Circuits Systems Video Technol., vol. 13, no. 6, pp. 503-513, Jun. 2003.

Anonymous
Sat 24 Jun 2017 09:23:42 PM UTC, comment #10: 

Just curious, did this ever get added?
 I don't see it listed at https://sourceforge.net/p/octave/image/ci/default/tree/inst/ ?

Am I expected to do it? Not sure if I have commit access.

Anonymous
Wed 24 May 2017 06:29:54 PM UTC, comment #9: 

I recommend to insert this function to image package,
maybe with a "fixme" line about implementing another method(s).

Avinoam Kalma <avinoam>
Group Member
Tue 11 Apr 2017 01:58:56 AM UTC, comment #8: 

For the "if 1", that's something I saw on this page: https://www.gnu.org/software/octave/doc/interpreter/Demonstration-Functions.html

Re-reading that, it looks like that was only necessary if I were to define any functions in the demo. Since I'm not doing that, it probably doesn't belong. Also looks like I had some commented out references to subplots as well. Cleaned that out as well.

Also, just noticed the Octave convention of endif instead of end. That's an interesting difference I didn't notice until now. Thanks for the fix.


(file #40349)

Timothy Parys <tparys>
Mon 10 Apr 2017 06:18:06 AM UTC, comment #7: 

@Timothy:

Thanks for the new submission, with the tests, and the nice demo.
I am uploading a new file, based on yours with

  • typos fixes
  • add sensorAlignment = tolower(sensorAlignment); so that "BGGR" can abe used.


Why did you use "if 1" in the demo?

If there no other comments, I recommend to add this function to image package. In the future we should implement another (un-patented) method, and let the user choose, like in
https://www.mathworks.com/help/vision/ref/demosaic.html in which the user can choose the Interpolation algorithm from "Bilinear" and "Gradient-corrected linear".

(file #40346)

Avinoam Kalma <avinoam>
Group Member
Sat 08 Apr 2017 12:15:17 AM UTC, comment #6: 

So, based on comments so far, I've left the interpolation algorithm as bilinear for now, and noted in the help text that it differs from the algorithm in use by Matlab.

I've drastically redone the code to remove the multiple of 2 restriction, and have done my best to adhere to the Octave coding style guide I found online.

There should be 11 tests and a demo implemented, if anyone's curious how their digital camera sees the world. The moire patterns in the second figure are actually pretty cool looking, though seems to  stress this related bug? http://savannah.gnu.org/bugs/?45372

Further feedback welcome.

(file #40327)

Timothy Parys <tparys>
Tue 04 Apr 2017 06:56:37 PM UTC, comment #5: 

I am no patent lawyer (but do have some experience in using patents). And to me this seems to be a valid US patent of the company Microsoft.  (A company that has money to pay lawyers...)

I will attach the (open source) original patend document as PDF. The second last page (with "claim 1") is the most important.

The patent was filed in March 2004, and is valid since 2009. The filing date is less than 20 years ago, so I would assume that the patent is still valid today. The quoted patend is valid in the US, but there are also other members of this patent family, for other countries today (namely China, Korea, Japan, Europe and World. The last two ones aren't exactly patents).

Matlab cites the following paper (from May 2004) in its help text of the demosaic command:


References
[1] Malvar, H.S., L. He, and R. Cutler, High quality linear interpolation for demosaicing of Bayer-patterned color images. ICASPP, Volume 34, Issue 11, pp. 2274-2282, May 2004.


So the patent submission was slightly earlier than this publication. This timing looks reasonable to me if someone really wants to hold a patent on this.

The fact that this is not patented in individual countries in Europe leads me to the sucpicion that this might not exactly be "worth" a patent. But currently some counties do have this method patented. Formally there are three ways of dealing with this:

  • pay money to Microsoft to purchase the right to use this patent
  • take Microsoft a US (and some asian) courts to fight this patent. And pay lawyers to do this fighting for you.
  • wait some (probably just 7) years, until the patent is not valid any more, at least the US patent.


The first two ways don't seem feasable to me for Octave code.

So what do we do about this? Should be just use the proposed code of Timothy, that might produce lesser quality results then the Matlab function? My vote would be: yes.



(file #40266)

Hartmut <hardy>
Mon 03 Apr 2017 10:36:30 PM UTC, comment #4: 

Malvar-He-Cutler appears patent encumbered?

https://www.google.com/patents/US7502505

Timothy Parys <tparys>
Mon 03 Apr 2017 08:12:57 PM UTC, comment #3: 

Thanks for contributing this function.

However, there are few problems:

This method uses a 5x5 filter (that's the reason for the minimal size)

  • There are no builtin self tests, as Carnë noted in comment #1.


Attached a new version which (try to) fix the first 6 items.


(file #40250)

Avinoam Kalma <avinoam>
Group Member
Mon 03 Apr 2017 05:46:35 PM UTC, comment #2: 

Shouldn't the function name not read "demosaic"? (letter a and i swapped)

Hartmut <hardy>
Mon 03 Apr 2017 12:45:07 PM UTC, comment #1: 

Hi! Thank you for the submission. Could you please provide some tests?

Carnë Draug <carandraug>
Group Member
Sun 02 Apr 2017 10:46:52 PM UTC, original submission:  

Found that octave did not have an implementation of demosiac. Wrote one while toying with Bayer images. Tested in Ubuntu 16.04.

Might also affect image-acquisition, per the comments in https://sourceforge.net/p/octave/image-acquisition/ci/ee385c9a7e1b7cf1897ca9ad9a53640de5f7ab70/tree/inst/SGRBGtoRGB.m

Timothy Parys <tparys>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #40349:  demosaic.m added by tparys (6KiB - text/x-objcsrc)
file #40346:  demosaic.m added by avinoam (6KiB - application/octet-stream)
file #40327:  demosaic.m added by tparys (6KiB - text/x-objcsrc)
file #40266:  US7502505B2.pdf added by hardy (2MiB - application/pdf)
file #40250:  demosaic.m added by avinoam (3KiB - application/octet-stream)
file #40238:  demosiac.m added by tparys (2KiB - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by avinoam (Updated the item)
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by tparys (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 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-26 mtmiller Severity3 - Normal 1 - Wish
        Priority5 - Normal 3 - Low
        Release4.0.0 dev
    2018-07-02 jwe Summary[octave-forge] (image) add demosaic.m [octave forge] (image) add demosaic.m
    2017-08-11 jwe Summaryimage package: add demosaic.m [octave-forge] (image) add demosaic.m
    2017-04-11 tparys Attached File- Added demosaic.m, #40349
    2017-04-10 avinoam Attached File- Added demosaic.m, #40346
    2017-04-08 tparys Attached File- Added demosaic.m, #40327
    2017-04-04 hardy Attached File- Added US7502505B2.pdf, #40266
    2017-04-03 avinoam Attached File- Added demosaic.m, #40250
        Summaryimage package: add demosiac.m image package: add demosaic.m
    2017-04-03 carandraug StatusNone Patch Submitted
    2017-04-02 tparys Attached File- Added demosiac.m, #40238

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code