| Message ID | 20220714152409.9780-8-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Thu, Jul 14, 2022 at 04:24:08PM +0100, Naushir Patuck via libcamera-devel wrote: > Use the tuning file conversion script to output the new tuning file format from > the CTT. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > utils/raspberrypi/ctt/ctt.py | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/utils/raspberrypi/ctt/ctt.py b/utils/raspberrypi/ctt/ctt.py > index 15064634c67f..0d91ada15afa 100755 > --- a/utils/raspberrypi/ctt/ctt.py > +++ b/utils/raspberrypi/ctt/ctt.py > @@ -15,7 +15,7 @@ from ctt_alsc import * > from ctt_lux import * > from ctt_noise import * > from ctt_geq import * > -from ctt_pretty_print_json import * ctt_pretty_print_json.py isn't used anymore, should it be dropped ? > +from convert_tuning import convert_v2 > import random > import json > import re > @@ -511,13 +511,12 @@ class Camera: > """ > def write_json(self): > """ > - Write json dictionary to file > + Write json dictionary to file using our version 2 format > """ > - jstring = json.dumps(self.json, sort_keys=False) > - """ > - make it pretty :) > - """ > - pretty_print_json(jstring, self.jf) > + out_json = convert_v2(self.json) That's cheating :-) I'm OK with it for now, Would it be difficult to output the v2 format natively though ? It seems that something along the lines of the following code should do: out_json = { "version": "2.0", "algorithms": [{name: data} for name, data in self.json.items()], } and then you could import the Encoder class from convert_tuning instead of the convert_v2 function, and do with open(self.jf, 'w') as f: f.write(json.dumps(out_json, cls=Encoder, indent=4, sort_keys=False)) Possibly even better, the Encoder class could be moved from convert_tuning.py to ctt_pretty_print_json and replace the existing hand-written JSONPrettyPrinter. > + > + with open(self.jf, 'w') as f: > + f.write(out_json) > > """ > add a new section to the log file
Hi Laurent, Thank you for your feedback. On Fri, 15 Jul 2022 at 00:55, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Naush, > > Thank you for the patch. > > On Thu, Jul 14, 2022 at 04:24:08PM +0100, Naushir Patuck via > libcamera-devel wrote: > > Use the tuning file conversion script to output the new tuning file > format from > > the CTT. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > utils/raspberrypi/ctt/ctt.py | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/utils/raspberrypi/ctt/ctt.py b/utils/raspberrypi/ctt/ctt.py > > index 15064634c67f..0d91ada15afa 100755 > > --- a/utils/raspberrypi/ctt/ctt.py > > +++ b/utils/raspberrypi/ctt/ctt.py > > @@ -15,7 +15,7 @@ from ctt_alsc import * > > from ctt_lux import * > > from ctt_noise import * > > from ctt_geq import * > > -from ctt_pretty_print_json import * > > ctt_pretty_print_json.py isn't used anymore, should it be dropped ? > > > +from convert_tuning import convert_v2 > > import random > > import json > > import re > > @@ -511,13 +511,12 @@ class Camera: > > """ > > def write_json(self): > > """ > > - Write json dictionary to file > > + Write json dictionary to file using our version 2 format > > """ > > - jstring = json.dumps(self.json, sort_keys=False) > > - """ > > - make it pretty :) > > - """ > > - pretty_print_json(jstring, self.jf) > > + out_json = convert_v2(self.json) > > That's cheating :-) I'm OK with it for now, Would it be difficult to > output the v2 format natively though ? It seems that something along the > lines of the following code should do: > > out_json = { > "version": "2.0", > "algorithms": [{name: data} for name, data in > self.json.items()], > } > > and then you could import the Encoder class from convert_tuning instead > of the convert_v2 function, and do > > with open(self.jf, 'w') as f: > f.write(json.dumps(out_json, cls=Encoder, indent=4, > sort_keys=False)) > > Possibly even better, the Encoder class could be moved from > convert_tuning.py to ctt_pretty_print_json and replace the existing > hand-written JSONPrettyPrinter. > I think I would prefer the first suggestion and delete ctt_pretty_print_json.py. It pretty much does what convert_tuning.py does, so seems redundant. Naush > > > + > > + with open(self.jf, 'w') as f: > > + f.write(out_json) > > > > """ > > add a new section to the log file > > -- > Regards, > > Laurent Pinchart >
Hi Laurent, On Fri, 15 Jul 2022 at 09:39, Naushir Patuck <naush@raspberrypi.com> wrote: > Hi Laurent, > > Thank you for your feedback. > > On Fri, 15 Jul 2022 at 00:55, Laurent Pinchart < > laurent.pinchart@ideasonboard.com> wrote: > >> Hi Naush, >> >> Thank you for the patch. >> >> On Thu, Jul 14, 2022 at 04:24:08PM +0100, Naushir Patuck via >> libcamera-devel wrote: >> > Use the tuning file conversion script to output the new tuning file >> format from >> > the CTT. >> > >> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> >> > --- >> > utils/raspberrypi/ctt/ctt.py | 13 ++++++------- >> > 1 file changed, 6 insertions(+), 7 deletions(-) >> > >> > diff --git a/utils/raspberrypi/ctt/ctt.py b/utils/raspberrypi/ctt/ctt.py >> > index 15064634c67f..0d91ada15afa 100755 >> > --- a/utils/raspberrypi/ctt/ctt.py >> > +++ b/utils/raspberrypi/ctt/ctt.py >> > @@ -15,7 +15,7 @@ from ctt_alsc import * >> > from ctt_lux import * >> > from ctt_noise import * >> > from ctt_geq import * >> > -from ctt_pretty_print_json import * >> >> ctt_pretty_print_json.py isn't used anymore, should it be dropped ? >> >> > +from convert_tuning import convert_v2 >> > import random >> > import json >> > import re >> > @@ -511,13 +511,12 @@ class Camera: >> > """ >> > def write_json(self): >> > """ >> > - Write json dictionary to file >> > + Write json dictionary to file using our version 2 format >> > """ >> > - jstring = json.dumps(self.json, sort_keys=False) >> > - """ >> > - make it pretty :) >> > - """ >> > - pretty_print_json(jstring, self.jf) >> > + out_json = convert_v2(self.json) >> >> That's cheating :-) I'm OK with it for now, Would it be difficult to >> output the v2 format natively though ? It seems that something along the >> lines of the following code should do: >> >> out_json = { >> "version": "2.0", >> "algorithms": [{name: data} for name, data in >> self.json.items()], >> } >> >> and then you could import the Encoder class from convert_tuning instead >> of the convert_v2 function, and do >> >> with open(self.jf, 'w') as f: >> f.write(json.dumps(out_json, cls=Encoder, indent=4, >> sort_keys=False)) >> >> Possibly even better, the Encoder class could be moved from >> convert_tuning.py to ctt_pretty_print_json and replace the existing >> hand-written JSONPrettyPrinter. >> > > I think I would prefer the first suggestion and delete > ctt_pretty_print_json.py. > It pretty much does what convert_tuning.py does, so seems redundant. > Actually, looking at it further, perhaps I should move the entire contents of convert_tuning.py into ctt_pretty_print_json.py as the former does not only convert but also prettifies any flat json. What do you think? > Naush > > > >> >> > + >> > + with open(self.jf, 'w') as f: >> > + f.write(out_json) >> > >> > """ >> > add a new section to the log file >> >> -- >> Regards, >> >> Laurent Pinchart >> >
Hi Naush, On Fri, Jul 15, 2022 at 09:51:53AM +0100, Naushir Patuck wrote: > On Fri, 15 Jul 2022 at 09:39, Naushir Patuck wrote: > > On Fri, 15 Jul 2022 at 00:55, Laurent Pinchart wrote: > >> On Thu, Jul 14, 2022 at 04:24:08PM +0100, Naushir Patuck via libcamera-devel wrote: > >> > Use the tuning file conversion script to output the new tuning file format from > >> > the CTT. > >> > > >> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > >> > --- > >> > utils/raspberrypi/ctt/ctt.py | 13 ++++++------- > >> > 1 file changed, 6 insertions(+), 7 deletions(-) > >> > > >> > diff --git a/utils/raspberrypi/ctt/ctt.py b/utils/raspberrypi/ctt/ctt.py > >> > index 15064634c67f..0d91ada15afa 100755 > >> > --- a/utils/raspberrypi/ctt/ctt.py > >> > +++ b/utils/raspberrypi/ctt/ctt.py > >> > @@ -15,7 +15,7 @@ from ctt_alsc import * > >> > from ctt_lux import * > >> > from ctt_noise import * > >> > from ctt_geq import * > >> > -from ctt_pretty_print_json import * > >> > >> ctt_pretty_print_json.py isn't used anymore, should it be dropped ? > >> > >> > +from convert_tuning import convert_v2 > >> > import random > >> > import json > >> > import re > >> > @@ -511,13 +511,12 @@ class Camera: > >> > """ > >> > def write_json(self): > >> > """ > >> > - Write json dictionary to file > >> > + Write json dictionary to file using our version 2 format > >> > """ > >> > - jstring = json.dumps(self.json, sort_keys=False) > >> > - """ > >> > - make it pretty :) > >> > - """ > >> > - pretty_print_json(jstring, self.jf) > >> > + out_json = convert_v2(self.json) > >> > >> That's cheating :-) I'm OK with it for now, Would it be difficult to > >> output the v2 format natively though ? It seems that something along the > >> lines of the following code should do: > >> > >> out_json = { > >> "version": "2.0", > >> "algorithms": [{name: data} for name, data in > >> self.json.items()], > >> } > >> > >> and then you could import the Encoder class from convert_tuning instead > >> of the convert_v2 function, and do > >> > >> with open(self.jf, 'w') as f: > >> f.write(json.dumps(out_json, cls=Encoder, indent=4, > >> sort_keys=False)) > >> > >> Possibly even better, the Encoder class could be moved from > >> convert_tuning.py to ctt_pretty_print_json and replace the existing > >> hand-written JSONPrettyPrinter. > > > > I think I would prefer the first suggestion and delete ctt_pretty_print_json.py. > > It pretty much does what convert_tuning.py does, so seems redundant. > > Actually, looking at it further, perhaps I should move the entire contents of convert_tuning.py > into ctt_pretty_print_json.py as the former does not only convert but also prettifies any > flat json. What do you think? Hmmmm... I think it would be nice to keep v1-related code centralized in a single place (or in as few places as possible). I would thus prefer moving only the Encoder class to ctt_pretty_print_json.py, and have convert_tuning.py be a small wrapper around it that converts to the latest version of tuning files. ctt_pretty_print_json.py can also be invoked directly, I would use that as a pretty printer/reformatter instead of advertising convert_tuning.py as a formatting tool. > >> > + > >> > + with open(self.jf, 'w') as f: > >> > + f.write(out_json) > >> > > >> > """ > >> > add a new section to the log file
diff --git a/utils/raspberrypi/ctt/ctt.py b/utils/raspberrypi/ctt/ctt.py index 15064634c67f..0d91ada15afa 100755 --- a/utils/raspberrypi/ctt/ctt.py +++ b/utils/raspberrypi/ctt/ctt.py @@ -15,7 +15,7 @@ from ctt_alsc import * from ctt_lux import * from ctt_noise import * from ctt_geq import * -from ctt_pretty_print_json import * +from convert_tuning import convert_v2 import random import json import re @@ -511,13 +511,12 @@ class Camera: """ def write_json(self): """ - Write json dictionary to file + Write json dictionary to file using our version 2 format """ - jstring = json.dumps(self.json, sort_keys=False) - """ - make it pretty :) - """ - pretty_print_json(jstring, self.jf) + out_json = convert_v2(self.json) + + with open(self.jf, 'w') as f: + f.write(out_json) """ add a new section to the log file
Use the tuning file conversion script to output the new tuning file format from the CTT. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- utils/raspberrypi/ctt/ctt.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)