| Message ID | 20230626130833.15895-1-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush Thanks for the patch. On Mon, 26 Jun 2023 at 14:08, Naushir Patuck via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > The imx708 must drop a single frame on startup - but only when in HDR > mode. Non-HDR modes do not need to drop frames. Fix the logic in > hideFramesModeSwitch() which currently unconditionally advertises to > drop one frame. > > Unfortunately there is no clear way to tell if the sensor is in the HDR > mode. So for now, look the resolution and framerate to deduce this. > > Additionally ensure we override hideFramesStartup() and return the same > number as hideFramesModeSwitch(). > > Bug: https://github.com/raspberrypi/libcamera-apps/issues/524 > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Indeed, it's not lovely is it? But in the absence of a "tonemapped HDR format" or something to that effect, it's hard to think of a good solution. Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks David > --- > src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 +++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp > index 641ba18f4b9d..2cd7afdbd8ec 100644 > --- a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp > @@ -21,6 +21,8 @@ using namespace RPiController; > using namespace libcamera; > using libcamera::utils::Duration; > > +using namespace std::literals::chrono_literals; > + > namespace libcamera { > LOG_DECLARE_CATEGORY(IPARPI) > } > @@ -56,7 +58,8 @@ public: > int &vblankDelay, int &hblankDelay) const override; > bool sensorEmbeddedDataPresent() const override; > double getModeSensitivity(const CameraMode &mode) const override; > - unsigned int hideFramesModeSwitch() const override { return 1; } // seems to be required for HDR > + unsigned int hideFramesModeSwitch() const override; > + unsigned int hideFramesStartup() const; > > private: > /* > @@ -225,6 +228,26 @@ double CamHelperImx708::getModeSensitivity(const CameraMode &mode) const > return (mode.width > 2304) ? 1.0 : 2.0; > } > > +unsigned int CamHelperImx708::hideFramesModeSwitch() const > +{ > + /* > + * We need to drop the first startup frame in HDR mode. > + * Unfortunately the only way to currently determine if the sensor is in > + * the HDR mode is to match with the resolution and framerate - the HDR > + * mode only runs upto 30fps. > + */ > + if (mode_.width == 2304 && mode_.height == 1296 && > + mode_.minFrameDuration <= 1.0s / 30) > + return 1; > + else > + return 0; > +} > + > +unsigned int CamHelperImx708::hideFramesStartup() const > +{ > + return hideFramesModeSwitch(); > +} > + > void CamHelperImx708::populateMetadata(const MdParser::RegisterMap ®isters, > Metadata &metadata) const > { > -- > 2.34.1 >
diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp index 641ba18f4b9d..2cd7afdbd8ec 100644 --- a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp +++ b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp @@ -21,6 +21,8 @@ using namespace RPiController; using namespace libcamera; using libcamera::utils::Duration; +using namespace std::literals::chrono_literals; + namespace libcamera { LOG_DECLARE_CATEGORY(IPARPI) } @@ -56,7 +58,8 @@ public: int &vblankDelay, int &hblankDelay) const override; bool sensorEmbeddedDataPresent() const override; double getModeSensitivity(const CameraMode &mode) const override; - unsigned int hideFramesModeSwitch() const override { return 1; } // seems to be required for HDR + unsigned int hideFramesModeSwitch() const override; + unsigned int hideFramesStartup() const; private: /* @@ -225,6 +228,26 @@ double CamHelperImx708::getModeSensitivity(const CameraMode &mode) const return (mode.width > 2304) ? 1.0 : 2.0; } +unsigned int CamHelperImx708::hideFramesModeSwitch() const +{ + /* + * We need to drop the first startup frame in HDR mode. + * Unfortunately the only way to currently determine if the sensor is in + * the HDR mode is to match with the resolution and framerate - the HDR + * mode only runs upto 30fps. + */ + if (mode_.width == 2304 && mode_.height == 1296 && + mode_.minFrameDuration <= 1.0s / 30) + return 1; + else + return 0; +} + +unsigned int CamHelperImx708::hideFramesStartup() const +{ + return hideFramesModeSwitch(); +} + void CamHelperImx708::populateMetadata(const MdParser::RegisterMap ®isters, Metadata &metadata) const {
The imx708 must drop a single frame on startup - but only when in HDR mode. Non-HDR modes do not need to drop frames. Fix the logic in hideFramesModeSwitch() which currently unconditionally advertises to drop one frame. Unfortunately there is no clear way to tell if the sensor is in the HDR mode. So for now, look the resolution and framerate to deduce this. Additionally ensure we override hideFramesStartup() and return the same number as hideFramesModeSwitch(). Bug: https://github.com/raspberrypi/libcamera-apps/issues/524 Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 +++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)