Removing Canon Banding with PJSR

[Niall Saunders]

Niall, don't worry. Juan doesn't know what the words "life" or "vacation" means

Yes Carlos - we know that. But I live in fear that, one day, he might accidentally see pictures of people enjoying themselves. Perhaps even outside, in the sunshine. We must all do our best to prevent that from ever happening.

Juan should always remain convinced that stale pizza and warm beer IS a healthy and nutritional diet. And that sore, red, dry eyes are the colour they are SUPPOSED to be (same goes for the numbness of a programmer's posterior - you are not supposed to be able to feel your legs anyway, it's an over-rated sensation !!) And that his wife and family don't 'really' need him (after all his youngest child has long been too scared to go into the darkness that is his programming room, because of a natural fear of 'disturbing the beast therein').

We CAN control him you know - like all of Pavlov's dogs, we just need to keep heaping accolades upon him - his responses are always the same : 'program harder' and 'make more users happier'. Now that we have PI scaling the dizzying heights of APoD it will be even easier (just TELL him that we made another APod, he'll be too busy debugging to actually go and check  )

Cheers,
Niall

[Juan Conejero]

Warm beer! Give me a good Ribera del Duero, Bierzo or Utiel-Requena (some Riojas may serve, too), or I won't even consider start working! Well, if you insist, a good Bourgogne could also do the trick.

[David Serrano]

I like the design you suggest, although I'd like to contribute a couple of ideas and questions (yes, questions are contributions):

* The external interface is relatively simple. It reduces to adding a unique String parameter to Script. A formal description for this parameter would be:

[...]

Note that the above description fits strictly within JavaScript syntax.

Good. As for the actual implementation, if I were you, I would choose JSON without a doubt. So we have a Script process with two read-only properties: .parameters (JSON, XML or whatever) and .codeText (UTF8 text), right?

[Internal interface]
- During execution, a script has access to several read-only properties of a special JavaScript object. As you probably figure out at this point, that properties are just the result of interpreting and executing Script::parameters as JavaScript code.

...by "eval"ing JSON .

But, as I understand it, when the user runs the code, Script.parameters is still empty (the Script instance needs to be created after the code ends execution, so that the user could have had a chance to configure it, via the usual configuration dialog), hence during execution of the script we don't know what the properties of the Parameters object are.

Thus, creating a Script instance directly from the editor window ("dragging the small blue triangle to the desktop area") wouldn't make much sense, since that would create a Settings process with an empty .parameters property (the script hasn't been run), and upon dragging that instance onto an image (i.e. execute it), the script's configuration dialog would configure that instance, but somehow that data should make its way to the Script icon that lies on the desktop. Is that an OO violation?

EXCEPT that users were expected to define a special function (e.g. "CONFIGURE") that did the usual configuration work (show a dialog or whatever) and returned a data structure. Then PJSR would gain a good share of freedom:

- when "dragging the triangle to the desktop", PJSR would be told to run that function (a dialog is shown), and we could have a full-blown Scripts object, already configured, ready to be applied to images (and to ImageContainers, yippie!). When applied, the Parameters object would have to be populated from Script.parameters (eval the JSON, or whatever).

- when running scripts from the editor, PJSR could set up the Parameters object from the data returned from CONFIGURE, and then would run the script as usual. The script would NOT need to call CONFIGURE.

(I have a little idea regarding the Settings object, but is offtopic in this discussion - feel free to ask though!).

Code: [Select]

   // Send parameters and values to Core.
   this.broadcast = function()

Since "broadcast" is a detail about how things work in PI Core, and users don't need to know anything about it, I would call this method "put" or something like that.

Code: [Select]

Object Parameters.get( String id )
Acquires the specified parameter id and returns its value, if it exists, or undefined otherwise.

I was going to suggest that the .has method isn't actually needed, since we could do (pseudocode):

Code: [Select]

foreach param ('radius', 'lineWidth', 'helloText') {
    value = Parameters.get (param);
    if (undef == value) { next; }
    this[param] = value;
}
But this breaks when a parameter exists and is undefined, since this code would treat this case as if the parameter didn't exist at all. Your .get is broken in the same way, the code that calls your .get and gets an undef, would interpret it as if the parameter didn't exist. I thing .get should throw an exception when asked for an non-existing parameter. OTOH, making a .get die seems quite radical to me, but that's what .has is for. Maybe a big red warning in the documentation could suffice. Or maybe the whole notion of "parameter having an undefined value" makes no sense at all.

Summary:

Code: [Select]

function CONFIGURE
{
   // obtain values from the usual configuration dialog, or whatever. Store them in this.

   // give data back to PJSR, which would set up Parameters
   return this;
}

function main()
{
   // directly use parameters:
   var diameter = Parameters.radius * 2 * PI;
   // ...
}
No MyParameters.retrieve, since the configuration values are class properties, readily visible in Parameters (read-only). No .broadcast either, given that that work is already done.

I can try to implement this for the upcoming version 1.5.8, or postpone it for 1.6, depending on the difficulties that I find, but this definitely has to be done. What do you think?

