Bug 148540

Summary: Improvements of adding noise tool
Product: [Applications] digikam Reporter: Dik Takken <kde>
Component: Plugin-Editor-FilmGrainAssignee: Digikam Developers <digikam-bugs-null>
Status: CLOSED FIXED    
Severity: wishlist CC: caulier.gilles, friiduh, Julien
Priority: NOR    
Version: 1.2.0   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 1.2.0
Sentry Crash Report:
Attachments: HSV noise generator
HSV noise generator V2
Gimp 2.6.8 noise HSV generator source code.
HSV noise generator V3
First approach of Chrominance noise implementation
Chrominance noise patch version 2
Chrominance noise patch version 3
First try to implement Graininess size support
New version of patch for grain size support

Description Dik Takken 2007-08-05 20:40:21 UTC
Version:           0.9.2 (using KDE KDE 3.5.5)
OS:                Linux

Adding a very, very subtle layer of noise to an image often works well for improving the apparent quality of the image. Noise can add pseudo-detail, which make the image appear more detailed than it really is. Noise can also hide imperfections, like filter artifacts. 

I often use GIMP to add 'HSV Scatter' noise after noise reduction, after applying the 'image restoration' plugin, and after enlargement (scaling). The 'Hue' and 'Saturation' settings are always reduced to zero in order to produce a type of noise that is barely visible.
Comment 1 caulier.gilles 2007-08-14 09:19:49 UTC
plugin lready exist : try FilmGrain tool

Gilles Caulier
Comment 2 Dik Takken 2007-08-14 22:05:59 UTC
I tried that, but the film grain plugin does something completely different than the HSV scatter tool found in GIMP. The HSV scatter tool does something like this, for every pixel:

Pixel.Lightness += LightnessAmplitude * Rand()
Pixel.Hue += HueAmplitude * Rand()
Pixel.Saturation += SaturationAmplitude * Rand()

And this is exactly what I need. I tried adding pseudo detail using the film grain tool, but I bumped into two problems:

* The 'grains' are too large. As a result, the image looks 'dirty' rather than detailed.
* It does not only add texture, but it also changes brightness/contrast.
Comment 3 Dik Takken 2008-06-03 22:09:14 UTC
Can this bug be re-opened please? The requested feature is clearly different from the film grain tool.
Comment 4 Arnd Baecker 2008-06-04 05:00:28 UTC
Re-opening on Dik's request.
Comment 5 caulier.gilles 2010-01-26 08:06:50 UTC
*** Bug 224241 has been marked as a duplicate of this bug. ***
Comment 6 caulier.gilles 2010-01-26 08:11:56 UTC
Created attachment 40242 [details]
HSV noise generator

Julien,

This patch is a first try, not optimized, not finalized, but it run. 

Original image is parsed pixel by pixel. Each color components are converted from RGB to HSL and hue saturation and lightness are adjusted with a random value receptively. Fianly, HSL color is reconverted to RGB to create target image.

I can see some side effect if sensitivity is very high, like color cast. This can be fixed to clamp adjusted value.

What do you think about ?

Gilles Caulier
Comment 7 Julien Narboux 2010-01-26 08:29:24 UTC
Great Gilles,

I think it is a good start. I will try to hack to make the amount of the random change depend on the lightness.

For clamping, I had the same problem in the vignetting plugin. Maybe we should have these methods in Dimg, or is there efficiency issues ?

Julien
Comment 8 caulier.gilles 2010-01-26 08:37:00 UTC
>I think it is a good start. I will try to hack to make the amount of the random
>change depend on the lightness.

yes, but my plan for the future is to have a photo world approach there. the idea is to reproduce the real analog film grainess as with this tool :

http://powerretouche.com/Film-Grain_plugin_tutorial.htm

Read in deep this paper, and if cou can try this photoshop plugin to compare result with digiKam tool.

>For clamping, I had the same problem in the vignetting plugin. Maybe we should
>have these methods in Dimg, or is there efficiency issues ?

I need to investigate where is the problem exactly.

Gilles Caulier
Comment 9 caulier.gilles 2010-01-26 08:44:10 UTC
Another stage for the future is to make noise generator code common in DImg filters and share it with Infrared tool which have also this option.

