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

[Umang Jain]

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>

[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":
>>               {