[libcamera-devel] [PATCH 00/15] Raspberry Pi IPA code refactor

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Jul 25 17:06:44 CEST 2022


Hi Naush,

Thank you for the series.

On Mon, Jul 25, 2022 at 02:46:24PM +0100, Naushir Patuck via libcamera-devel wrote:
> Hi,
> 
> This rather large patch series performs a (largely overdue) code refactoring on
> all our IPA source files to match the libcamera coding style guidelines.
> 
> Given the size of the first round of changes (switching to CamelCase), I've split
> a single large patch into multiple smaller patches (1/15 - 10/15).  Please note,
> THESE CANNOT BE MERGED AS-IS (hence the DNI tag) as they will cause intermediate
> compile breakages.  Once ready for merging, these must be squashed into a single
> large patch. It also means that the entire context of the change will not fully
> be available in patches 1/15 - 10/15.  Apologies for that, but this seems the
> most sensible way to help with the review.

That's totally fine.

I've noticed the following remainign usages of snake_case:

md_parser.h
        int bits_per_pixel_;
        unsigned int num_lines_;
        unsigned int line_length_bytes_;
noise_status.h
        double noise_constant;
        double noise_slope;
awb.h
        int frameCount_; /* counts up to startup_frames */
        double computeDelta2Sum(double gain_r, double gainB);
agc_status.h
        char constraint_mode[32];
denoise_status.h
        double noise_constant;
        double noise_slope;
black_level_status.h
        uint16_t black_level_r; /* out of 16 bits */
        uint16_t black_level_g;
        uint16_t black_level_b;