After that, Add Noise generator to BQM is the goal...

Perhaps also we can factorize infrared and filmgrain tools ? The advantage is to reduce tools to load at startup. What do you think about ?

Gilles Caulier
Comment 10 Julien Narboux 2010-01-26 10:19:39 UTC
I agree that noise generator should be moved to DImg filters. Maybe all filters should be moved there because it would simplify reusing filters.
I do not use the infrared tool... and I think it would be counter intuitive to mix the two tools. 

Maybe to simplify the gui we could move the infrared option to the b/w tool as a new lens filter and noise generator to the b/w tool as a new tab ?

Julien
Comment 11 Johannes Wienke 2010-01-26 10:24:29 UTC
I wouldn't expect a tool to add noise at the b/w tool.
Comment 12 caulier.gilles 2010-01-26 10:28:51 UTC
>I agree that noise generator should be moved to DImg filters. Maybe all filters
>should be moved there because it would simplify reusing filters.
>I do not use the infrared tool... and I think it would be counter intuitive to
>mix the two tools. 

Progressively yes, step by step.

>Maybe to simplify the gui we could move the infrared option to the b/w tool as
>a new lens filter and noise generator to the b/w tool as a new tab ?

Definitively yes, but in a second stage, because B&W preview still imageguidewidget, and IR / NG use imageregionwidget.

I will port B&W preview to imageregionwidget after 1.1.0, when previewwidget will be ported to pure Qt4. This can take a while due to huge regression test to do.

Gilles
Comment 13 caulier.gilles 2010-01-26 10:29:42 UTC
Another point to take a care is about B&W tool : it's not yet multithreaded, where NG and IR are already.

Gilles Caulier
Comment 14 caulier.gilles 2010-01-26 10:31:38 UTC
To Johannes, #11:

>I wouldn't expect a tool to add noise at the b/w tool.

You have right... What happen if user want to add noise in color image, not B&W...

Gilles Caulier
Comment 15 Julien Narboux 2010-01-26 10:33:02 UTC
Yes, Johannes you are right, it needs to be kept (also ?) as a separate tool.

Julien
Comment 16 caulier.gilles 2010-01-26 10:37:44 UTC
Johannes, 

i'm agree with Julien about IR tool : it must be merged later in B&W tool

Gilles Caulier
Comment 17 caulier.gilles 2010-01-26 15:26:36 UTC
Created attachment 40257 [details]
HSV noise generator V2

new version fixing crash if sensitivity < 1000
Comment 18 caulier.gilles 2010-01-26 15:58:46 UTC
Created attachment 40260 [details]
Gimp 2.6.8 noise HSV generator source code.

For information, the gimp plugin to take a look how to adjust HSV values randomly.

This plugin is too technical and not photo oriented. We need to make an abstraction for end user as PowerRetouche tool. Typically, we need to adjust the right H,S,V values to increase/decrease noise in image. User don't care about HSV adjustments. He want to see graininess depending to sensitivity in ISO. that all...

Gilles Caulier
Comment 19 caulier.gilles 2010-03-05 13:57:41 UTC
Created attachment 41352 [details]
HSV noise generator V3

updated patch to run with current code from svn...

Julien, Do you will manage this entry ?

Gilles Caulier
Comment 20 caulier.gilles 2010-03-05 14:39:25 UTC
SVN commit 1099317 by cgilles:

Film Grain Image Editor tool : use HSL color space to adjust luminosity (and only this component) to add graininess to image.
Code is now more simple and faster. There is not alteration of luminosity from original image.
BUGS: 148540


 M  +1 -1      CMakeLists.txt  
 D             filmgrain.cpp  
 D             filmgrain.h  
 A             filmgrainfilter.cpp   filmgrain.cpp#1099200 [License: GPL (v2+)]
 A             filmgrainfilter.h   filmgrain.h#1099200 [License: GPL (v2+)]
 M  +13 -15    filmgraintool.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1099317
Comment 21 Julien Narboux 2010-03-05 16:39:14 UTC
SVN commit 1099445 by jnarboux:

Do not increase lightness on everage.
Clamp values to prevent artefacts.
Increase range of the filter.

