[libcamera-devel] [PATCH] ipa: rpi: cam_helper: PDAF support for IMX519

Kieran Bingham kieran.bingham at ideasonboard.com
Mon Jun 19 10:27:27 CEST 2023


Quoting Umang Jain via libcamera-devel (2023-06-18 11:03:56)
> Hi Laurent,
> 
> On 6/17/23 8:20 PM, Laurent Pinchart wrote:
> > Hi Umang,
> >
> > On Thu, Jun 15, 2023 at 11:50:44PM +0530, Umang Jain via libcamera-devel wrote:
> >> From: sdfasdf <asdfasdfasdf at asdfasdf.com>
> >>
> >> Adds the PDAF support for IMX519 camera module by Arducam.
> >>
> >> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >> ---
> >> Patch originally not authored by me, but we are missing main author's
> >> full-name and email.
> > That needs to be fixed. We can't merge code of unknown origin. The
> > original author has to be recorded, the origin of the code clearly
> > indicated, and the author should be CC'ed.
> 
> There is,
> 
> Author: john <john at arducam.com>
> Date:   Thu Mar 9 16:12:37 2023 +0800
> 
> But no S-o-B tag when the commits are checked on [1]
> 
> Does this suffice ? And should  I add an additional S-o-B ?
> 
>      Signed-off-by: john <john at arducam.com>

No, you can't add someone elses 'signature' if they haven't provided it.

We'll have to contact Arducam and get them to provide a Signed-off-by:
to certify they authored the code.

--
Kieran


