[{"id":14182,"web_url":"https://patchwork.libcamera.org/comment/14182/","msgid":"<CAHW6GYL-xCkHLwRa30zRMYXxFL1MfOsjXuCu=q3nEK79-t2FhQ@mail.gmail.com>","date":"2020-12-10T09:05:49","subject":"Re: [libcamera-devel] [PATCH v4 2/3] libcamera: raspberrypi: Add\n\tcontrol of sensor vblanking","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch. Just a couple of very minor nit-picks/questions!\n\nOn Wed, 9 Dec 2020 at 10:26, Naushir Patuck <naush@raspberrypi.com> wrote:\n>\n> Add support for setting V4L2_CID_VBLANK appropriately when setting\n> V4L2_CID_EXPOSURE. This will allow adaptive framerates during\n> viewfinder use cases (e.g. when the exposure time goes above 33ms, we\n> can reduce the framerate to lower than 30fps).\n>\n> The minimum and maximum frame durations are provided via libcamera\n> controls, and will prioritise exposure time limits over any AGC request.\n>\n> V4L2_CID_VBLANK is controlled through the staggered writer, just like\n> the exposure and gain controls.\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  include/libcamera/ipa/raspberrypi.h           |  1 +\n>  src/ipa/raspberrypi/cam_helper.cpp            | 37 ++++++++++++++++-\n>  src/ipa/raspberrypi/cam_helper.hpp            | 15 ++++++-\n>  src/ipa/raspberrypi/cam_helper_imx219.cpp     | 13 +++++-\n>  src/ipa/raspberrypi/cam_helper_imx477.cpp     | 11 ++++-\n>  src/ipa/raspberrypi/cam_helper_ov5647.cpp     | 11 ++++-\n>  src/ipa/raspberrypi/raspberrypi.cpp           | 41 ++++++++++++++++---\n>  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n>  8 files changed, 119 insertions(+), 13 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index 01fe5abc..160ca681 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -65,6 +65,7 @@ static const ControlInfoMap Controls = {\n>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n>         { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n>         { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> +       { &controls::FrameDurations, ControlInfo(1.0e3f, 1.0e9f) },\n\nJust for my curiosity, any specific reasons for these limits?\n\n>  };\n>\n>  } /* namespace RPi */\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> index c8ac3232..03da127f 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -34,8 +34,10 @@ CamHelper *CamHelper::Create(std::string const &cam_name)\n>         return nullptr;\n>  }\n>\n> -CamHelper::CamHelper(MdParser *parser)\n> -       : parser_(parser), initialized_(false)\n> +CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength,\n> +                    unsigned int frameIntegrationDiff)\n> +       : parser_(parser), initialized_(false), maxFrameLength_(maxFrameLength),\n> +         frameIntegrationDiff_(frameIntegrationDiff)\n>  {\n>  }\n>\n> @@ -56,6 +58,37 @@ double CamHelper::Exposure(uint32_t exposure_lines) const\n>         return exposure_lines * mode_.line_length / 1000.0;\n>  }\n>\n> +uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,\n> +                                double maxFrameDuration) const\n> +{\n> +       uint32_t frameLengthMin, frameLengthMax, vblank;\n> +       uint32_t exposureLines = ExposureLines(exposure);\n> +\n> +       assert(initialized_);\n> +\n> +       /*\n> +        * Clip frame length by the frame duration range and the maximum allowable\n> +        * value in the sensor, given by maxFrameLength_.\n> +        */\n> +       frameLengthMin = std::clamp<uint32_t>(1e3 * minFrameDuration / mode_.line_length,\n> +                                             mode_.height, maxFrameLength_);\n\n(note 1 - see below)\n\n> +       frameLengthMax = std::clamp<uint32_t>(1e3 * maxFrameDuration / mode_.line_length,\n> +                                             mode_.height, maxFrameLength_);\n> +       /*\n> +        * Limit the exposure to the maximum frame duration requested, and\n> +        * re-calculate if it has been clipped.\n> +        */\n> +       exposureLines = std::min(frameLengthMax - frameIntegrationDiff_, exposureLines);\n> +       exposure = Exposure(exposureLines);\n> +\n> +       /* Limit the vblank to the range allowed by the frame length limits. */\n> +       vblank = std::max<uint32_t>(exposureLines + frameIntegrationDiff_, mode_.height);\n\n(note 2 - see below)\n\n> +       vblank = std::clamp(vblank - mode_.height,\n> +                           frameLengthMin - mode_.height, frameLengthMax - mode_.height);\n\nI did just wonder about rewriting this last line as\n\nvblank = std::clamp(vblank, frameLengthMin, frameLengthMax) - mode.height;\n\nThat also makes it more obvious to me that the line identified by note\n2 is to some extent redundant, because the line identified by note 1\nassures frameLengthMin >= mode_.height already.\n\nSo maybe these last 2 lines would be clearer as:\n\nvblank = std::clamp(exposureLines + frameIntegrationDiff,\nframeLengthMin, frameLengthMax) - mode.height;\n\nDid I get that right? Anyway, all small stuff!\n\n> +\n> +       return vblank;\n> +}\n> +\n>  void CamHelper::SetCameraMode(const CameraMode &mode)\n>  {\n>         mode_ = mode;\n> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\n> index 044c2866..b1739a57 100644\n> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> @@ -62,12 +62,15 @@ class CamHelper\n>  {\n>  public:\n>         static CamHelper *Create(std::string const &cam_name);\n> -       CamHelper(MdParser *parser);\n> +       CamHelper(MdParser *parser, unsigned int maxFrameLength,\n> +                 unsigned int frameIntegrationDiff);\n>         virtual ~CamHelper();\n>         void SetCameraMode(const CameraMode &mode);\n>         MdParser &Parser() const { return *parser_; }\n>         uint32_t ExposureLines(double exposure_us) const;\n>         double Exposure(uint32_t exposure_lines) const; // in us\n> +       virtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,\n> +                                     double maxFrameDuration) const;\n>         virtual uint32_t GainCode(double gain) const = 0;\n>         virtual double Gain(uint32_t gain_code) const = 0;\n>         virtual void GetDelays(int &exposure_delay, int &gain_delay) const;\n> @@ -76,10 +79,20 @@ public:\n>         virtual unsigned int HideFramesModeSwitch() const;\n>         virtual unsigned int MistrustFramesStartup() const;\n>         virtual unsigned int MistrustFramesModeSwitch() const;\n> +\n>  protected:\n>         MdParser *parser_;\n>         CameraMode mode_;\n> +\n> +private:\n>         bool initialized_;\n> +       /* Largest possible frame length, in units of lines. */\n> +       unsigned int maxFrameLength_;\n> +       /*\n> +        * Smallest difference between the frame length and integration time,\n> +        * in units of lines.\n> +        */\n> +       unsigned int frameIntegrationDiff_;\n>  };\n>\n>  // This is for registering camera helpers with the system, so that the\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index db8ab879..8688ec09 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -49,13 +49,22 @@ public:\n>         double Gain(uint32_t gain_code) const override;\n>         unsigned int MistrustFramesModeSwitch() const override;\n>         bool SensorEmbeddedDataPresent() const override;\n> +\n> +private:\n> +       /*\n> +        * Smallest difference between the frame length and integration time,\n> +        * in units of lines.\n> +        */\n> +       static constexpr int frameIntegrationDiff = 4;\n> +       /* Largest possible frame length, in units of lines. */\n> +       static constexpr int maxFrameLength = 0xffff;\n>  };\n>\n>  CamHelperImx219::CamHelperImx219()\n>  #if ENABLE_EMBEDDED_DATA\n> -       : CamHelper(new MdParserImx219())\n> +       : CamHelper(new MdParserImx219(), maxFrameLength, frameIntegrationDiff)\n>  #else\n> -       : CamHelper(new MdParserRPi())\n> +       : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)\n>  #endif\n>  {\n>  }\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index 0e896ac7..53961310 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -38,10 +38,19 @@ public:\n>         uint32_t GainCode(double gain) const override;\n>         double Gain(uint32_t gain_code) const override;\n>         bool SensorEmbeddedDataPresent() const override;\n> +\n> +private:\n> +       /*\n> +        * Smallest difference between the frame length and integration time,\n> +        * in units of lines.\n> +        */\n> +       static constexpr int frameIntegrationDiff = 22;\n> +       /* Largest possible frame length, in units of lines. */\n> +       static constexpr int maxFrameLength = 0xffdc;\n>  };\n>\n>  CamHelperImx477::CamHelperImx477()\n> -       : CamHelper(new MdParserImx477())\n> +       : CamHelper(new MdParserImx477(), maxFrameLength, frameIntegrationDiff)\n>  {\n>  }\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> index dc5d8275..7630c127 100644\n> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> @@ -22,6 +22,15 @@ public:\n>         unsigned int HideFramesModeSwitch() const override;\n>         unsigned int MistrustFramesStartup() const override;\n>         unsigned int MistrustFramesModeSwitch() const override;\n> +\n> +private:\n> +       /*\n> +        * Smallest difference between the frame length and integration time,\n> +        * in units of lines.\n> +        */\n> +       static constexpr int frameIntegrationDiff = 4;\n> +       /* Largest possible frame length, in units of lines. */\n> +       static constexpr int maxFrameLength = 0xffff;\n>  };\n>\n>  /*\n> @@ -30,7 +39,7 @@ public:\n>   */\n>\n>  CamHelperOv5647::CamHelperOv5647()\n> -       : CamHelper(new MdParserRPi())\n> +       : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)\n>  {\n>  }\n>\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 60cfdc27..4e7c2b94 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -58,6 +58,8 @@ namespace libcamera {\n>  /* Configure the sensor with these values initially. */\n>  constexpr double DefaultAnalogueGain = 1.0;\n>  constexpr unsigned int DefaultExposureTime = 20000;\n> +constexpr double defaultMinFrameDuration = 1e6 / 30.0;\n> +constexpr double defaultMaxFrameDuration = 1e6 / 0.01;\n>\n>  LOG_DEFINE_CATEGORY(IPARPI)\n>\n> @@ -145,6 +147,9 @@ private:\n>         /* LS table allocation passed in from the pipeline handler. */\n>         FileDescriptor lsTableHandle_;\n>         void *lsTable_;\n> +\n> +       /* Frame duration (1/fps) given in microseconds. */\n> +       double minFrameDuration_, maxFrameDuration_;\n>  };\n>\n>  int IPARPi::init(const IPASettings &settings)\n> @@ -266,7 +271,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n>                 sensorMetadata = helper_->SensorEmbeddedDataPresent();\n>\n>                 result->data.push_back(gainDelay);\n> -               result->data.push_back(exposureDelay);\n> +               result->data.push_back(exposureDelay); /* FOR EXPOSURE ctrl */\n> +               result->data.push_back(exposureDelay); /* For VBLANK ctrl */\n>                 result->data.push_back(sensorMetadata);\n>\n>                 result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;\n> @@ -335,6 +341,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n>                 AgcStatus agcStatus;\n>                 agcStatus.shutter_time = DefaultExposureTime;\n>                 agcStatus.analogue_gain = DefaultAnalogueGain;\n> +               minFrameDuration_ = defaultMinFrameDuration;\n> +               maxFrameDuration_ = defaultMaxFrameDuration;\n>                 applyAGC(&agcStatus, ctrls);\n>\n>                 result->controls.emplace_back(ctrls);\n> @@ -712,6 +720,17 @@ void IPARPi::queueRequest(const ControlList &controls)\n>                         break;\n>                 }\n>\n> +               case controls::FRAME_DURATIONS: {\n> +                       auto frameDurations = ctrl.second.get<Span<const float>>();\n> +\n> +                       /* This will be applied once AGC recalculations occur. */\n> +                       minFrameDuration_ = frameDurations[0];\n> +                       maxFrameDuration_ = frameDurations[1];\n> +                       libcameraMetadata_.set(controls::FrameDurations,\n> +                                              { frameDurations[0], frameDurations[1] });\n\nWould \"libcameraMetadata_.set(controls::FrameDurations,\nframeDurations)\" work? Just a teeny bit tider!\n\nMy other general comment is that the subsequent patch you sent me,\nabout getting the vblank setting to happen first, seems to me like it\nwill be important. But we can do that as a subsequent patch, right?\n\nI also think we'll want to pursue the matter about getting the max\nexposure into the AGC, but that's not critical-path right now!\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\nTested-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks again and best regards\nDavid\n\n> +                       break;\n> +               }\n> +\n>                 default:\n>                         LOG(IPARPI, Warning)\n>                                 << \"Ctrl \" << controls::controls.at(ctrl.first)->name()\n> @@ -882,7 +901,12 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n>  void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n>  {\n>         int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n> -       int32_t exposureLines = helper_->ExposureLines(agcStatus->shutter_time);\n> +\n> +       /* GetVBlanking might clip exposure time to the fps limits. */\n> +       double exposure = agcStatus->shutter_time;\n> +       int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_,\n> +                                                 maxFrameDuration_);\n> +       int32_t exposureLines = helper_->ExposureLines(exposure);\n>\n>         if (unicamCtrls_.find(V4L2_CID_ANALOGUE_GAIN) == unicamCtrls_.end()) {\n>                 LOG(IPARPI, Error) << \"Can't find analogue gain control\";\n> @@ -894,13 +918,20 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n>                 return;\n>         }\n>\n> -       LOG(IPARPI, Debug) << \"Applying AGC Exposure: \" << agcStatus->shutter_time\n> -                          << \" (Shutter lines: \" << exposureLines << \") Gain: \"\n> +       LOG(IPARPI, Debug) << \"Applying AGC Exposure: \" << exposure\n> +                          << \" (Shutter lines: \" << exposureLines << \", AGC requested \"\n> +                          << agcStatus->shutter_time << \") Gain: \"\n>                            << agcStatus->analogue_gain << \" (Gain Code: \"\n>                            << gainCode << \")\";\n>\n> -       ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);\n> +       /*\n> +        * Due to the behavior of V4L2, the current value of VBLANK could clip the\n> +        * exposure time without us knowing. The next time though this function should\n> +        * clip exposure correctly.\n> +        */\n> +       ctrls.set(V4L2_CID_VBLANK, vblanking);\n>         ctrls.set(V4L2_CID_EXPOSURE, exposureLines);\n> +       ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);\n>  }\n>\n>  void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 2ec1f6e7..13349f31 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1221,7 +1221,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n>                 if (!staggeredCtrl_) {\n>                         staggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n>                                             { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },\n> -                                             { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });\n> +                                             { V4L2_CID_EXPOSURE, result.data[resultIdx++] },\n> +                                             { V4L2_CID_VBLANK, result.data[resultIdx++] } });\n>                         sensorMetadata_ = result.data[resultIdx++];\n>                 }\n>         }\n> --\n> 2.25.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8A9A1BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Dec 2020 09:06:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 030C467F6E;\n\tThu, 10 Dec 2020 10:06:05 +0100 (CET)","from mail-oo1-xc43.google.com (mail-oo1-xc43.google.com\n\t[IPv6:2607:f8b0:4864:20::c43])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD53C60323\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 10:06:02 +0100 (CET)","by mail-oo1-xc43.google.com with SMTP id i18so1102717ooh.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 01:06:02 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Olowtj/m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=pFj6Tz12E/1FvQK4IIQr+SPH1+a1Z5zQs2d+/9z1otU=;\n\tb=Olowtj/miI99RDw6Yi4Tc53DKryXWUCJx0vX0g2VbJu05S2/NHK9N1N25WQcMJBAiO\n\tpt1ZJv3OS6XZ08N+8MjF4wJhz9EL/2Lq4AdtKbS8GP8xXTmPkv5C7eDe1eTteh/RH7VU\n\tb+62ZhdEKTBktnL6+9X874gSoylD9sg2E50Sb6aJo/GAS5/Y2U/8Cn+dt1fbgNIjftAl\n\tuxQinYGft1yDy8b43iY+/OFkKOwW+bd0TcdIVS8w+jZJmT2W2dKnP4a57/5XaM7+zoxA\n\tj7DK0EoRpaAeMUucpZGoEdjIZgruN7sAjzXzRbd9OrpE7Mf3Rrwj+74yXSeDGuO+vhjq\n\t20qA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=pFj6Tz12E/1FvQK4IIQr+SPH1+a1Z5zQs2d+/9z1otU=;\n\tb=gZrACUHDRaGNFlG+J+31f8u0svzOeOqihietly6D1rORPdPJIpvD6g6GoOTCiUDZS0\n\tdatgPPhS6f9/cwhjrh9QDbm6ad7VVPf4jJco9vZk58QbFfp61pztt5oAPcSLtXfcy3H0\n\tQm6uYaA7HQqaTgZ3kr3z/0IbM30Jm4q1CT7bTs/Iv/AKGHzYcvIdakLrj9ZFXmTONw+H\n\toUBdDdHoetRby6DLR5HelC0YthBjtrsMMvIeCkN4CudkjUw/ol8PuSGuRQdixWghWkQU\n\tvs/VSa5BctkrFTYHIykj0DCXUJMeWvhXn4iJaEmKc6eBNO/L0JAgewEzlXRPNn/ak7dI\n\tQJUQ==","X-Gm-Message-State":"AOAM532TSHH0OspXsNeuHLtl79NaWqH5oDSPJ0KW2iWS6NgJsqcUwtGj\n\tG3kytycbd4Rlf3Hq4/G25rfX1KUcWcvCWbtRszo0xA==","X-Google-Smtp-Source":"ABdhPJw6RaVXyEf1xETceCltNniCyIcJfX2DG/quXBmdotc4b6Eiy2YHd93FtkZDa7SyDhPkjGuZ7aI/o++1o+VOqLU=","X-Received":"by 2002:a4a:a444:: with SMTP id w4mr5180675ool.5.1607591161289; \n\tThu, 10 Dec 2020 01:06:01 -0800 (PST)","MIME-Version":"1.0","References":"<20201209102630.5562-1-naush@raspberrypi.com>\n\t<20201209102630.5562-3-naush@raspberrypi.com>","In-Reply-To":"<20201209102630.5562-3-naush@raspberrypi.com>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Thu, 10 Dec 2020 09:05:49 +0000","Message-ID":"<CAHW6GYL-xCkHLwRa30zRMYXxFL1MfOsjXuCu=q3nEK79-t2FhQ@mail.gmail.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/3] libcamera: raspberrypi: Add\n\tcontrol of sensor vblanking","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14185,"web_url":"https://patchwork.libcamera.org/comment/14185/","msgid":"<CAEmqJPpL8ZZB_k06WrhzA3-A6qh6Q=U4Cf7VgcNX56AU1fmRsg@mail.gmail.com>","date":"2020-12-10T09:32:54","subject":"Re: [libcamera-devel] [PATCH v4 2/3] libcamera: raspberrypi: Add\n\tcontrol of sensor vblanking","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for the review.\n\nOn Thu, 10 Dec 2020 at 09:06, David Plowman <david.plowman@raspberrypi.com>\nwrote:\n\n> Hi Naush\n>\n> Thanks for the patch. Just a couple of very minor nit-picks/questions!\n>\n> On Wed, 9 Dec 2020 at 10:26, Naushir Patuck <naush@raspberrypi.com> wrote:\n> >\n> > Add support for setting V4L2_CID_VBLANK appropriately when setting\n> > V4L2_CID_EXPOSURE. This will allow adaptive framerates during\n> > viewfinder use cases (e.g. when the exposure time goes above 33ms, we\n> > can reduce the framerate to lower than 30fps).\n> >\n> > The minimum and maximum frame durations are provided via libcamera\n> > controls, and will prioritise exposure time limits over any AGC request.\n> >\n> > V4L2_CID_VBLANK is controlled through the staggered writer, just like\n> > the exposure and gain controls.\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  include/libcamera/ipa/raspberrypi.h           |  1 +\n> >  src/ipa/raspberrypi/cam_helper.cpp            | 37 ++++++++++++++++-\n> >  src/ipa/raspberrypi/cam_helper.hpp            | 15 ++++++-\n> >  src/ipa/raspberrypi/cam_helper_imx219.cpp     | 13 +++++-\n> >  src/ipa/raspberrypi/cam_helper_imx477.cpp     | 11 ++++-\n> >  src/ipa/raspberrypi/cam_helper_ov5647.cpp     | 11 ++++-\n> >  src/ipa/raspberrypi/raspberrypi.cpp           | 41 ++++++++++++++++---\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n> >  8 files changed, 119 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.h\n> b/include/libcamera/ipa/raspberrypi.h\n> > index 01fe5abc..160ca681 100644\n> > --- a/include/libcamera/ipa/raspberrypi.h\n> > +++ b/include/libcamera/ipa/raspberrypi.h\n> > @@ -65,6 +65,7 @@ static const ControlInfoMap Controls = {\n> >         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> >         { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f)\n> },\n> >         { &controls::ScalerCrop, ControlInfo(Rectangle{},\n> Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > +       { &controls::FrameDurations, ControlInfo(1.0e3f, 1.0e9f) },\n>\n> Just for my curiosity, any specific reasons for these limits?\n>\n\nThese limits are somewhat arbitrary (0.001 to 1000fps).  Not really sure\nwhat sensible limits might actually be to cover every possible use case.\nPerhaps 1000fps is a bit optimistic?\n\n\n>\n> >  };\n> >\n> >  } /* namespace RPi */\n> > diff --git a/src/ipa/raspberrypi/cam_helper.cpp\n> b/src/ipa/raspberrypi/cam_helper.cpp\n> > index c8ac3232..03da127f 100644\n> > --- a/src/ipa/raspberrypi/cam_helper.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> > @@ -34,8 +34,10 @@ CamHelper *CamHelper::Create(std::string const\n> &cam_name)\n> >         return nullptr;\n> >  }\n> >\n> > -CamHelper::CamHelper(MdParser *parser)\n> > -       : parser_(parser), initialized_(false)\n> > +CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength,\n> > +                    unsigned int frameIntegrationDiff)\n> > +       : parser_(parser), initialized_(false),\n> maxFrameLength_(maxFrameLength),\n> > +         frameIntegrationDiff_(frameIntegrationDiff)\n> >  {\n> >  }\n> >\n> > @@ -56,6 +58,37 @@ double CamHelper::Exposure(uint32_t exposure_lines)\n> const\n> >         return exposure_lines * mode_.line_length / 1000.0;\n> >  }\n> >\n> > +uint32_t CamHelper::GetVBlanking(double &exposure, double\n> minFrameDuration,\n> > +                                double maxFrameDuration) const\n> > +{\n> > +       uint32_t frameLengthMin, frameLengthMax, vblank;\n> > +       uint32_t exposureLines = ExposureLines(exposure);\n> > +\n> > +       assert(initialized_);\n> > +\n> > +       /*\n> > +        * Clip frame length by the frame duration range and the maximum\n> allowable\n> > +        * value in the sensor, given by maxFrameLength_.\n> > +        */\n> > +       frameLengthMin = std::clamp<uint32_t>(1e3 * minFrameDuration /\n> mode_.line_length,\n> > +                                             mode_.height,\n> maxFrameLength_);\n>\n> (note 1 - see below)\n>\n> > +       frameLengthMax = std::clamp<uint32_t>(1e3 * maxFrameDuration /\n> mode_.line_length,\n> > +                                             mode_.height,\n> maxFrameLength_);\n> > +       /*\n> > +        * Limit the exposure to the maximum frame duration requested,\n> and\n> > +        * re-calculate if it has been clipped.\n> > +        */\n> > +       exposureLines = std::min(frameLengthMax - frameIntegrationDiff_,\n> exposureLines);\n> > +       exposure = Exposure(exposureLines);\n> > +\n> > +       /* Limit the vblank to the range allowed by the frame length\n> limits. */\n> > +       vblank = std::max<uint32_t>(exposureLines +\n> frameIntegrationDiff_, mode_.height);\n>\n> (note 2 - see below)\n>\n> > +       vblank = std::clamp(vblank - mode_.height,\n> > +                           frameLengthMin - mode_.height,\n> frameLengthMax - mode_.height);\n>\n> I did just wonder about rewriting this last line as\n>\n> vblank = std::clamp(vblank, frameLengthMin, frameLengthMax) - mode.height;\n>\n> That also makes it more obvious to me that the line identified by note\n> 2 is to some extent redundant, because the line identified by note 1\n> assures frameLengthMin >= mode_.height already.\n>\n> So maybe these last 2 lines would be clearer as:\n>\n> vblank = std::clamp(exposureLines + frameIntegrationDiff,\n> frameLengthMin, frameLengthMax) - mode.height;\n>\n> Did I get that right? Anyway, all small stuff!\n>\n\nYes, I think you might be right.  frameLengthMin/Max will be >= mode.height\nso the last 2 statements can be simplified by your suggestion.  Will make\nthat change.\n\nRegards,\nNaush\n\n\n\n>\n> > +\n> > +       return vblank;\n> > +}\n> > +\n> >  void CamHelper::SetCameraMode(const CameraMode &mode)\n> >  {\n> >         mode_ = mode;\n> > diff --git a/src/ipa/raspberrypi/cam_helper.hpp\n> b/src/ipa/raspberrypi/cam_helper.hpp\n> > index 044c2866..b1739a57 100644\n> > --- a/src/ipa/raspberrypi/cam_helper.hpp\n> > +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> > @@ -62,12 +62,15 @@ class CamHelper\n> >  {\n> >  public:\n> >         static CamHelper *Create(std::string const &cam_name);\n> > -       CamHelper(MdParser *parser);\n> > +       CamHelper(MdParser *parser, unsigned int maxFrameLength,\n> > +                 unsigned int frameIntegrationDiff);\n> >         virtual ~CamHelper();\n> >         void SetCameraMode(const CameraMode &mode);\n> >         MdParser &Parser() const { return *parser_; }\n> >         uint32_t ExposureLines(double exposure_us) const;\n> >         double Exposure(uint32_t exposure_lines) const; // in us\n> > +       virtual uint32_t GetVBlanking(double &exposure_us, double\n> minFrameDuration,\n> > +                                     double maxFrameDuration) const;\n> >         virtual uint32_t GainCode(double gain) const = 0;\n> >         virtual double Gain(uint32_t gain_code) const = 0;\n> >         virtual void GetDelays(int &exposure_delay, int &gain_delay)\n> const;\n> > @@ -76,10 +79,20 @@ public:\n> >         virtual unsigned int HideFramesModeSwitch() const;\n> >         virtual unsigned int MistrustFramesStartup() const;\n> >         virtual unsigned int MistrustFramesModeSwitch() const;\n> > +\n> >  protected:\n> >         MdParser *parser_;\n> >         CameraMode mode_;\n> > +\n> > +private:\n> >         bool initialized_;\n> > +       /* Largest possible frame length, in units of lines. */\n> > +       unsigned int maxFrameLength_;\n> > +       /*\n> > +        * Smallest difference between the frame length and integration\n> time,\n> > +        * in units of lines.\n> > +        */\n> > +       unsigned int frameIntegrationDiff_;\n> >  };\n> >\n> >  // This is for registering camera helpers with the system, so that the\n> > diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > index db8ab879..8688ec09 100644\n> > --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > @@ -49,13 +49,22 @@ public:\n> >         double Gain(uint32_t gain_code) const override;\n> >         unsigned int MistrustFramesModeSwitch() const override;\n> >         bool SensorEmbeddedDataPresent() const override;\n> > +\n> > +private:\n> > +       /*\n> > +        * Smallest difference between the frame length and integration\n> time,\n> > +        * in units of lines.\n> > +        */\n> > +       static constexpr int frameIntegrationDiff = 4;\n> > +       /* Largest possible frame length, in units of lines. */\n> > +       static constexpr int maxFrameLength = 0xffff;\n> >  };\n> >\n> >  CamHelperImx219::CamHelperImx219()\n> >  #if ENABLE_EMBEDDED_DATA\n> > -       : CamHelper(new MdParserImx219())\n> > +       : CamHelper(new MdParserImx219(), maxFrameLength,\n> frameIntegrationDiff)\n> >  #else\n> > -       : CamHelper(new MdParserRPi())\n> > +       : CamHelper(new MdParserRPi(), maxFrameLength,\n> frameIntegrationDiff)\n> >  #endif\n> >  {\n> >  }\n> > diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > index 0e896ac7..53961310 100644\n> > --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > @@ -38,10 +38,19 @@ public:\n> >         uint32_t GainCode(double gain) const override;\n> >         double Gain(uint32_t gain_code) const override;\n> >         bool SensorEmbeddedDataPresent() const override;\n> > +\n> > +private:\n> > +       /*\n> > +        * Smallest difference between the frame length and integration\n> time,\n> > +        * in units of lines.\n> > +        */\n> > +       static constexpr int frameIntegrationDiff = 22;\n> > +       /* Largest possible frame length, in units of lines. */\n> > +       static constexpr int maxFrameLength = 0xffdc;\n> >  };\n> >\n> >  CamHelperImx477::CamHelperImx477()\n> > -       : CamHelper(new MdParserImx477())\n> > +       : CamHelper(new MdParserImx477(), maxFrameLength,\n> frameIntegrationDiff)\n> >  {\n> >  }\n> >\n> > diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > index dc5d8275..7630c127 100644\n> > --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > @@ -22,6 +22,15 @@ public:\n> >         unsigned int HideFramesModeSwitch() const override;\n> >         unsigned int MistrustFramesStartup() const override;\n> >         unsigned int MistrustFramesModeSwitch() const override;\n> > +\n> > +private:\n> > +       /*\n> > +        * Smallest difference between the frame length and integration\n> time,\n> > +        * in units of lines.\n> > +        */\n> > +       static constexpr int frameIntegrationDiff = 4;\n> > +       /* Largest possible frame length, in units of lines. */\n> > +       static constexpr int maxFrameLength = 0xffff;\n> >  };\n> >\n> >  /*\n> > @@ -30,7 +39,7 @@ public:\n> >   */\n> >\n> >  CamHelperOv5647::CamHelperOv5647()\n> > -       : CamHelper(new MdParserRPi())\n> > +       : CamHelper(new MdParserRPi(), maxFrameLength,\n> frameIntegrationDiff)\n> >  {\n> >  }\n> >\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 60cfdc27..4e7c2b94 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -58,6 +58,8 @@ namespace libcamera {\n> >  /* Configure the sensor with these values initially. */\n> >  constexpr double DefaultAnalogueGain = 1.0;\n> >  constexpr unsigned int DefaultExposureTime = 20000;\n> > +constexpr double defaultMinFrameDuration = 1e6 / 30.0;\n> > +constexpr double defaultMaxFrameDuration = 1e6 / 0.01;\n> >\n> >  LOG_DEFINE_CATEGORY(IPARPI)\n> >\n> > @@ -145,6 +147,9 @@ private:\n> >         /* LS table allocation passed in from the pipeline handler. */\n> >         FileDescriptor lsTableHandle_;\n> >         void *lsTable_;\n> > +\n> > +       /* Frame duration (1/fps) given in microseconds. */\n> > +       double minFrameDuration_, maxFrameDuration_;\n> >  };\n> >\n> >  int IPARPi::init(const IPASettings &settings)\n> > @@ -266,7 +271,8 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> >                 sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> >\n> >                 result->data.push_back(gainDelay);\n> > -               result->data.push_back(exposureDelay);\n> > +               result->data.push_back(exposureDelay); /* FOR EXPOSURE\n> ctrl */\n> > +               result->data.push_back(exposureDelay); /* For VBLANK\n> ctrl */\n> >                 result->data.push_back(sensorMetadata);\n> >\n> >                 result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;\n> > @@ -335,6 +341,8 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> >                 AgcStatus agcStatus;\n> >                 agcStatus.shutter_time = DefaultExposureTime;\n> >                 agcStatus.analogue_gain = DefaultAnalogueGain;\n> > +               minFrameDuration_ = defaultMinFrameDuration;\n> > +               maxFrameDuration_ = defaultMaxFrameDuration;\n> >                 applyAGC(&agcStatus, ctrls);\n> >\n> >                 result->controls.emplace_back(ctrls);\n> > @@ -712,6 +720,17 @@ void IPARPi::queueRequest(const ControlList\n> &controls)\n> >                         break;\n> >                 }\n> >\n> > +               case controls::FRAME_DURATIONS: {\n> > +                       auto frameDurations = ctrl.second.get<Span<const\n> float>>();\n> > +\n> > +                       /* This will be applied once AGC recalculations\n> occur. */\n> > +                       minFrameDuration_ = frameDurations[0];\n> > +                       maxFrameDuration_ = frameDurations[1];\n> > +                       libcameraMetadata_.set(controls::FrameDurations,\n> > +                                              { frameDurations[0],\n> frameDurations[1] });\n>\n> Would \"libcameraMetadata_.set(controls::FrameDurations,\n> frameDurations)\" work? Just a teeny bit tider!\n>\n> My other general comment is that the subsequent patch you sent me,\n> about getting the vblank setting to happen first, seems to me like it\n> will be important. But we can do that as a subsequent patch, right?\n>\n> I also think we'll want to pursue the matter about getting the max\n> exposure into the AGC, but that's not critical-path right now!\n>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> Tested-by: David Plowman <david.plowman@raspberrypi.com>\n>\n> Thanks again and best regards\n> David\n>\n> > +                       break;\n> > +               }\n> > +\n> >                 default:\n> >                         LOG(IPARPI, Warning)\n> >                                 << \"Ctrl \" << controls::controls.at\n> (ctrl.first)->name()\n> > @@ -882,7 +901,12 @@ void IPARPi::applyAWB(const struct AwbStatus\n> *awbStatus, ControlList &ctrls)\n> >  void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList\n> &ctrls)\n> >  {\n> >         int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n> > -       int32_t exposureLines =\n> helper_->ExposureLines(agcStatus->shutter_time);\n> > +\n> > +       /* GetVBlanking might clip exposure time to the fps limits. */\n> > +       double exposure = agcStatus->shutter_time;\n> > +       int32_t vblanking = helper_->GetVBlanking(exposure,\n> minFrameDuration_,\n> > +                                                 maxFrameDuration_);\n> > +       int32_t exposureLines = helper_->ExposureLines(exposure);\n> >\n> >         if (unicamCtrls_.find(V4L2_CID_ANALOGUE_GAIN) ==\n> unicamCtrls_.end()) {\n> >                 LOG(IPARPI, Error) << \"Can't find analogue gain control\";\n> > @@ -894,13 +918,20 @@ void IPARPi::applyAGC(const struct AgcStatus\n> *agcStatus, ControlList &ctrls)\n> >                 return;\n> >         }\n> >\n> > -       LOG(IPARPI, Debug) << \"Applying AGC Exposure: \" <<\n> agcStatus->shutter_time\n> > -                          << \" (Shutter lines: \" << exposureLines << \")\n> Gain: \"\n> > +       LOG(IPARPI, Debug) << \"Applying AGC Exposure: \" << exposure\n> > +                          << \" (Shutter lines: \" << exposureLines << \",\n> AGC requested \"\n> > +                          << agcStatus->shutter_time << \") Gain: \"\n> >                            << agcStatus->analogue_gain << \" (Gain Code: \"\n> >                            << gainCode << \")\";\n> >\n> > -       ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);\n> > +       /*\n> > +        * Due to the behavior of V4L2, the current value of VBLANK\n> could clip the\n> > +        * exposure time without us knowing. The next time though this\n> function should\n> > +        * clip exposure correctly.\n> > +        */\n> > +       ctrls.set(V4L2_CID_VBLANK, vblanking);\n> >         ctrls.set(V4L2_CID_EXPOSURE, exposureLines);\n> > +       ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);\n> >  }\n> >\n> >  void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList\n> &ctrls)\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 2ec1f6e7..13349f31 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1221,7 +1221,8 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> >                 if (!staggeredCtrl_) {\n> >                         staggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n> >                                             { { V4L2_CID_ANALOGUE_GAIN,\n> result.data[resultIdx++] },\n> > -                                             { V4L2_CID_EXPOSURE,\n> result.data[resultIdx++] } });\n> > +                                             { V4L2_CID_EXPOSURE,\n> result.data[resultIdx++] },\n> > +                                             { V4L2_CID_VBLANK,\n> result.data[resultIdx++] } });\n> >                         sensorMetadata_ = result.data[resultIdx++];\n> >                 }\n> >         }\n> > --\n> > 2.25.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E9DF8BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Dec 2020 09:33:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7876B67F72;\n\tThu, 10 Dec 2020 10:33:12 +0100 (CET)","from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F75060323\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 10:33:11 +0100 (CET)","by mail-lj1-x22e.google.com with SMTP id f11so5915660ljm.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 01:33:11 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Ha5BdCNr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=4gmaRMkUhdyCGs0kHnZbUBwdO4ADkqmNrEAGp06PggI=;\n\tb=Ha5BdCNrCEnJgcna2DACUHOL8uRNLG1YUrDE0FJI+Arxkpwq5c7yUS4GWhOmileJt+\n\tbAzHC1JPlrNRY7o9Up4Kkf7o1M/+YxhUyVWQIbGC6P6j0qb8iGyQDr/1tq7oC/AL33kq\n\tUq6FMtgERukOcRflNFCYpjv9Xlcrb2wXkrhqsdQIRhx67HsttN3A4oKiQFsnFnqaUmsl\n\t7kkMQqOKtxojR1MMD1by+D5olqo5hGaS5UyDT8qoHeB0HCbX5qAjzxJJgXkZOxNb20ow\n\tIce4mZRji9PwdYnuBAQR2xayO5oGyIduThNxcKCz4Nht8N+eRKUcn0zlTaPjKw4H4kgt\n\tzX4w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=4gmaRMkUhdyCGs0kHnZbUBwdO4ADkqmNrEAGp06PggI=;\n\tb=JeGYljCyWNhXSvASxlxEZtzeNeSRquYKeAjo7COrljPARr1l+lS2pgLg2csy5gZe0H\n\tqx7h69Ar8wiW85MGnH4lNFSsKaWOJYqLFmShpcdqszHYhLAng3LQeKEp3evIr9Cpzxkq\n\t9SWQjR50t/2RKCnsREr/+76pIG2AdYs+6jSc9LzvT3FG0DD5eE2l9PM5SdNU7zZW+Eh6\n\tOP5dxI9drD/6aZrgu5GFoBMr0MM5ylfB/qfmenz6AurGPzvaaOp4aIKeAcucW7+RJCOe\n\tWhqm78CdoD5jSI0SrRccBwaReGOY0dcmmUZMyIiee5ES9VfVITN/DaymZpquYvbqX+0D\n\tgRwA==","X-Gm-Message-State":"AOAM532vs4aaOE7RDTHz8wEQHVJYrASdlA7NOpo3ZGCifQa7R7ghFpGD\n\ty457IwXVNAURjvC4nFowZsUrClh4Ht8lGWgXjJKJmg==","X-Google-Smtp-Source":"ABdhPJyC59t86+yUVYf9B2ZCXMCgKK4WsxPh4UWxOlBbb4Y/yF1dJ9JJCD84etvjSRKYs7DRRrULh2W3bmA/Kms1Z6Y=","X-Received":"by 2002:a05:651c:503:: with SMTP id\n\to3mr2510511ljp.253.1607592790529; \n\tThu, 10 Dec 2020 01:33:10 -0800 (PST)","MIME-Version":"1.0","References":"<20201209102630.5562-1-naush@raspberrypi.com>\n\t<20201209102630.5562-3-naush@raspberrypi.com>\n\t<CAHW6GYL-xCkHLwRa30zRMYXxFL1MfOsjXuCu=q3nEK79-t2FhQ@mail.gmail.com>","In-Reply-To":"<CAHW6GYL-xCkHLwRa30zRMYXxFL1MfOsjXuCu=q3nEK79-t2FhQ@mail.gmail.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Thu, 10 Dec 2020 09:32:54 +0000","Message-ID":"<CAEmqJPpL8ZZB_k06WrhzA3-A6qh6Q=U4Cf7VgcNX56AU1fmRsg@mail.gmail.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/3] libcamera: raspberrypi: Add\n\tcontrol of sensor vblanking","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"multipart/mixed;\n\tboundary=\"===============5119656476071601938==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]