BUGS: 148540



 M  +1 -1      imageplugins/coreplugin/filmgraintool.cpp  
 M  +9 -3      libs/dimg/filters/fx/filmgrainfilter.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1099445
Comment 22 caulier.gilles 2010-03-05 19:04:48 UTC
Julien,

You little fix in filmgraintool.cpp must be backported to film gram BQM code, to be homogenous. Just few lines to patch...

Gilles Caulier
Comment 23 Julien Narboux 2010-03-05 19:23:38 UTC
Gilles,

Yes I know, but I plan to add three sliders in the film grain tool to change noise level depending on lightness. The user should have the possibility to add noise in midtones while preserving white/blacks. Then I will try to factorize the setting widgets betweeen bqm and other tool as you did with other tools.

What about adding the possibility to add chroma noise as well ? it is not usefull for me but maybe other people need chroma noise ? 

Julien
Comment 24 caulier.gilles 2010-03-05 20:17:57 UTC
>Yes I know, but I plan to add three sliders in the film grain tool to change
>noise level depending on lightness. The user should have the possibility to add
>noise in midtones while preserving white/blacks. Then I will try to factorize
>the setting widgets betweeen bqm and other tool as you did with other tools.

OK. grain settings widget must go to /libs/dimg/filters/fx. Look how i do with others filter, as bcgsettings.cpp for ex.

>What about adding the possibility to add chroma noise as well ? it is not
>usefull for me but maybe other people need chroma noise ?

yes. chroma noise will simulate CCD camera. I supose that you need to convert RG to YCbCr color space and vice versa, to add noise on right channel. Look in nrfilter.cpp code. 

Gilles
Comment 25 Dik Takken 2010-03-05 21:53:20 UTC
Very subtle chroma noise can hide imperfections in the chroma channels, especially after enlargement. For instance, enlarged CCD noise can look bad. Adding some high resolution chroma noise might make it look nicer.
Comment 26 Julien Narboux 2010-03-06 14:14:42 UTC
SVN commit 1099980 by jnarboux:

Add sliders to add different amounts of noise to highlights, midtones and shadows.

BUGS: 148540



 M  +46 -5     imageplugins/coreplugin/filmgraintool.cpp  
 M  +37 -6     libs/dimg/filters/fx/filmgrainfilter.cpp  
 M  +8 -3      libs/dimg/filters/fx/filmgrainfilter.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1099980
Comment 27 Fri13 2010-03-07 01:19:47 UTC
I think the chroma noise is needed. We already have two different terms what to use. Film grain and Noise. Grain is for films where it is not causing color changes (digital term would be Luminance noise) but noise is for digital cameras where we get color changes as well (chroma noise).

So if we can simulate grain and noise, it is better. Even that noisy photo is not usually so pleasing as grainy photo.
Comment 28 Julien Narboux 2010-03-07 11:26:09 UTC
Ok I will add chroma noise as well.
Gilles, I don't like the fact that the amount of noise is expressed as "ISO" it does not mean much because noise depend on the film/sensor.

What do you think ?

The gui could look like this:

Luminance noise:
- Amount
- Shadows
- Midtones
- Highlights

Chroma noise:
- Amount
- Shadows
- Midtones
- Highlights

What about adding a tab in bw tool with only the luminance noise settings ?

In the future, we should add a parameter for the size of the noise and a preview of the noise. But this would require deeper changes in the code.

Julien
Comment 29 Julien Narboux 2010-03-07 11:34:13 UTC
SVN commit 1100341 by jnarboux:

Use default values which simulates real film grain. More noise is applied to midtones.

BUGS: 148540



 M  +2 -2      filmgraintool.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1100341
Comment 30 caulier.gilles 2010-03-07 17:39:38 UTC
Julien,

The GUI layout proposal is fine for me.

About new B&W tab, what do you mean by "luminance noise settings". To add noise with this tool ? Why only luminance noise, why not all ? If you design a Noise Settings widget, with a Noise settings container as i already done with other editor tools, it will be easy to setup Noise settings in BW tool with only few code. 

Gilles
Comment 31 Julien Narboux 2010-03-07 18:22:24 UTC
SVN commit 1100503 by jnarboux:

