Message ID | 20230615182044.95347-1-umang.jain@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Umang On Thu, 15 Jun 2023 at 19:20, Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > From: sdfasdf <asdfasdfasdf@asdfasdf.com> > > Adds the PDAF support for IMX519 camera module by Arducam. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > Patch originally not authored by me, but we are missing main author's > full-name and email. Partly for my own curiosity, where has this patch come from? Arducam have published their libcamera tree[1], and this almost looks like their commit [2] which does have a commit author (no S-o-b though). Dave [1] https://github.com/ArduCAM/libcamera [2] https://github.com/ArduCAM/libcamera/commit/b20728fe2a37ee1cbdeb8c4b2473d0f5035771e4 > --- > 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 ®isters, > 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 ®isters, > 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": > { > -- > 2.39.1 >
Hi, On 6/16/23 10:12 PM, Dave Stevenson wrote: > Hi Umang > > On Thu, 15 Jun 2023 at 19:20, Umang Jain via libcamera-devel > <libcamera-devel@lists.libcamera.org> wrote: >> From: sdfasdf <asdfasdfasdf@asdfasdf.com> >> >> Adds the PDAF support for IMX519 camera module by Arducam. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> Patch originally not authored by me, but we are missing main author's >> full-name and email. > Partly for my own curiosity, where has this patch come from? > > Arducam have published their libcamera tree[1], and this almost looks > like their commit [2] which does have a commit author (no S-o-b > though). Oh so there is a libcamera published tree from them, I didn't know. I referred to [3] instead and didn't checkout the fork. Anyways, if we have an author and their full name, it should be merged with their name. So will update. It has been modified/rebased as per the latest reworks on rpi/vc4 PH and IPA to get compiled successfully on master. [3] https://github.com/raspberrypi/libcamera/commit/b20728fe2a37ee1cbdeb8c4b2473d0f5035771e4 > > Dave > > [1] https://github.com/ArduCAM/libcamera > [2] https://github.com/ArduCAM/libcamera/commit/b20728fe2a37ee1cbdeb8c4b2473d0f5035771e4 > >> --- >> 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 ®isters, >> 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 ®isters, >> 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": >> { >> -- >> 2.39.1 >>
Hi Umang, On Thu, Jun 15, 2023 at 11:50:44PM +0530, Umang Jain via libcamera-devel wrote: > From: sdfasdf <asdfasdfasdf@asdfasdf.com> > > Adds the PDAF support for IMX519 camera module by Arducam. > > Signed-off-by: Umang Jain <umang.jain@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. > --- > 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 ®isters, > 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 ®isters, > 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": > {
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@asdfasdf.com> >> >> Adds the PDAF support for IMX519 camera module by Arducam. >> >> Signed-off-by: Umang Jain <umang.jain@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@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@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 ®isters, >> 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 ®isters, >> 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": >> {
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@asdfasdf.com> > >> > >> Adds the PDAF support for IMX519 camera module by Arducam. > >> > >> Signed-off-by: Umang Jain <umang.jain@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@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@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 ®isters, > >> 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 ®isters, > >> 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": > >> { >
Hi, CC'ing John at Arducam, On 6/19/23 1:57 PM, Kieran Bingham wrote: > 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@asdfasdf.com> >>>> >>>> Adds the PDAF support for IMX519 camera module by Arducam. >>>> >>>> Signed-off-by: Umang Jain <umang.jain@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@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@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. There's now a v2 on the list as well, and john@arducam.com is CC'ed. I will wait for them to provide a S-o-B. > > -- > 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 ®isters, >>>> 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 ®isters, >>>> 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": >>>> {
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 ®isters, 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 ®isters, 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": {