> Naushir Patuck (15):
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   DNI: ipa: raspberrypi: Code refactoring to match style guidelines
>   ipa: raspberrypi: Change to C style code comments
>   ipa: raspberrypi: Remove extern "C" declarations
>   ipa: raspberrypi: Rename header files from *.hpp to *.h
>   raspberrypi: Update Copyright statement in all Raspberry Pi source
>     files
>   ipa: raspberryip: Remove all exception throw statements
> 
>  .reuse/dep5                                   |    2 +-
>  include/libcamera/color_space.h               |    2 +-
>  include/libcamera/internal/bayer_format.h     |    2 +-
>  include/libcamera/internal/delayed_controls.h |    2 +-
>  include/libcamera/internal/v4l2_pixelformat.h |    2 +-
>  include/libcamera/transform.h                 |    2 +-
>  include/linux/bcm2835-isp.h                   |    2 +-
>  src/cam/stream_options.cpp                    |    2 +-
>  src/cam/stream_options.h                      |    2 +-
>  src/ipa/libipa/histogram.cpp                  |    2 +-
>  src/ipa/libipa/histogram.h                    |    2 +-
>  src/ipa/raspberrypi/cam_helper.cpp            |   94 +-
>  src/ipa/raspberrypi/cam_helper.h              |  127 ++
>  src/ipa/raspberrypi/cam_helper.hpp            |  123 --
>  src/ipa/raspberrypi/cam_helper_imx219.cpp     |   40 +-
>  src/ipa/raspberrypi/cam_helper_imx290.cpp     |   36 +-
>  src/ipa/raspberrypi/cam_helper_imx296.cpp     |   28 +-
>  src/ipa/raspberrypi/cam_helper_imx477.cpp     |   78 +-
>  src/ipa/raspberrypi/cam_helper_imx519.cpp     |   76 +-
>  src/ipa/raspberrypi/cam_helper_ov5647.cpp     |   48 +-
>  src/ipa/raspberrypi/cam_helper_ov9281.cpp     |   32 +-
>  .../raspberrypi/controller/agc_algorithm.h    |   31 +
>  .../raspberrypi/controller/agc_algorithm.hpp  |   32 -
>  src/ipa/raspberrypi/controller/agc_status.h   |   46 +-
>  src/ipa/raspberrypi/controller/algorithm.cpp  |   26 +-
>  src/ipa/raspberrypi/controller/algorithm.h    |   64 +
>  src/ipa/raspberrypi/controller/algorithm.hpp  |   60 -
>  src/ipa/raspberrypi/controller/alsc_status.h  |   16 +-
>  .../raspberrypi/controller/awb_algorithm.h    |   23 +
>  .../raspberrypi/controller/awb_algorithm.hpp  |   23 -
>  src/ipa/raspberrypi/controller/awb_status.h   |   24 +-
>  .../controller/black_level_status.h           |   14 +-
>  src/ipa/raspberrypi/controller/camera_mode.h  |   54 +-
>  .../raspberrypi/controller/ccm_algorithm.h    |   21 +
>  .../raspberrypi/controller/ccm_algorithm.hpp  |   21 -
>  src/ipa/raspberrypi/controller/ccm_status.h   |   12 +-
>  .../controller/contrast_algorithm.h           |   22 +
>  .../controller/contrast_algorithm.hpp         |   22 -
>  .../raspberrypi/controller/contrast_status.h  |   16 +-
>  src/ipa/raspberrypi/controller/controller.cpp |   86 +-
>  src/ipa/raspberrypi/controller/controller.h   |   58 +
>  src/ipa/raspberrypi/controller/controller.hpp |   54 -
>  ...oise_algorithm.hpp => denoise_algorithm.h} |   12 +-
>  .../raspberrypi/controller/denoise_status.h   |   12 +-
>  .../raspberrypi/controller/device_status.cpp  |   20 +-
>  .../raspberrypi/controller/device_status.h    |   18 +-
>  src/ipa/raspberrypi/controller/dpc_status.h   |   14 +-
>  src/ipa/raspberrypi/controller/focus_status.h |   20 +-
>  src/ipa/raspberrypi/controller/geq_status.h   |   12 +-
>  src/ipa/raspberrypi/controller/histogram.cpp  |   46 +-
>  src/ipa/raspberrypi/controller/histogram.h    |   48 +
>  src/ipa/raspberrypi/controller/histogram.hpp  |   44 -
>  src/ipa/raspberrypi/controller/lux_status.h   |   28 +-
>  .../controller/{metadata.hpp => metadata.h}   |   40 +-
>  src/ipa/raspberrypi/controller/noise_status.h |   12 +-
>  src/ipa/raspberrypi/controller/pwl.cpp        |  174 +--
>  src/ipa/raspberrypi/controller/pwl.h          |  126 ++
>  src/ipa/raspberrypi/controller/pwl.hpp        |  112 --
>  src/ipa/raspberrypi/controller/rpi/agc.cpp    | 1031 +++++++++--------
>  src/ipa/raspberrypi/controller/rpi/agc.h      |  141 +++
>  src/ipa/raspberrypi/controller/rpi/agc.hpp    |  139 ---
>  src/ipa/raspberrypi/controller/rpi/alsc.cpp   |  854 +++++++-------
>  src/ipa/raspberrypi/controller/rpi/alsc.h     |  110 ++
>  src/ipa/raspberrypi/controller/rpi/alsc.hpp   |  106 --
>  src/ipa/raspberrypi/controller/rpi/awb.cpp    |  762 ++++++------
>  src/ipa/raspberrypi/controller/rpi/awb.h      |  193 +++
>  src/ipa/raspberrypi/controller/rpi/awb.hpp    |  179 ---
>  .../controller/rpi/black_level.cpp            |   46 +-
>  .../raspberrypi/controller/rpi/black_level.h  |   30 +
>  .../controller/rpi/black_level.hpp            |   30 -
>  src/ipa/raspberrypi/controller/rpi/ccm.cpp    |  117 +-
>  .../controller/rpi/{ccm.hpp => ccm.h}         |   24 +-
>  .../raspberrypi/controller/rpi/contrast.cpp   |  200 ++--
>  src/ipa/raspberrypi/controller/rpi/contrast.h |   52 +
>  .../raspberrypi/controller/rpi/contrast.hpp   |   50 -
>  src/ipa/raspberrypi/controller/rpi/dpc.cpp    |   34 +-
>  src/ipa/raspberrypi/controller/rpi/dpc.h      |   32 +
>  src/ipa/raspberrypi/controller/rpi/dpc.hpp    |   32 -
>  src/ipa/raspberrypi/controller/rpi/focus.cpp  |   18 +-
>  .../controller/rpi/{focus.hpp => focus.h}     |   12 +-
>  src/ipa/raspberrypi/controller/rpi/geq.cpp    |   64 +-
>  src/ipa/raspberrypi/controller/rpi/geq.h      |   34 +
>  src/ipa/raspberrypi/controller/rpi/geq.hpp    |   34 -
>  src/ipa/raspberrypi/controller/rpi/lux.cpp    |   90 +-
>  src/ipa/raspberrypi/controller/rpi/lux.h      |   45 +
>  src/ipa/raspberrypi/controller/rpi/lux.hpp    |   43 -
>  src/ipa/raspberrypi/controller/rpi/noise.cpp  |   60 +-
>  src/ipa/raspberrypi/controller/rpi/noise.h    |   32 +
>  src/ipa/raspberrypi/controller/rpi/noise.hpp  |   32 -
>  src/ipa/raspberrypi/controller/rpi/sdn.cpp    |   46 +-
>  src/ipa/raspberrypi/controller/rpi/sdn.h      |   32 +
>  src/ipa/raspberrypi/controller/rpi/sdn.hpp    |   32 -
>  .../raspberrypi/controller/rpi/sharpen.cpp    |   72 +-
>  src/ipa/raspberrypi/controller/rpi/sharpen.h  |   34 +
>  .../raspberrypi/controller/rpi/sharpen.hpp    |   34 -
>  .../controller/sharpen_algorithm.h            |   21 +
>  .../controller/sharpen_algorithm.hpp          |   21 -
>  .../raspberrypi/controller/sharpen_status.h   |   22 +-
>  .../{md_parser.hpp => md_parser.h}            |   40 +-
>  src/ipa/raspberrypi/md_parser_smia.cpp        |  100 +-
>  src/ipa/raspberrypi/raspberrypi.cpp           |  274 ++---
>  src/libcamera/bayer_format.cpp                |    2 +-
>  src/libcamera/color_space.cpp                 |    2 +-
>  src/libcamera/delayed_controls.cpp            |    2 +-
>  .../pipeline/raspberrypi/dma_heaps.cpp        |    2 +-
>  .../pipeline/raspberrypi/dma_heaps.h          |    2 +-
>  .../pipeline/raspberrypi/raspberrypi.cpp      |    2 +-
>  .../pipeline/raspberrypi/rpi_stream.cpp       |    2 +-
>  .../pipeline/raspberrypi/rpi_stream.h         |    2 +-
>  src/libcamera/transform.cpp                   |    2 +-
>  src/libcamera/v4l2_pixelformat.cpp            |    2 +-
>  src/qcam/dng_writer.cpp                       |    2 +-
>  src/qcam/dng_writer.h                         |    2 +-
>  utils/raspberrypi/ctt/ctt.py                  |    2 +-
>  utils/raspberrypi/ctt/ctt_alsc.py             |    2 +-
>  utils/raspberrypi/ctt/ctt_awb.py              |    2 +-
>  utils/raspberrypi/ctt/ctt_ccm.py              |    2 +-
>  utils/raspberrypi/ctt/ctt_geq.py              |    2 +-
>  utils/raspberrypi/ctt/ctt_image_load.py       |    2 +-
>  utils/raspberrypi/ctt/ctt_lux.py              |    2 +-
>  utils/raspberrypi/ctt/ctt_macbeth_locator.py  |    2 +-
>  utils/raspberrypi/ctt/ctt_noise.py            |    2 +-
>  .../raspberrypi/ctt/ctt_pretty_print_json.py  |    2 +-
>  utils/raspberrypi/ctt/ctt_ransac.py           |    2 +-
>  utils/raspberrypi/ctt/ctt_tools.py            |    2 +-
>  125 files changed, 3817 insertions(+), 3752 deletions(-)
>  create mode 100644 src/ipa/raspberrypi/cam_helper.h
>  delete mode 100644 src/ipa/raspberrypi/cam_helper.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/agc_algorithm.h
>  delete mode 100644 src/ipa/raspberrypi/controller/agc_algorithm.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/algorithm.h
>  delete mode 100644 src/ipa/raspberrypi/controller/algorithm.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/awb_algorithm.h
>  delete mode 100644 src/ipa/raspberrypi/controller/awb_algorithm.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/ccm_algorithm.h
>  delete mode 100644 src/ipa/raspberrypi/controller/ccm_algorithm.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/contrast_algorithm.h
>  delete mode 100644 src/ipa/raspberrypi/controller/contrast_algorithm.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/controller.h
>  delete mode 100644 src/ipa/raspberrypi/controller/controller.hpp
>  rename src/ipa/raspberrypi/controller/{denoise_algorithm.hpp => denoise_algorithm.h} (53%)
>  create mode 100644 src/ipa/raspberrypi/controller/histogram.h
>  delete mode 100644 src/ipa/raspberrypi/controller/histogram.hpp
>  rename src/ipa/raspberrypi/controller/{metadata.hpp => metadata.h} (61%)
>  create mode 100644 src/ipa/raspberrypi/controller/pwl.h
>  delete mode 100644 src/ipa/raspberrypi/controller/pwl.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/agc.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/agc.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/alsc.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/alsc.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/awb.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/awb.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/black_level.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/black_level.hpp
>  rename src/ipa/raspberrypi/controller/rpi/{ccm.hpp => ccm.h} (68%)
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/contrast.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/contrast.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/dpc.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/dpc.hpp
>  rename src/ipa/raspberrypi/controller/rpi/{focus.hpp => focus.h} (59%)
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/geq.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/geq.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/lux.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/lux.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/noise.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/noise.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/sdn.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/sdn.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/rpi/sharpen.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/sharpen.hpp
>  create mode 100644 src/ipa/raspberrypi/controller/sharpen_algorithm.h
>  delete mode 100644 src/ipa/raspberrypi/controller/sharpen_algorithm.hpp
>  rename src/ipa/raspberrypi/{md_parser.hpp => md_parser.h} (80%)

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list