First try at chrominance noise.

BUGS: 148540




 M  +159 -62   imageplugins/coreplugin/filmgraintool.cpp  
 M  +39 -20    libs/dimg/filters/fx/filmgrainfilter.cpp  
 M  +14 -6     libs/dimg/filters/fx/filmgrainfilter.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1100503
Comment 32 Julien Narboux 2010-03-07 18:24:08 UTC
Gilles,

Because in a black and white tool, changing the color  would look bad in my opinion. That is why we could only put only luminance noise settings.

Julien
Comment 33 caulier.gilles 2010-03-07 18:29:59 UTC
Ah, yes, right Julien...

Gilles
Comment 34 Julien Narboux 2010-03-07 18:36:11 UTC
It would be nice if someone can check it works under windows.

Julien
Comment 35 caulier.gilles 2010-03-07 18:41:30 UTC
I will check windows stuff. I currently Seven entreprise in a virtualbox...

Gilles
Comment 36 Julien Narboux 2010-03-17 08:22:50 UTC
I am not sure we can be satisfied with the chrominance noise, it looks not very different from the luminance noise. I don't know if it is what people expect, I just randomly changed the hue channel.

Julien
Comment 37 caulier.gilles 2010-03-17 08:40:31 UTC
Julien,

This is normal. You try to add Chrominance noise (from YCbCr color space) to use Hue channel from HSL color space. Both color space are not the same.

To fix it, you must convert RGB to YCbCr and randomize Cb or Cr channel ! after that, convert back to RGB.

To convert RGB<=>YCrCb, look into Noise Reduction filter, methods are there :

http://lxr.kde.org/source/extragear/graphics/digikam/libs/dimg/filters/nr/nrfilter.h#94

To re-use these methods, you must foctoring somewhere the code. DImg APi sound like the best place.

About luminance noise, all is fine.

Gilles Caulier
Comment 38 caulier.gilles 2010-03-18 13:47:51 UTC
SVN commit 1104744 by cgilles:

DColor : added 2 methods to convert from RGB to YCrCb color space and vice versa.
CCBUGS: 148540


 M  +26 -1     dcolor.cpp  
 M  +15 -1     dcolor.h  
 M  +2 -2      filters/bw/tonalityfilter.cpp  
 M  +1 -1      filters/fx/filmgrainfilter.cpp  
 M  +2 -2      filters/hsl/hslfilter.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1104744
Comment 39 caulier.gilles 2010-03-18 14:18:51 UTC
Created attachment 41733 [details]
First approach of Chrominance noise implementation

Julien,

Please, take a look in my patch. It use 2 new methods from DColor class to convert RGB <-> YCbCr color space.

Currently chrominance blue and red are adjusted at the same time. I think it will be better to add Blue and Red noise independently.

Also, in GUI, the user must be able to enable/disable noise type adjustement. Typically, user must be able to apply only luminance noise, or/and Chrominance Blue noise, or/and Chrominance Red noise. Using Checkbox there is the solution. Of course filter need new settings for that.

The result are not very nice about Chrominance (over-saturated). Something is wrong somewhere, perhaps in DColor::getYCbCr(), DColor::setYCbCr(), or in filter as well. I don't know yet. Investigations are require.

Anyway, this is the right way to go about Chrominance noise.

Gilles Caulier
Comment 40 Julien Narboux 2010-03-18 14:37:58 UTC
Gilles,

Concerning settings I agree, we can add a checkbox. We could also make the iso setting start at 0, and disable the shadow/midtones/highlights sliders if iso is set to 0.

>The result are not very nice about Chrominance (over-saturated). Something is
>wrong somewhere, perhaps in DColor::getYCbCr(), DColor::setYCbCr(), or in
>filter as well. I don't know yet. Investigations are require.

To test DColor::getYCbCr(), DColor::setYCbCr(), you can just remove the call to randomize to see if we get the same picture. I don't have a decent computer to test at the moment and I will be busy this week and week-end

Julien
Comment 41 caulier.gilles 2010-03-18 14:44:19 UTC
Julien,

Just tested to disable all randomize call in filter. The image is a little corrupted. Something is wrong in YCrCb color space conversion.