You know, release early, release often!   (although I must admit, to my embarrasment, that I don't follow that...). I guess, though, that my suggestions are quite big a change, and would need to wait 1.6 - they break compatibility. PJSR could search for a function "CONFIGURE" and, depending on its existence, could behave as it does now or enable the new functionality. Then, in PI 1.7 or 1.8, support for old scripts could be dropped.

I guess PI 2.0 will never see the light, it's one of those big projects that you can't give an overhaul big enough to warrant a major version bump. We'll have 1.10 instead .

[Juan Conejero]

Hi David

yes, questions are contributions

Of course. Always!

As for the actual implementation, if I were you, I would choose JSON without a doubt. So we have a Script process with two read-only properties: .parameters (JSON, XML or whatever) and .codeText (UTF8 text), right?

First of all, please take a look at today's prerelease information post, if you haven't already done so.

I was considering JSON (XML is just too cumbersome to implement this functionality, IMO). However I have decided to abandon this idea, for security reasons. JSON means calling the eval function to evaluate the parameters, and this is extremely dangerous because it is an easy way to inject malicious code.

I have preferred a much more controlled implementation, where all parameters are plain strings and are never evaluated as JavaScript code. Sure this is much less flexible than JSON, but it is secure, and provides the necessary functionality.

...by "eval"ing JSON

Same as before. eval() is very dangerous.

But, as I understand it, when the user runs the code, Script.parameters is still empty (the Script instance needs to be created after the code ends execution, so that the user could have had a chance to configure it, via the usual configuration dialog), hence during execution of the script we don't know what the properties of the Parameters object are.

Correct. This happens when a script is being executed in the "direct context", e.g. from the Script Editor --see my prerelease post. The new Script interface allows you to edit Script instances, just as a regular process. The main difference with a module-defined process is that there are no type checks at all for script working parameters, as corresponds to the dynamic nature of the JavaScript language. See my post for more on this.

EXCEPT that users were expected to define a special function (e.g. "CONFIGURE") that did the usual configuration work (show a dialog or whatever) and returned a data structure. Then PJSR would gain a good share of freedom:

I have implemented a different approach, which is based on the Parameters object, and is even simpler (IMO) than relying on a "standard" function name. You have described the exact functionality that Script has; my implementation is different (mainly due to security issues), but the goals and the achieved functionality are the same. We can improve it more, of course, if necessary.

(I have a little idea regarding the Settings object, but is offtopic in this discussion - feel free to ask though!).

Consider yourself asked 

Since "broadcast" is a detail about how things work in PI Core, and users don't need to know anything about it, I would call this method "put" or something like that.

Finally, I have preferred "export", but a script is free to use any name for such functions. Parameter's methods are "get" and "set", which are not very original indeed

But this breaks when a parameter exists and is undefined, since this code would treat this case as if the parameter didn't exist at all. Your .get is broken in the same way, the code that calls your .get and gets an undef, would interpret it as if the parameter didn't exist. I thing .get should throw an exception when asked for an non-existing parameter. OTOH, making a .get die seems quite radical to me, but that's what .has is for. Maybe a big red warning in the documentation could suffice. Or maybe the whole notion of "parameter having an undefined value" makes no sense at all.

Interesting points. However since script parameters are String objects, no parameter can be undefined or null (it can be an empty string, of course). Calling Parameters.get() for a nonexistent parameter does throw an Error exception. The Parameters.has() method is necessary in this context.

I guess PI 2.0 will never see the light, it's one of those big projects that you can't give an overhaul big enough to warrant a major version bump. We'll have 1.10 instead

Oh yes it will see the light, of course. One reason is that "PixInsight 2.0" looks so nice (as a headline), that I won't be able to resist 8) (joke!)

[David Serrano]

The main difference with a module-defined process is that there are no type checks at all for script working parameters, as corresponds to the dynamic nature of the JavaScript language. See my post for more on this.

Ah, no need to see your post on this. Perl is a dynamic language so I know very well what you're talking about . Dynamic languages for the win!!

(I have a little idea regarding the Settings object, but is offtopic in this discussion - feel free to ask though!).

Consider yourself asked 

I don't seem to recall it now . 'Twas something along the lines of "after running CONFIGURE(), automatically save the user-specified parameters to Settings" or something like that.

[about undefined parameters and .get behavior]
Interesting points. However since script parameters are String objects, no parameter can be undefined or null (it can be an empty string, of course). Calling Parameters.get() for a nonexistent parameter does throw an Error exception. The Parameters.has() method is necessary in this context.

Then, if undefined values make no sense, you could dump .has entirely and rely on .get returning:

1.- A (possibly empty) value, meaning that the parameter exists, and at the same time returning the actual value of course.
2.- Undef, meaning that the parameter doesn't exist.

I don't know how this matches the multiple getters that will be implemented in Parameters (one for each data type). Don't have time to think about it, I should be sleeping right now, and I still want to make a comment in the 1.5.8 thread!

[georg.viehoever]

Dear all,

here is a new version of the Canon Banding Script, an update to the version that comes with PI 1.5.9. It fixes one important issue (wrong computation of some statistics), but also reduces memory consumption and improves speed. It is a drop in replacement for the current script. Below the "Release Notes".

Enjoy,
Georg

   Changelog:
   0.9.0: Review and bugfix release:
         - removed bug in statistics computation. Only the global mean of R was computed correctly, sometimes resulting
           in a colour change of the fixed image.
         - removed wasting of memory: 64 bit images were used as intermediates even when 32 bit was sufficient. For example,
           full CanonEOS40D images can now be handled with Vista32 in 3 GBytes.
         - The fix above also improves performance.
         - changed normalization of fixed images: instead of using image.normalize, which changes the range covered by a
           dark image to cover the whole range from 0.0 to 1.0, I just clip values not to exceed this range. Should make postprocessing
           easier.
         - using fixed identifier ""tmpSTFWindow" for the hidden ImageWindow used by STF. Avoids use of "Image1", "Image2" etc.
         - added some comments

[Juan Conejero]

Very nice work, Georg. I'll include this version in the next release of PixInsight.