> [1]: https://github.com/ArduCAM/libcamera
> >
> >> ---
> >>   src/ipa/rpi/cam_helper/cam_helper_imx519.cpp | 50 ++++++++++++++++++++
> >>   src/ipa/rpi/vc4/data/imx519.json             | 40 ++++++++++++++++
> >>   2 files changed, 90 insertions(+)
> >>
> >> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
> >> index c7262aa0..6d032082 100644
> >> --- a/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
> >> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
> >> @@ -15,9 +15,13 @@
> >>   
> >>   #include <libcamera/base/log.h>
> >>   
> >> +#include "controller/pdaf_data.h"
> >> +
> >>   #include "cam_helper.h"
> >>   #include "md_parser.h"
> >>   
> >> +#define ALIGN_UP(x, a) (((x) + (a)-1) & ~(a - 1))
> >> +
> >>   using namespace RPiController;
> >>   using namespace libcamera;
> >>   using libcamera::utils::Duration;
> >> @@ -66,8 +70,13 @@ private:
> >>      /* Largest long exposure scale factor given as a left shift on the frame length. */
> >>      static constexpr int longExposureShiftMax = 7;
> >>   
> >> +    static constexpr int pdafStatsRows = 12;
> >> +    static constexpr int pdafStatsCols = 16;
> >> +
> >>      void populateMetadata(const MdParser::RegisterMap &registers,
> >>                            Metadata &metadata) const override;
> >> +    static bool parsePdafData(const uint8_t *ptr, size_t len, unsigned bpp,
> >> +                              PdafRegions &pdaf);
> >>   };
> >>   
> >>   CamHelperImx519::CamHelperImx519()
> >> @@ -90,6 +99,11 @@ void CamHelperImx519::prepare(libcamera::Span<const uint8_t> buffer, Metadata &m
> >>      MdParser::RegisterMap registers;
> >>      DeviceStatus deviceStatus;
> >>   
> >> +    LOG(IPARPI, Debug) << "Embedded buffer size: " << buffer.size();
> >> +
> >> +    size_t bytesPerLine = (mode_.width * mode_.bitdepth) >> 3;
> >> +    bytesPerLine = ALIGN_UP(bytesPerLine, 16);
> >> +
> >>      if (metadata.get("device.status", deviceStatus)) {
> >>              LOG(IPARPI, Error) << "DeviceStatus not found from DelayedControls";
> >>              return;
> >> @@ -97,6 +111,14 @@ void CamHelperImx519::prepare(libcamera::Span<const uint8_t> buffer, Metadata &m
> >>   
> >>      parseEmbeddedData(buffer, metadata);
> >>   
> >> +    if (buffer.size() > 2 * bytesPerLine) {
> >> +            PdafRegions pdaf;
> >> +            parsePdafData(&buffer[2 * bytesPerLine],
> >> +                          buffer.size() - 2 * bytesPerLine,
> >> +                          mode_.bitdepth, pdaf);
> >> +            metadata.set("pdaf.data", pdaf);
> >> +    }
> >> +
> >>      /*
> >>       * The DeviceStatus struct is first populated with values obtained from
> >>       * DelayedControls. If this reports frame length is > frameLengthMax,
> >> @@ -188,6 +210,34 @@ void CamHelperImx519::populateMetadata(const MdParser::RegisterMap &registers,
> >>      metadata.set("device.status", deviceStatus);
> >>   }
> >>   
> >> +bool CamHelperImx519::parsePdafData(const uint8_t *ptr, size_t len,
> >> +                                unsigned bpp, PdafRegions &pdaf)
> >> +{
> >> +    size_t step = bpp >> 1; /* bytes per PDAF grid entry */
> >> +
> >> +    if (bpp < 10 || bpp > 12 || len < 194 * step || ptr[0] != 0 || ptr[1] >= 0x40) {
> >> +            LOG(IPARPI, Error) << "PDAF data in unsupported format";
> >> +            return false;
> >> +    }
> >> +
> >> +    pdaf.init({ pdafStatsCols, pdafStatsRows });
> >> +
> >> +    ptr += 2 * step;
> >> +    for (unsigned i = 0; i < pdafStatsRows; ++i) {
> >> +            for (unsigned j = 0; j < pdafStatsCols; ++j) {
> >> +                    unsigned c = (ptr[0] << 3) | (ptr[1] >> 5);
> >> +                    int p = (((ptr[1] & 0x0F) - (ptr[1] & 0x10)) << 6) | (ptr[2] >> 2);
> >> +                    PdafData pdafData;
> >> +                    pdafData.conf = c;
> >> +                    pdafData.phase = c ? p : 0;
> >> +                    pdaf.set(libcamera::Point(j, i), { pdafData, 1, 0 });
> >> +                    ptr += step;
> >> +            }
> >> +    }
> >> +
> >> +    return true;
> >> +}
> >> +
> >>   static CamHelper *create()
> >>   {
> >>      return new CamHelperImx519();
> >> diff --git a/src/ipa/rpi/vc4/data/imx519.json b/src/ipa/rpi/vc4/data/imx519.json
> >> index 1b0a7747..0733d97e 100644
> >> --- a/src/ipa/rpi/vc4/data/imx519.json
> >> +++ b/src/ipa/rpi/vc4/data/imx519.json
> >> @@ -350,6 +350,46 @@
> >>                   ]
> >>               }
> >>           },
> >> +        {
> >> +            "rpi.af":
> >> +            {
> >> +                "ranges":
> >> +                {
> >> +                    "normal":
> >> +                    {
> >> +                        "min": 0.0,
> >> +                        "max": 12.0,
> >> +                        "default": 1.0
> >> +                    },
> >> +                    "macro":
> >> +                    {
> >> +                        "min": 3.0,
> >> +                        "max": 15.0,
> >> +                        "default": 4.0
> >> +                    }
> >> +                },
> >> +                "speeds":
> >> +                {
> >> +                    "normal":
> >> +                    {
> >> +                        "step_coarse": 1.0,
> >> +                        "step_fine": 0.25,
> >> +                        "contrast_ratio": 0.75,
> >> +                        "pdaf_gain": -0.02,
> >> +                        "pdaf_squelch": 0.125,
> >> +                        "max_slew": 2.0,
> >> +                        "pdaf_frames": 20,
> >> +                        "dropout_frames": 6,
> >> +                        "step_frames": 4
> >> +                    }
> >> +                },
> >> +                "conf_epsilon": 8,
> >> +                "conf_thresh": 16,
> >> +                "conf_clip": 512,
> >> +                "skip_frames": 5,
> >> +                "map": [ 0.0, 0.0, 15.0, 4095 ]
> >> +            }
> >> +        },
> >>           {
> >>               "rpi.ccm":
> >>               {
>


More information about the libcamera-devel mailing list