Gilles
Comment 42 caulier.gilles 2010-03-18 15:48:27 UTC
Created attachment 41738 [details]
Chrominance noise patch version 2

Julien,

I fixed Dcolor methods, to convert to/from YCbCr color space. 

Now the patch is updated, but something still wrong, probably the methods FilmGrainFilter::randomize() or FilmGrainFilter::interpolate() are not adapted to work with YCbCr color space.

Can you take a look ?

Gilles
Comment 43 caulier.gilles 2010-03-18 16:57:52 UTC
Created attachment 41740 [details]
 Chrominance noise patch version 3

Julien, 

Some code adjustement due to computation on value using double format.

Gilles
Comment 44 caulier.gilles 2010-03-19 15:32:08 UTC
SVN commit 1105213 by cgilles:

add settings to set luma or chroma noise independently
CCBUGS: 148540


 M  +18 -9     libs/dimg/filters/fx/filmgrainfilter.cpp  
 M  +20 -16    libs/dimg/filters/fx/filmgrainfilter.h  
 M  +56 -31    libs/dimg/filters/fx/filmgrainsettings.cpp  
 M  +40 -32    utilities/queuemanager/basetools/filters/filmgrain.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1105213
Comment 45 caulier.gilles 2010-03-22 16:05:30 UTC
Julien,

Some interesting code can be see there, especially other noise generator. But i think that Gaussian noise is the best one.

http://registry.gimp.org/node/13016

Gilles
Comment 46 caulier.gilles 2010-03-22 16:14:45 UTC
SVN commit 1106344 by cgilles:

include experimental Chrominance code here. Use preprocessor settings to test this code.
CCBUGS: 148540


 M  +57 -17    filmgrainfilter.cpp  
 M  +6 -1      filmgrainfilter.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1106344
Comment 47 caulier.gilles 2010-03-22 16:29:04 UTC
Julien, 

About default values, we have :

shadows : -100
midtones : 0
highlight : -100

It's fine for you ?

Gilles
Comment 48 Julien Narboux 2010-03-22 18:38:34 UTC
Gilles,

Yes these defautl values are fine with me (I set this myself). The logic is the following: film grain in real life is higher in midtones than in highlights and shadows. Beginners do not know that and should use the default value. Experts can change these values.

Concerning gaussian noise, I agree that we should add this and maybe only this one otherwise we will have something too complicated, that only mathematicians can understand. So we can replace the randomize function which compute a uniform noise by a function to compute a gaussian noise.

Another thing on the todo list: make the size of the noise adjustable, for the time being the size of the noise = the size of a pixel it is not realistic. We need an option to choose the size of the noise et to choose the "contrast" of the noise. So we need to generate the noise in a different layer, resize it and add it to the picture.

Julien
Comment 49 caulier.gilles 2010-03-23 12:22:26 UTC
Thanks Julien for the info.

I'm working on Chrominance noise generator using YCbCr color space. look this screenshot : 

http://farm5.static.flickr.com/4066/4457108350_cfbd666b65_o.png

Not too bad, but in shwadows tone, chromance red and blue are oversatured. This due certainly to values adjustement over the YCbCr color space limit. Some clamp need to be done. I will commit this experimental code in svn. 

Please take a look when you can.

Gilles Caulier
Comment 50 caulier.gilles 2010-03-23 16:25:53 UTC
SVN commit 1106666 by cgilles:

use double value everywhere to compute chominance adjustement.
Clamp adjusted value with noise between 0.0 and 1.0.
CCBUGS: 148540


 M  +38 -21    filmgrainfilter.cpp  
 M  +1 -1      filmgrainfilter.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1106666
Comment 51 caulier.gilles 2010-03-23 16:37:15 UTC
Julien,

I think the chrominance noise generator is not too bad, excepted in shadows and hightlight. 

Conversion in YCbCr color space is done in floating point to prevent rounding error. Noise is added to Chroma red and blue chanels and clamped to prevent saturation.

But, with highkey or lowkey photo, color are saturated if noise is too important. I don't know why exactly, but i suspect a problem with the "footroom" and the "headroom" values as explained in wikipidia page :

http://en.wikipedia.org/wiki/YCbCr

