Message ID | 20200924091927.116385-1-naush@raspberrypi.com |
---|---|
Headers | show |
Series |
|
Related | show |
Hi all, Gentle nudge on this one. If there are no further comments, I think this is ready to be submitted - all review tags are present. Many thanks, Naush On Thu, 24 Sep 2020 at 10:19, Naushir Patuck <naush@raspberrypi.com> wrote: > > Hi, > > Thank you all for the comments on v1. This v2 update addresses all that was discussed: > > - Reword commit message for patch 1/4. > - Re-order commits 2/4 and 3/4 to have a more logical flow of changes. > - Some blank lines added in 4/4 for better code readability. > - constexpr int -> constexpr double > > I have not added an ipa:: namespace. This may warrant some more thought, would we > consider adding libcamera core parameters to this namespace, or is it going to be > vendor specific? Either way, I think this can be addeded at a later date. > > Regards, > Naush > > > Naushir Patuck (4): > pipeline: raspberrypi: Rename RPi::RPiStream and RPi::RPiDevice > ipa: raspberrypi: Rename the controller namespace from Rpi to > RpiController > ipa: raspberrypi: Move IPA parameters to the RPi namespace > ipa: raspberrypi: Tidy up variable names to be consistent > > include/libcamera/ipa/raspberrypi.h | 42 +-- > src/ipa/raspberrypi/cam_helper.cpp | 2 +- > src/ipa/raspberrypi/cam_helper.hpp | 2 +- > src/ipa/raspberrypi/cam_helper_imx219.cpp | 2 +- > src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +- > src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +- > .../raspberrypi/controller/agc_algorithm.hpp | 4 +- > src/ipa/raspberrypi/controller/algorithm.cpp | 4 +- > src/ipa/raspberrypi/controller/algorithm.hpp | 4 +- > .../raspberrypi/controller/awb_algorithm.hpp | 4 +- > .../raspberrypi/controller/ccm_algorithm.hpp | 4 +- > .../controller/contrast_algorithm.hpp | 4 +- > src/ipa/raspberrypi/controller/controller.cpp | 2 +- > src/ipa/raspberrypi/controller/controller.hpp | 4 +- > src/ipa/raspberrypi/controller/histogram.cpp | 2 +- > src/ipa/raspberrypi/controller/histogram.hpp | 4 +- > src/ipa/raspberrypi/controller/metadata.hpp | 4 +- > src/ipa/raspberrypi/controller/pwl.cpp | 2 +- > src/ipa/raspberrypi/controller/pwl.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/agc.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/agc.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/awb.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 +- > .../controller/rpi/black_level.cpp | 2 +- > .../controller/rpi/black_level.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +- > .../raspberrypi/controller/rpi/contrast.cpp | 2 +- > .../raspberrypi/controller/rpi/contrast.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/focus.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/focus.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/geq.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/geq.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/lux.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/lux.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/noise.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/noise.hpp | 4 +- > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 2 +- > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 4 +- > .../raspberrypi/controller/rpi/sharpen.cpp | 2 +- > .../raspberrypi/controller/rpi/sharpen.hpp | 4 +- > .../controller/sharpen_algorithm.hpp | 4 +- > src/ipa/raspberrypi/md_parser.cpp | 2 +- > src/ipa/raspberrypi/md_parser.hpp | 2 +- > src/ipa/raspberrypi/md_parser_rpi.cpp | 2 +- > src/ipa/raspberrypi/md_parser_rpi.hpp | 2 +- > src/ipa/raspberrypi/raspberrypi.cpp | 269 +++++++++--------- > .../pipeline/raspberrypi/raspberrypi.cpp | 84 +++--- > .../pipeline/raspberrypi/rpi_stream.cpp | 38 +-- > .../pipeline/raspberrypi/rpi_stream.h | 20 +- > 54 files changed, 304 insertions(+), 295 deletions(-) > > -- > 2.25.1 >
Hi Naush, On 29/09/2020 09:46, Naushir Patuck wrote: > Hi all, > > Gentle nudge on this one. If there are no further comments, I think > this is ready to be submitted - all review tags are present. Indeed, I will be handling this integration along with David's 2D transform series this morning. Thanks. Kieran > > Many thanks, > Naush > > > On Thu, 24 Sep 2020 at 10:19, Naushir Patuck <naush@raspberrypi.com> wrote: >> >> Hi, >> >> Thank you all for the comments on v1. This v2 update addresses all that was discussed: >> >> - Reword commit message for patch 1/4. >> - Re-order commits 2/4 and 3/4 to have a more logical flow of changes. >> - Some blank lines added in 4/4 for better code readability. >> - constexpr int -> constexpr double >> >> I have not added an ipa:: namespace. This may warrant some more thought, would we >> consider adding libcamera core parameters to this namespace, or is it going to be >> vendor specific? Either way, I think this can be addeded at a later date. >> >> Regards, >> Naush >> >> >> Naushir Patuck (4): >> pipeline: raspberrypi: Rename RPi::RPiStream and RPi::RPiDevice >> ipa: raspberrypi: Rename the controller namespace from Rpi to >> RpiController >> ipa: raspberrypi: Move IPA parameters to the RPi namespace >> ipa: raspberrypi: Tidy up variable names to be consistent >> >> include/libcamera/ipa/raspberrypi.h | 42 +-- >> src/ipa/raspberrypi/cam_helper.cpp | 2 +- >> src/ipa/raspberrypi/cam_helper.hpp | 2 +- >> src/ipa/raspberrypi/cam_helper_imx219.cpp | 2 +- >> src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +- >> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +- >> .../raspberrypi/controller/agc_algorithm.hpp | 4 +- >> src/ipa/raspberrypi/controller/algorithm.cpp | 4 +- >> src/ipa/raspberrypi/controller/algorithm.hpp | 4 +- >> .../raspberrypi/controller/awb_algorithm.hpp | 4 +- >> .../raspberrypi/controller/ccm_algorithm.hpp | 4 +- >> .../controller/contrast_algorithm.hpp | 4 +- >> src/ipa/raspberrypi/controller/controller.cpp | 2 +- >> src/ipa/raspberrypi/controller/controller.hpp | 4 +- >> src/ipa/raspberrypi/controller/histogram.cpp | 2 +- >> src/ipa/raspberrypi/controller/histogram.hpp | 4 +- >> src/ipa/raspberrypi/controller/metadata.hpp | 4 +- >> src/ipa/raspberrypi/controller/pwl.cpp | 2 +- >> src/ipa/raspberrypi/controller/pwl.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/agc.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/agc.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/awb.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 +- >> .../controller/rpi/black_level.cpp | 2 +- >> .../controller/rpi/black_level.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +- >> .../raspberrypi/controller/rpi/contrast.cpp | 2 +- >> .../raspberrypi/controller/rpi/contrast.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/focus.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/focus.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/geq.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/geq.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/lux.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/lux.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/noise.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/noise.hpp | 4 +- >> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 2 +- >> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 4 +- >> .../raspberrypi/controller/rpi/sharpen.cpp | 2 +- >> .../raspberrypi/controller/rpi/sharpen.hpp | 4 +- >> .../controller/sharpen_algorithm.hpp | 4 +- >> src/ipa/raspberrypi/md_parser.cpp | 2 +- >> src/ipa/raspberrypi/md_parser.hpp | 2 +- >> src/ipa/raspberrypi/md_parser_rpi.cpp | 2 +- >> src/ipa/raspberrypi/md_parser_rpi.hpp | 2 +- >> src/ipa/raspberrypi/raspberrypi.cpp | 269 +++++++++--------- >> .../pipeline/raspberrypi/raspberrypi.cpp | 84 +++--- >> .../pipeline/raspberrypi/rpi_stream.cpp | 38 +-- >> .../pipeline/raspberrypi/rpi_stream.h | 20 +- >> 54 files changed, 304 insertions(+), 295 deletions(-) >> >> -- >> 2.25.1 >> > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel >
Hi Naush, On 29/09/2020 09:50, Kieran Bingham wrote: > Hi Naush, > > On 29/09/2020 09:46, Naushir Patuck wrote: >> Hi all, >> >> Gentle nudge on this one. If there are no further comments, I think >> this is ready to be submitted - all review tags are present. > > Indeed, I will be handling this integration along with David's 2D > transform series this morning. There were some merge conflicts to deal with, and a checkstyle warning that crept in. I've fixed up the warning, and conflicts due to appyling on top of David's 2d-transform series, but as your series was only non-functional changes I had a higher confidence that it was correct when compiled, and have pushed upstream. Let me know if you notice any issues. Regards Kieran > Thanks. > > Kieran > > >> >> Many thanks, >> Naush >> >> >> On Thu, 24 Sep 2020 at 10:19, Naushir Patuck <naush@raspberrypi.com> wrote: >>> >>> Hi, >>> >>> Thank you all for the comments on v1. This v2 update addresses all that was discussed: >>> >>> - Reword commit message for patch 1/4. >>> - Re-order commits 2/4 and 3/4 to have a more logical flow of changes. >>> - Some blank lines added in 4/4 for better code readability. >>> - constexpr int -> constexpr double >>> >>> I have not added an ipa:: namespace. This may warrant some more thought, would we >>> consider adding libcamera core parameters to this namespace, or is it going to be >>> vendor specific? Either way, I think this can be addeded at a later date. >>> >>> Regards, >>> Naush >>> >>> >>> Naushir Patuck (4): >>> pipeline: raspberrypi: Rename RPi::RPiStream and RPi::RPiDevice >>> ipa: raspberrypi: Rename the controller namespace from Rpi to >>> RpiController >>> ipa: raspberrypi: Move IPA parameters to the RPi namespace >>> ipa: raspberrypi: Tidy up variable names to be consistent >>> >>> include/libcamera/ipa/raspberrypi.h | 42 +-- >>> src/ipa/raspberrypi/cam_helper.cpp | 2 +- >>> src/ipa/raspberrypi/cam_helper.hpp | 2 +- >>> src/ipa/raspberrypi/cam_helper_imx219.cpp | 2 +- >>> src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +- >>> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +- >>> .../raspberrypi/controller/agc_algorithm.hpp | 4 +- >>> src/ipa/raspberrypi/controller/algorithm.cpp | 4 +- >>> src/ipa/raspberrypi/controller/algorithm.hpp | 4 +- >>> .../raspberrypi/controller/awb_algorithm.hpp | 4 +- >>> .../raspberrypi/controller/ccm_algorithm.hpp | 4 +- >>> .../controller/contrast_algorithm.hpp | 4 +- >>> src/ipa/raspberrypi/controller/controller.cpp | 2 +- >>> src/ipa/raspberrypi/controller/controller.hpp | 4 +- >>> src/ipa/raspberrypi/controller/histogram.cpp | 2 +- >>> src/ipa/raspberrypi/controller/histogram.hpp | 4 +- >>> src/ipa/raspberrypi/controller/metadata.hpp | 4 +- >>> src/ipa/raspberrypi/controller/pwl.cpp | 2 +- >>> src/ipa/raspberrypi/controller/pwl.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/agc.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/agc.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/awb.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 +- >>> .../controller/rpi/black_level.cpp | 2 +- >>> .../controller/rpi/black_level.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +- >>> .../raspberrypi/controller/rpi/contrast.cpp | 2 +- >>> .../raspberrypi/controller/rpi/contrast.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/focus.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/focus.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/geq.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/geq.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/lux.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/lux.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/noise.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/noise.hpp | 4 +- >>> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 2 +- >>> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 4 +- >>> .../raspberrypi/controller/rpi/sharpen.cpp | 2 +- >>> .../raspberrypi/controller/rpi/sharpen.hpp | 4 +- >>> .../controller/sharpen_algorithm.hpp | 4 +- >>> src/ipa/raspberrypi/md_parser.cpp | 2 +- >>> src/ipa/raspberrypi/md_parser.hpp | 2 +- >>> src/ipa/raspberrypi/md_parser_rpi.cpp | 2 +- >>> src/ipa/raspberrypi/md_parser_rpi.hpp | 2 +- >>> src/ipa/raspberrypi/raspberrypi.cpp | 269 +++++++++--------- >>> .../pipeline/raspberrypi/raspberrypi.cpp | 84 +++--- >>> .../pipeline/raspberrypi/rpi_stream.cpp | 38 +-- >>> .../pipeline/raspberrypi/rpi_stream.h | 20 +- >>> 54 files changed, 304 insertions(+), 295 deletions(-) >>> >>> -- >>> 2.25.1 >>> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel >> >
Thanks Kieran, will test it out and let you know if I run into any issues. Regards, Naush On Tue, 29 Sep 2020 at 13:02, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Hi Naush, > > On 29/09/2020 09:50, Kieran Bingham wrote: > > Hi Naush, > > > > On 29/09/2020 09:46, Naushir Patuck wrote: > >> Hi all, > >> > >> Gentle nudge on this one. If there are no further comments, I think > >> this is ready to be submitted - all review tags are present. > > > > Indeed, I will be handling this integration along with David's 2D > > transform series this morning. > > > There were some merge conflicts to deal with, and a checkstyle warning > that crept in. > > I've fixed up the warning, and conflicts due to appyling on top of > David's 2d-transform series, but as your series was only non-functional > changes I had a higher confidence that it was correct when compiled, and > have pushed upstream. > > Let me know if you notice any issues. > > Regards > > Kieran > > > > > > Thanks. > > > > Kieran > > > > > >> > >> Many thanks, > >> Naush > >> > >> > >> On Thu, 24 Sep 2020 at 10:19, Naushir Patuck <naush@raspberrypi.com> wrote: > >>> > >>> Hi, > >>> > >>> Thank you all for the comments on v1. This v2 update addresses all that was discussed: > >>> > >>> - Reword commit message for patch 1/4. > >>> - Re-order commits 2/4 and 3/4 to have a more logical flow of changes. > >>> - Some blank lines added in 4/4 for better code readability. > >>> - constexpr int -> constexpr double > >>> > >>> I have not added an ipa:: namespace. This may warrant some more thought, would we > >>> consider adding libcamera core parameters to this namespace, or is it going to be > >>> vendor specific? Either way, I think this can be addeded at a later date. > >>> > >>> Regards, > >>> Naush > >>> > >>> > >>> Naushir Patuck (4): > >>> pipeline: raspberrypi: Rename RPi::RPiStream and RPi::RPiDevice > >>> ipa: raspberrypi: Rename the controller namespace from Rpi to > >>> RpiController > >>> ipa: raspberrypi: Move IPA parameters to the RPi namespace > >>> ipa: raspberrypi: Tidy up variable names to be consistent > >>> > >>> include/libcamera/ipa/raspberrypi.h | 42 +-- > >>> src/ipa/raspberrypi/cam_helper.cpp | 2 +- > >>> src/ipa/raspberrypi/cam_helper.hpp | 2 +- > >>> src/ipa/raspberrypi/cam_helper_imx219.cpp | 2 +- > >>> src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +- > >>> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +- > >>> .../raspberrypi/controller/agc_algorithm.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/algorithm.cpp | 4 +- > >>> src/ipa/raspberrypi/controller/algorithm.hpp | 4 +- > >>> .../raspberrypi/controller/awb_algorithm.hpp | 4 +- > >>> .../raspberrypi/controller/ccm_algorithm.hpp | 4 +- > >>> .../controller/contrast_algorithm.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/controller.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/controller.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/histogram.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/histogram.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/metadata.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/pwl.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/pwl.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/agc.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/agc.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/awb.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 +- > >>> .../controller/rpi/black_level.cpp | 2 +- > >>> .../controller/rpi/black_level.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +- > >>> .../raspberrypi/controller/rpi/contrast.cpp | 2 +- > >>> .../raspberrypi/controller/rpi/contrast.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/focus.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/focus.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/geq.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/geq.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/lux.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/lux.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/noise.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/noise.hpp | 4 +- > >>> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 2 +- > >>> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 4 +- > >>> .../raspberrypi/controller/rpi/sharpen.cpp | 2 +- > >>> .../raspberrypi/controller/rpi/sharpen.hpp | 4 +- > >>> .../controller/sharpen_algorithm.hpp | 4 +- > >>> src/ipa/raspberrypi/md_parser.cpp | 2 +- > >>> src/ipa/raspberrypi/md_parser.hpp | 2 +- > >>> src/ipa/raspberrypi/md_parser_rpi.cpp | 2 +- > >>> src/ipa/raspberrypi/md_parser_rpi.hpp | 2 +- > >>> src/ipa/raspberrypi/raspberrypi.cpp | 269 +++++++++--------- > >>> .../pipeline/raspberrypi/raspberrypi.cpp | 84 +++--- > >>> .../pipeline/raspberrypi/rpi_stream.cpp | 38 +-- > >>> .../pipeline/raspberrypi/rpi_stream.h | 20 +- > >>> 54 files changed, 304 insertions(+), 295 deletions(-) > >>> > >>> -- > >>> 2.25.1 > >>> > >> _______________________________________________ > >> libcamera-devel mailing list > >> libcamera-devel@lists.libcamera.org > >> https://lists.libcamera.org/listinfo/libcamera-devel > >> > > > > -- > Regards > -- > Kieran