In this page these ranges are defined for 8 bits color depth integer conversion matrix, not for floating point conversion matrix. So i have a doubt here, but i think that clamping values must be adjusted accordingly [not clamp(0.0, 1.0, chroma].

What do you think about ?

Gilles Caulier
Comment 52 Julien Narboux 2010-03-23 22:45:39 UTC
Gilles,

Sorry I don't know  about color spaces, I am really not a specialist about image processing, so I can not help. If you want I can try to find people in my lab who know about that.

I tried your implementation, the result looks nice, for what it is worth ;-).

Julien
Comment 53 caulier.gilles 2010-03-24 12:43:38 UTC
Julien,

I find the problem. It due to a missing clamp of RGB values computed from YUV values during YCbCr to RGB color space conversion. All work fine now on my computers with all photo type (low-key and high-key image included). I will commit the code today and remove all method to randomize hue, which has not a sence in photo.

Now about graininess size, i'm not agree with you. If you compute grain in a separate mask and merge it with original image, you will reproduce the lead problem of original implementation which decrease lightness of image. Do you remember ? For details, look how code work in the pass here :

http://websvn.kde.org/tags/digikam/0.10.0/imageplugins/filmgrain/filmgrain.cpp?revision=945616&view=markup

... especially at line 175 and after...

We must still to apply grain to change color channel directly in YUV color space.

So the solution to change graininess size is to decompose image using a matrix of points, and to work on these points to take a care of pixels neighbor. Increasing matrix size will increase graininess size. You scan whole image using matrix size as step. I see this technic apply in VirtualDub "Add Noise" plugin available here :

http://emiliano.deepabyss.org/All_filters/All_filters_Src.zip

Gilles
Comment 54 caulier.gilles 2010-03-24 12:51:42 UTC
SVN commit 1106979 by cgilles:

compute all noise on YCbCr color space. Drop HSL color space. 

To check shadows, midtone, and highlight area of image, use Luma chanel from YUV instead Lightness chanel from HSL.

Fix all glitch color generated aroung shadows and hightlight pixels to clamp properly all values adjusted with graininess.

CCBUGS: 148540



 M  +37 -93    filmgrainfilter.cpp  
 M  +4 -6      filmgrainfilter.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1106979
Comment 55 caulier.gilles 2010-03-24 14:35:57 UTC
Created attachment 42226 [details]
First try to implement Graininess size support

Julien,

Look there :

http://farm3.static.flickr.com/2782/4459991422_cebb5591ce_o.png

Attached patch add support and experimental concept to adjust graininess size.
Of course, like you can see, I use the same adjustment for all pixel from the matrix, and result is a mosaicked image. 

The pixel on the top left is the reference to compute nRand. All other pixels from the matrix are adjusted with this value. 

The start idea is there. Instead to use same value for each matrix points, we must compute alternative random values in the matrix loop one by one. These alternative nRand must be near of the reference one.

What do you think about this concept ?

Gilles Caulier
Comment 56 caulier.gilles 2010-03-24 16:15:09 UTC
Created attachment 42235 [details]
New version of patch for grain size support

Julien,

This time, Grain Size is randomized using Guaussian noise generator for each matrix pixels. mlead matrix adjustement is computed using uniform noise generator.

Chrominance noise, grain size 1 : http://farm5.static.flickr.com/4071/4460203462_56ef6e186c_o.png
Chrominance noise, grain size 2 : http://farm3.static.flickr.com/2801/4459423743_e9727d7954_o.png
Chrominance noise, grain size 4 : http://farm3.static.flickr.com/2682/4459423911_9e6219b03e_o.png
Chrominance noise, grain size 6 : http://farm5.static.flickr.com/4010/4460203996_80660b83d7_o.png


The results are not too bad. Certainly we need to adjust better the rejection of grain size value into the gaussian noise generator (see lines 138 and 149). Please test in all cases with your image collection.

Gilles Caulier
Comment 57 caulier.gilles 2010-03-25 10:11:27 UTC
SVN commit 1107292 by cgilles:

Add film grain size settings.
Separate chrominance blue and red noise settings.
Fix inversion of shadows and highlight adjustements.
Factorize methods to set YCrCb adjustements.
Set methods as inline to speed up computation.
Use d private container.
CCBUGS: 148540



 M  +109 -44   libs/dimg/filters/fx/filmgrainfilter.cpp  
 M  +43 -24    libs/dimg/filters/fx/filmgrainfilter.h  
 M  +288 -133  libs/dimg/filters/fx/filmgrainsettings.cpp  
 M  +2 -1      libs/dimg/filters/fx/filmgrainsettings.h  
 M  +64 -40    utilities/queuemanager/basetools/filters/filmgrain.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1107292
Comment 58 caulier.gilles 2010-03-25 12:39:49 UTC
Julien,

you said previously :

"Another thing on the todo list: make the size of the noise adjustable, for the
time being the size of the noise = the size of a pixel it is not realistic. We
need an option to choose the size of the noise et to choose the "contrast" of
the noise."

About the option to choose the grain size, code is in svn trunk now.

About the contrast of the noise, all intensity adjustements are not enough ?

Gilles Caulier
Comment 59 Julien Narboux 2010-03-25 13:46:26 UTC
Gilles,

Sorry I am very busy these days.

I just tested rev 1107345, you did a wonderful job. The chroma noise looks very realistic. The photographic distribution looks very nice as well. Maybe it should just be the default.

I still see two problems:
- the grain size creates mosaics
- the photographic distribution option creates a color change, it seems that the randomization always adds a value.

I don't understand why we can not create the noise in a separate layer starting from midgrey and then merge it with the picture. To prevent the luminosity from changing I think we juste need to add the second layer-128.

Concerning the check boxes, I think it would be better to have another kind of expander widget with the check box directly included in the title of the expander right after the arrow. This would factorize code about hiding/showing the setings widgets and this widget could be used in the local contrast plugin as well. 

Julien
Comment 60 caulier.gilles 2010-03-25 13:54:47 UTC
> the grain size creates mosaics

Ah. i thinking to have fixed this problem to adjust coefficient in these lines :

n2 = randomizePoisson((d->settings.grainSize/2.0)*(range/1.414), d->settings.grainSize, col.sixteenBit());

n2 = randomizeGauss((d->settings.grainSize/2.0)*(range/1.414), col.sixteenBit());

... from FilmGrainFilter::adjustYCbCr() method. In fact (d->settings.grainSize/2.0)*(range/1.414) must randomize right to prevent mosaic. Perhaps it's not enough...

> the photographic distribution option creates a color change, it seems that
the randomization always adds a value.

Ah. i don't see that. But in some case Poisson noise generator give an infinite loop (it's a recursive method). I need to fix it or to find another algorithm (if you have one, let's me hear (:=)))

Gilles Caulier
Comment 61 Julien Narboux 2010-03-25 14:25:33 UTC
Gilles,

Here is a screenshot showing the color cast:
http://www.flickr.com/photos/julien_narboux/4462478974/in/pool-digikam-labs

Julien
Comment 62 caulier.gilles 2010-03-25 16:20:30 UTC
Julien,

Sound like Poisson noise generator has a problem.

Try to load 16 bits image and apply Photo distribution : you have an infinite loop.

And i don't find why. There is no code relevant of color bits depth in Poisson code. Any idea ?

Look randomizePoisson() method from line 226 :

http://websvn.kde.org/trunk/extragear/graphics/digikam/libs/dimg/filters/fx/filmgrainfilter.cpp?revision=1107413&view=markup

Gilles
Comment 63 Julien Narboux 2010-03-25 19:51:31 UTC
Gilles,

I solved the color cast problem by removing lambda at the end. I read that Lambda is equal to the mean for Poisson distribution.
For me using 16bits picture is very slow but it is not a loop, I do not understand why either, because the parameter of the loop in Poisson is lambda and this does not depend on 16bits or not ?

Julien
Comment 64 caulier.gilles 2010-03-26 11:53:57 UTC
SVN commit 1107638 by cgilles:

New Possoin noise generator based on an approximation of Gauss generator. It's very fast.
Now mosaic must diseapear with large film grain value
CCBUGS: 148540


 M  +5 -13     filmgrainfilter.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1107638