[{"id":14194,"web_url":"https://patchwork.libcamera.org/comment/14194/","msgid":"<20201210144811.oz3gziiqtswurvdd@uno.localdomain>","date":"2020-12-10T14:48:11","subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush,\n   thanks for the update\n\nOn Thu, Dec 10, 2020 at 01:34:24PM +0000, Naushir Patuck wrote:\n> Add a float array control (controls::FrameDurations) to specify the\n> minimum and maximum (in that order) frame duration to be used by the\n> camera sensor.\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> Tested-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  src/libcamera/control_ids.yaml | 15 +++++++++++++++\n>  1 file changed, 15 insertions(+)\n>\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index 6d6f0fee..7f1f8624 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -554,4 +554,19 @@ controls:\n>          detection, additional format conversions etc) count as an additional\n>          pipeline stage.\n\nSo, I've gone through the lentghy discussions on v2 and v3.\n\nTo be very honest, I think we still have some missing pieces and the\none that concerns me the more is the interaction of this control with\nthe selected AE mode and the consequences on exposure/shutter-time\npriorites. I see all controls about Exposure have their interaction\ndefinition defferred to a \\todo item. This is of course not the ideal\nsituation but adding one make the issue only slightly worse. Deferring\nthese to the pipeline model definition might be an option.\n\nI'm willing to ack this patch, but I think there are a few details I\nwould like to discuss:\n\n- Clipping. We need a (per-configuration, like the ScalerCrop\n  rectangle) property to provide application limits and refer to it in\n  this control description. I'll address this on top, but I would\n  apreciate a:\n\n  \\todo Refer to the frame duration limits property to describe how\n  application-provided values gets clipped or how to reset the control\n  value to its default.\n\n- This is both a control and a metadata. I think the\n  description is only about 'setting' the FrameDurations, not reading\n  it. We have discussed in length the former but somewhat ignored the\n  latter.\n\n  The 'reading' case could addressed by introducing a read-only control\n  (ie FrameDuration) only to be used as metadata. But this might even be\n  more confusing as people will wonder why they have to use\n  'Duration-s-' when they want a precise value and theres a 'Duration'\n  (without 's') available. I'll propose an additional section but if\n  you have ideas please suggest them. Also, feel free to leave this\n  last part out if it turns out to be controversial and would delay the\n  series any longer.\n\n>\n> +  - FrameDurations:\n> +      type: float\n\nI understand this is meant to accommodate standard FPS like 29,97 FPS\n(-.-) but won't expressing this as nanoseconds with a uin64_t\nrepresentation be capable of achieving the same. Floating point\narithmentic is generally a bit harder and clunky to handle when doing\ncalculations. I won't push if you think a float is better.\n\n> +      description: |\n\nI would:\n        size: [2]\n        description:\n          The minimum and maximum (in that order) frame duration,\n          expressed in micro-seconds.\n\n          When provided by applications the control specifies the\n          sensor frame duration interval the pipeline has to use. This\n          could also limit the largest exposure times the sensor can\n          use. For example, if a maximum frame duration of 33ms is\n          requested (corresponding to 30 frames per second), the\n          sensor will not be able to raise the exposure time above\n          33ms. A fixed frame duration is achieved by setting the\n          minimum and maximum values to be the same.\n\n          \\todo Refer to the frame duration limits property to describe how\n          application-provided values gets clipped or how to reset the control\n          values to their defaults.\n\n          \\todo Better specify how the frame durations interact with the\n          exposure control algorithm.\n          \\sa AeEnable\n          \\sa AeExposureMode\n          \\sa ExposureTime\n\n          -----------------8< you can cut here 8<--------------------\n\n          When reported by pipelines the control expresses the duration\n          of the sensor frame used to produce streams part of the completed\n          Request. The minimum and maximum values shall then be the same, as the\n          sensor frame duration is a fixed parameter. The sensor frame\n          duration is one of the parameter that defines the capture\n          frame rate but it does not alone provide enough information\n          to fully calculate it as it does not account for pipeline\n          processing delays.\n\n          \\todo Define how to calculate the capture frame rate by\n          defining controls to report additional delays introduced by\n          the capture pipeline or post-processing stages (ie JPEG\n          conversion, frame scaling).\n\n> +        Specifies the minimum and maximum (in that order) allowable frame\n> +        duration, in micro-seconds, for the sensor to use. This could also limit\n> +        the largest exposure times the sensor can use. For example, if a maximum\n> +        frame duration of 33ms is requested (corresponding to 30 frames per\n> +        second), the sensor will not be able raise the exposure time above 33ms.\n\ns/will not be able raise/will not be able to raise/\nFixed in the above suggestions.\n\nThanks\n  j\n\n> +        A fixed frame duration is achieved by setting the minimum and maximum\n> +        values to be the same. Note that the sensor may not always be able to\n> +        provide the requested frame duration limits depending on its mode\n> +        configuration.\n\n> +        \\sa ExposureTime\n> +      size: [2]\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 AA0C8BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Dec 2020 14:48:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 37E8C67F72;\n\tThu, 10 Dec 2020 15:48:06 +0100 (CET)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A64567E4D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 15:48:04 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id AA3521BF21B;\n\tThu, 10 Dec 2020 14:48:01 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 10 Dec 2020 15:48:11 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20201210144811.oz3gziiqtswurvdd@uno.localdomain>","References":"<20201210133426.206679-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201210133426.206679-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","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@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":14195,"web_url":"https://patchwork.libcamera.org/comment/14195/","msgid":"<CAEmqJPqk8CfAranY+VbrK6ZmUa2y7_L1GKxZRCSvUy12_E5YhA@mail.gmail.com>","date":"2020-12-10T15:17:49","subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThank you for your review comments.\n\nOn Thu, 10 Dec 2020 at 14:48, Jacopo Mondi <jacopo@jmondi.org> wrote:\n\n> Hi Naush,\n>    thanks for the update\n>\n> On Thu, Dec 10, 2020 at 01:34:24PM +0000, Naushir Patuck wrote:\n> > Add a float array control (controls::FrameDurations) to specify the\n> > minimum and maximum (in that order) frame duration to be used by the\n> > camera sensor.\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > Tested-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  src/libcamera/control_ids.yaml | 15 +++++++++++++++\n> >  1 file changed, 15 insertions(+)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml\n> b/src/libcamera/control_ids.yaml\n> > index 6d6f0fee..7f1f8624 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -554,4 +554,19 @@ controls:\n> >          detection, additional format conversions etc) count as an\n> additional\n> >          pipeline stage.\n>\n> So, I've gone through the lentghy discussions on v2 and v3.\n>\n> To be very honest, I think we still have some missing pieces and the\n> one that concerns me the more is the interaction of this control with\n> the selected AE mode and the consequences on exposure/shutter-time\n> priorites. I see all controls about Exposure have their interaction\n> definition defferred to a \\todo item. This is of course not the ideal\n> situation but adding one make the issue only slightly worse. Deferring\n> these to the pipeline model definition might be an option.\n>\n\nYou are entirely correct here on all ponts.  There is a tightly coupled\ninteraction with this FrameDurations control and interaction with the AGC\nalgorithm.  You have also thrown a spanner in the works for my plan :)\nDavid and I have been working on exactly this and we think we have\naddressed all these interactions.  I have a set of patches that are ready\nto be pushed as a \"phase 2\", but wanted to get this through first so as not\nto muddy the waters too much.  This set of \"phase 2\" patches address the\nfollowing:\n\n1) Set VBLANK ahead of any other controls.  This avoids the problem of\nsetting EXPOSURE and having v4l2 reject the value due to stale limits.\n2) Limit VBLANK (and hence frame duration) to what the sensor mode can\nactually support.  This forms part of your Clipping discussion below.\n3) Add an interaction with AE when FrameDurations.  This addresses your\nconcerns above, with AE knowing exactly what limits of shutter speed are\nachievable and working around any limitations based on exposure modes\nselected.\n\nPerhaps I should make those changes as part of this series so you get the\nfull picture right now?\n\n\n>\n> I'm willing to ack this patch, but I think there are a few details I\n> would like to discuss:\n>\n> - Clipping. We need a (per-configuration, like the ScalerCrop\n>   rectangle) property to provide application limits and refer to it in\n>   this control description. I'll address this on top, but I would\n>   apreciate a:\n>\n>   \\todo Refer to the frame duration limits property to describe how\n>   application-provided values gets clipped or how to reset the control\n>   value to its default.\n>\n\nThis is partially addressed in my point (2) above.  However, I did not add\na property, rather just did clipping based on VBLANK limits to the IPA.  So\ndo you think we should have a \"properties::SensorMaxFramerate\" or similar\nthat provides the application the maximum possible framerate for this mode\n(derived from VBLANK control limits)?  As mentioned above, right now I\nsimply clip the user request to what the sensor mode can achieve.\n\n\n>\n> - This is both a control and a metadata. I think the\n>   description is only about 'setting' the FrameDurations, not reading\n>   it. We have discussed in length the former but somewhat ignored the\n>   latter.\n>\n>   The 'reading' case could addressed by introducing a read-only control\n>   (ie FrameDuration) only to be used as metadata. But this might even be\n>   more confusing as people will wonder why they have to use\n>   'Duration-s-' when they want a precise value and theres a 'Duration'\n>   (without 's') available. I'll propose an additional section but if\n>   you have ideas please suggest them. Also, feel free to leave this\n>   last part out if it turns out to be controversial and would delay the\n>   series any longer.\n>\n\nYes, good point here.  Returning a \"FrameDuration\" may be a bit redundant,\nas this information is conveyed in the FrameBuffer timestamps.  What do\nother folks think?\n\n\n>\n> >\n> > +  - FrameDurations:\n> > +      type: float\n>\n> I understand this is meant to accommodate standard FPS like 29,97 FPS\n> (-.-) but won't expressing this as nanoseconds with a uin64_t\n> representation be capable of achieving the same. Floating point\n> arithmentic is generally a bit harder and clunky to handle when doing\n> calculations. I won't push if you think a float is better.\n>\n\nI only used float here as that is what we use in our IPA :) Happy to change\nto int64_t based numbers.\n\n>\n> > +      description: |\n>\n> I would:\n>         size: [2]\n>         description:\n>           The minimum and maximum (in that order) frame duration,\n>           expressed in micro-seconds.\n>\n>           When provided by applications the control specifies the\n>           sensor frame duration interval the pipeline has to use. This\n>           could also limit the largest exposure times the sensor can\n>           use. For example, if a maximum frame duration of 33ms is\n>           requested (corresponding to 30 frames per second), the\n>           sensor will not be able to raise the exposure time above\n>           33ms. A fixed frame duration is achieved by setting the\n>           minimum and maximum values to be the same.\n>\n>           \\todo Refer to the frame duration limits property to describe how\n>           application-provided values gets clipped or how to reset the\n> control\n>           values to their defaults.\n>\n>           \\todo Better specify how the frame durations interact with the\n>           exposure control algorithm.\n>           \\sa AeEnable\n>           \\sa AeExposureMode\n>           \\sa ExposureTime\n>\n>           -----------------8< you can cut here 8<--------------------\n>\n>           When reported by pipelines the control expresses the duration\n>           of the sensor frame used to produce streams part of the completed\n>           Request. The minimum and maximum values shall then be the same,\n> as the\n>           sensor frame duration is a fixed parameter. The sensor frame\n>           duration is one of the parameter that defines the capture\n>           frame rate but it does not alone provide enough information\n>           to fully calculate it as it does not account for pipeline\n>           processing delays.\n>\n>           \\todo Define how to calculate the capture frame rate by\n>           defining controls to report additional delays introduced by\n>           the capture pipeline or post-processing stages (ie JPEG\n>           conversion, frame scaling).\n>\n\nSure, no problem.  I will use the above wording and perhaps expand a bit\nmore on the AE interactions.\n\n\n>\n> > +        Specifies the minimum and maximum (in that order) allowable\n> frame\n> > +        duration, in micro-seconds, for the sensor to use. This could\n> also limit\n> > +        the largest exposure times the sensor can use. For example, if\n> a maximum\n> > +        frame duration of 33ms is requested (corresponding to 30 frames\n> per\n> > +        second), the sensor will not be able raise the exposure time\n> above 33ms.\n>\n> s/will not be able raise/will not be able to raise/\n> Fixed in the above suggestions.\n>\n\nAck.\n\nRegards,\nNaush\n\n\n>\n> Thanks\n>   j\n\n\n> > +        A fixed frame duration is achieved by setting the minimum and\n> maximum\n> > +        values to be the same. Note that the sensor may not always be\n> able to\n> > +        provide the requested frame duration limits depending on its\n> mode\n> > +        configuration.\n>\n> > +        \\sa ExposureTime\n> > +      size: [2]\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 3A565BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Dec 2020 15:18:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BDB3467F73;\n\tThu, 10 Dec 2020 16:18:06 +0100 (CET)","from mail-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE6D867E4D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 16:18:05 +0100 (CET)","by mail-lf1-x131.google.com with SMTP id l11so8767039lfg.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 07:18:05 -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=\"a9LNsUyu\"; 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=E+OSHh0DkQOQA2+4feDUzEbdhJtbGn7k+Kh7534JHgw=;\n\tb=a9LNsUyucmSDlj+ODgUZu++y7KyQMHZkj0cRf7hq7Qon2LTC3pBZwiEEnV8+6kgGPi\n\t4nNFbfi+wYqFlecYbwErAyv9irRix3IhP8W0vWupZTKYYLb8dKZ/E/iT/XWP77hD3oIW\n\t9aacGDXBY0anYLwSF4lB5sQ35t6QNy9MbnSVqN6jHySKw33LQB2Ax5iyjr1sitHGuSLk\n\tLB11so8OkzdpgODR3q9BSKgc2LgL4vhVsUgJdt1djxPtSMJXuXcJZIBKFQz5NVnXYywY\n\tXH0ApNlXGdrq4T5DHxC7Nsy1TAw+qaaDagvkbiHutf4ZUEbAK28A6uMoQqCP9PEShDVj\n\tWMdA==","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=E+OSHh0DkQOQA2+4feDUzEbdhJtbGn7k+Kh7534JHgw=;\n\tb=YVmH4E4cKtb23UNDD+n5Beo1U64p9tWmWnpSCcSqzw0XOFLsxNchLRxR4/eY/GidaY\n\tTxdr+LC4F6U7mwvIINOnsCSit3PaUtb02JsrGVvtObeM7VAgwZvD2tahKVSyKYEBO1Oh\n\txs/TkI14nNTCpgwIpE+5LcuhZNcRUPicHjHIX4AvD7cgNWIAOHwX/DmK9ttjcXHoM+vo\n\tNNYWutHfIZk2FxJGVM8u1XXVxESU6U0DJUHcyodxyCBUMAX0GtfXdQucZdXGy8KLkjr/\n\tPC7tv03wz3P9ryJcU3Ia1lTC/CG2OIrQPyMiWQk3U+efoTKkfgj/v7LQZ+h2/9wL+46m\n\t/UYw==","X-Gm-Message-State":"AOAM531AN+fAVr9zn9uuBmvn7JzDXXOSwbDpwkdu6xV+sZkQzdQosdXe\n\ts0Fzd/o9fCaRkAIqtW9tpu6uzk6VY+in8HgXyoC5UA==","X-Google-Smtp-Source":"ABdhPJxDPlMkDCab9aOnGs/7efA11kJb10Jq+yXtsNZAPr1QMSKALk0SULD3sjYpKYfo/XoXvV1kU7nQgm0VJvNtoNo=","X-Received":"by 2002:a19:8357:: with SMTP id\n\tf84mr2906822lfd.567.1607613485056; \n\tThu, 10 Dec 2020 07:18:05 -0800 (PST)","MIME-Version":"1.0","References":"<20201210133426.206679-1-naush@raspberrypi.com>\n\t<20201210144811.oz3gziiqtswurvdd@uno.localdomain>","In-Reply-To":"<20201210144811.oz3gziiqtswurvdd@uno.localdomain>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Thu, 10 Dec 2020 15:17:49 +0000","Message-ID":"<CAEmqJPqk8CfAranY+VbrK6ZmUa2y7_L1GKxZRCSvUy12_E5YhA@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","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=\"===============1952041071321545255==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14196,"web_url":"https://patchwork.libcamera.org/comment/14196/","msgid":"<20201210160415.qgp7dcwail4jmjx7@uno.localdomain>","date":"2020-12-10T16:04:15","subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush,\n\nOn Thu, Dec 10, 2020 at 03:17:49PM +0000, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> Thank you for your review comments.\n>\n> On Thu, 10 Dec 2020 at 14:48, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> > Hi Naush,\n> >    thanks for the update\n> >\n> > On Thu, Dec 10, 2020 at 01:34:24PM +0000, Naushir Patuck wrote:\n> > > Add a float array control (controls::FrameDurations) to specify the\n> > > minimum and maximum (in that order) frame duration to be used by the\n> > > camera sensor.\n> > >\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > Tested-by: David Plowman <david.plowman@raspberrypi.com>\n> > > ---\n> > >  src/libcamera/control_ids.yaml | 15 +++++++++++++++\n> > >  1 file changed, 15 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/control_ids.yaml\n> > b/src/libcamera/control_ids.yaml\n> > > index 6d6f0fee..7f1f8624 100644\n> > > --- a/src/libcamera/control_ids.yaml\n> > > +++ b/src/libcamera/control_ids.yaml\n> > > @@ -554,4 +554,19 @@ controls:\n> > >          detection, additional format conversions etc) count as an\n> > additional\n> > >          pipeline stage.\n> >\n> > So, I've gone through the lentghy discussions on v2 and v3.\n> >\n> > To be very honest, I think we still have some missing pieces and the\n> > one that concerns me the more is the interaction of this control with\n> > the selected AE mode and the consequences on exposure/shutter-time\n> > priorites. I see all controls about Exposure have their interaction\n> > definition defferred to a \\todo item. This is of course not the ideal\n> > situation but adding one make the issue only slightly worse. Deferring\n> > these to the pipeline model definition might be an option.\n> >\n>\n> You are entirely correct here on all ponts.  There is a tightly coupled\n> interaction with this FrameDurations control and interaction with the AGC\n> algorithm.  You have also thrown a spanner in the works for my plan :)\n> David and I have been working on exactly this and we think we have\n> addressed all these interactions.  I have a set of patches that are ready\n> to be pushed as a \"phase 2\", but wanted to get this through first so as not\n> to muddy the waters too much.  This set of \"phase 2\" patches address the\n> following:\n\nAh great :)\n\n>\n> 1) Set VBLANK ahead of any other controls.  This avoids the problem of\n> setting EXPOSURE and having v4l2 reject the value due to stale limits.\n> 2) Limit VBLANK (and hence frame duration) to what the sensor mode can\n> actually support.  This forms part of your Clipping discussion below.\n> 3) Add an interaction with AE when FrameDurations.  This addresses your\n> concerns above, with AE knowing exactly what limits of shutter speed are\n> achievable and working around any limitations based on exposure modes\n> selected.\n\nThese are all improvements to the RPi implementation which are\ncertainly positive, but if I got these right, it's mostly about\nimplementation.\n\nUsing your point 3) as example, my main concern is more where/how\nto specify how pipelines behaves (in the control documentation, in the\npipeline handler model etc), or if we want all pipelines to behave the\nsame, which I'm not even sure it's possible or desirable to enforce.\n\nDid I get you right and your IPA will:\n1) Select the appropriate exposure time using the hint provided by\n   AeExposureMode\n2) Clip the selected interval in the FrameDurations limits\n\nThis seems more than reasonable but then the first question I have is\nhow to make this consistent for application among different\nplatforms/IPA implementations and how to clearly document that.\n\nThis additional controls does not define that, and I think it's fair.\nWhat we should avoid is going in a direction that makes it too hard to\nbacktrack, and I don't think there's anything that bad here.\n\nWe just need more IPAs, to have a large enough base to prove these\nconcepts against a different implementation I guess.\n\n>\n> Perhaps I should make those changes as part of this series so you get the\n> full picture right now?\n\nThe risk is to delay this more. I would do that on top if I got you\nright and these are mostly improvements specific to the RPi\nimplementation.\n\n>\n>\n> >\n> > I'm willing to ack this patch, but I think there are a few details I\n> > would like to discuss:\n> >\n> > - Clipping. We need a (per-configuration, like the ScalerCrop\n> >   rectangle) property to provide application limits and refer to it in\n> >   this control description. I'll address this on top, but I would\n> >   apreciate a:\n> >\n> >   \\todo Refer to the frame duration limits property to describe how\n> >   application-provided values gets clipped or how to reset the control\n> >   value to its default.\n> >\n>\n> This is partially addressed in my point (2) above.  However, I did not add\n> a property, rather just did clipping based on VBLANK limits to the IPA.  So\n> do you think we should have a \"properties::SensorMaxFramerate\" or similar\n> that provides the application the maximum possible framerate for this mode\n> (derived from VBLANK control limits)?  As mentioned above, right now I\n> simply clip the user request to what the sensor mode can achieve.\n\nAs an implementation it's fine, but I think we need a mechanism to allow\napplication to access that information. At least, I know we need this\nfor Android, so something has to be added.\n\nAnd I think once we have that property is fair to mention here that\nvalues outside the limits there reported will be clipped (and that's\nsomething I think we can assume for all pipelines).\n\nThen if you want to support this, you will simply have to update the\nCamera property to report the VBLANK limits.\n\n>\n>\n> >\n> > - This is both a control and a metadata. I think the\n> >   description is only about 'setting' the FrameDurations, not reading\n> >   it. We have discussed in length the former but somewhat ignored the\n> >   latter.\n> >\n> >   The 'reading' case could addressed by introducing a read-only control\n> >   (ie FrameDuration) only to be used as metadata. But this might even be\n> >   more confusing as people will wonder why they have to use\n> >   'Duration-s-' when they want a precise value and theres a 'Duration'\n> >   (without 's') available. I'll propose an additional section but if\n> >   you have ideas please suggest them. Also, feel free to leave this\n> >   last part out if it turns out to be controversial and would delay the\n> >   series any longer.\n> >\n>\n> Yes, good point here.  Returning a \"FrameDuration\" may be a bit redundant,\n> as this information is conveyed in the FrameBuffer timestamps.  What do\n> other folks think?\n\nThat's a good point. We have timestamps and durations can be\ncalculated. I still think it would be nicer for application to\nhave a direct way to access this as part of the Request metadata. Or\nwe can expose the sensor timestamp from the Request and let them calculate\ndurations..\n\n>\n>\n> >\n> > >\n> > > +  - FrameDurations:\n> > > +      type: float\n> >\n> > I understand this is meant to accommodate standard FPS like 29,97 FPS\n> > (-.-) but won't expressing this as nanoseconds with a uin64_t\n> > representation be capable of achieving the same. Floating point\n> > arithmentic is generally a bit harder and clunky to handle when doing\n> > calculations. I won't push if you think a float is better.\n> >\n>\n> I only used float here as that is what we use in our IPA :) Happy to change\n> to int64_t based numbers.\n>\n\nI would slightly prefer, but it's a gut feeling. I suspect you have a\nreasons why you used float in your IPA I am missing. If that's the\ncase feel free to keep float here.\n\n> >\n> > > +      description: |\n> >\n> > I would:\n> >         size: [2]\n> >         description:\n> >           The minimum and maximum (in that order) frame duration,\n> >           expressed in micro-seconds.\n> >\n> >           When provided by applications the control specifies the\n> >           sensor frame duration interval the pipeline has to use. This\n> >           could also limit the largest exposure times the sensor can\n> >           use. For example, if a maximum frame duration of 33ms is\n> >           requested (corresponding to 30 frames per second), the\n> >           sensor will not be able to raise the exposure time above\n> >           33ms. A fixed frame duration is achieved by setting the\n> >           minimum and maximum values to be the same.\n> >\n> >           \\todo Refer to the frame duration limits property to describe how\n> >           application-provided values gets clipped or how to reset the\n> > control\n> >           values to their defaults.\n> >\n> >           \\todo Better specify how the frame durations interact with the\n> >           exposure control algorithm.\n> >           \\sa AeEnable\n> >           \\sa AeExposureMode\n> >           \\sa ExposureTime\n> >\n> >           -----------------8< you can cut here 8<--------------------\n> >\n> >           When reported by pipelines the control expresses the duration\n> >           of the sensor frame used to produce streams part of the completed\n> >           Request. The minimum and maximum values shall then be the same,\n> > as the\n> >           sensor frame duration is a fixed parameter. The sensor frame\n> >           duration is one of the parameter that defines the capture\n> >           frame rate but it does not alone provide enough information\n> >           to fully calculate it as it does not account for pipeline\n> >           processing delays.\n> >\n> >           \\todo Define how to calculate the capture frame rate by\n> >           defining controls to report additional delays introduced by\n> >           the capture pipeline or post-processing stages (ie JPEG\n> >           conversion, frame scaling).\n> >\n>\n> Sure, no problem.  I will use the above wording and perhaps expand a bit\n> more on the AE interactions.\n\nGreat thanks, we can hopefully ack this sooner than later then.\n\nThanks\n  j\n\n>\n>\n> >\n> > > +        Specifies the minimum and maximum (in that order) allowable\n> > frame\n> > > +        duration, in micro-seconds, for the sensor to use. This could\n> > also limit\n> > > +        the largest exposure times the sensor can use. For example, if\n> > a maximum\n> > > +        frame duration of 33ms is requested (corresponding to 30 frames\n> > per\n> > > +        second), the sensor will not be able raise the exposure time\n> > above 33ms.\n> >\n> > s/will not be able raise/will not be able to raise/\n> > Fixed in the above suggestions.\n> >\n>\n> Ack.\n>\n> Regards,\n> Naush\n>\n>\n> >\n> > Thanks\n> >   j\n>\n>\n> > > +        A fixed frame duration is achieved by setting the minimum and\n> > maximum\n> > > +        values to be the same. Note that the sensor may not always be\n> > able to\n> > > +        provide the requested frame duration limits depending on its\n> > mode\n> > > +        configuration.\n> >\n> > > +        \\sa ExposureTime\n> > > +      size: [2]\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 B4C4CBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Dec 2020 16:04:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4867367F75;\n\tThu, 10 Dec 2020 17:04:08 +0100 (CET)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED2BE67E4D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 17:04:06 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 4338224000E;\n\tThu, 10 Dec 2020 16:04:06 +0000 (UTC)"],"Date":"Thu, 10 Dec 2020 17:04:15 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20201210160415.qgp7dcwail4jmjx7@uno.localdomain>","References":"<20201210133426.206679-1-naush@raspberrypi.com>\n\t<20201210144811.oz3gziiqtswurvdd@uno.localdomain>\n\t<CAEmqJPqk8CfAranY+VbrK6ZmUa2y7_L1GKxZRCSvUy12_E5YhA@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPqk8CfAranY+VbrK6ZmUa2y7_L1GKxZRCSvUy12_E5YhA@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","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":14199,"web_url":"https://patchwork.libcamera.org/comment/14199/","msgid":"<CAEmqJPofC5+pSCw-ntS0ovMqiUqVeqQiZeS5BnriEwzKr1_mrQ@mail.gmail.com>","date":"2020-12-10T16:20:38","subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\n\nOn Thu, 10 Dec 2020 at 16:04, Jacopo Mondi <jacopo@jmondi.org> wrote:\n\n> Hi Naush,\n>\n> On Thu, Dec 10, 2020 at 03:17:49PM +0000, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> > Thank you for your review comments.\n> >\n> > On Thu, 10 Dec 2020 at 14:48, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > > Hi Naush,\n> > >    thanks for the update\n> > >\n> > > On Thu, Dec 10, 2020 at 01:34:24PM +0000, Naushir Patuck wrote:\n> > > > Add a float array control (controls::FrameDurations) to specify the\n> > > > minimum and maximum (in that order) frame duration to be used by the\n> > > > camera sensor.\n> > > >\n> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > Tested-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > ---\n> > > >  src/libcamera/control_ids.yaml | 15 +++++++++++++++\n> > > >  1 file changed, 15 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/control_ids.yaml\n> > > b/src/libcamera/control_ids.yaml\n> > > > index 6d6f0fee..7f1f8624 100644\n> > > > --- a/src/libcamera/control_ids.yaml\n> > > > +++ b/src/libcamera/control_ids.yaml\n> > > > @@ -554,4 +554,19 @@ controls:\n> > > >          detection, additional format conversions etc) count as an\n> > > additional\n> > > >          pipeline stage.\n> > >\n> > > So, I've gone through the lentghy discussions on v2 and v3.\n> > >\n> > > To be very honest, I think we still have some missing pieces and the\n> > > one that concerns me the more is the interaction of this control with\n> > > the selected AE mode and the consequences on exposure/shutter-time\n> > > priorites. I see all controls about Exposure have their interaction\n> > > definition defferred to a \\todo item. This is of course not the ideal\n> > > situation but adding one make the issue only slightly worse. Deferring\n> > > these to the pipeline model definition might be an option.\n> > >\n> >\n> > You are entirely correct here on all ponts.  There is a tightly coupled\n> > interaction with this FrameDurations control and interaction with the AGC\n> > algorithm.  You have also thrown a spanner in the works for my plan :)\n> > David and I have been working on exactly this and we think we have\n> > addressed all these interactions.  I have a set of patches that are ready\n> > to be pushed as a \"phase 2\", but wanted to get this through first so as\n> not\n> > to muddy the waters too much.  This set of \"phase 2\" patches address the\n> > following:\n>\n> Ah great :)\n>\n> >\n> > 1) Set VBLANK ahead of any other controls.  This avoids the problem of\n> > setting EXPOSURE and having v4l2 reject the value due to stale limits.\n> > 2) Limit VBLANK (and hence frame duration) to what the sensor mode can\n> > actually support.  This forms part of your Clipping discussion below.\n> > 3) Add an interaction with AE when FrameDurations.  This addresses your\n> > concerns above, with AE knowing exactly what limits of shutter speed are\n> > achievable and working around any limitations based on exposure modes\n> > selected.\n>\n> These are all improvements to the RPi implementation which are\n> certainly positive, but if I got these right, it's mostly about\n> implementation.\n>\n\nThat's correct.  The \"phase 2\" patches are entirely Raspberry Pi\nimplementation specific.\n\n\n>\n> Using your point 3) as example, my main concern is more where/how\n> to specify how pipelines behaves (in the control documentation, in the\n> pipeline handler model etc), or if we want all pipelines to behave the\n> same, which I'm not even sure it's possible or desirable to enforce.\n>\n\nI suppose what the best thing to do for now is put some wording in the\ncontrol documentation on how it *should* interact with the AE.  My next\npatch will add this, and we can discuss if this is appropriate or not.\nMaybe a larger and seperate piece of work would be to expand on this in the\npipeline handler model as well?\n\n\n>\n> Did I get you right and your IPA will:\n> 1) Select the appropriate exposure time using the hint provided by\n>    AeExposureMode\n> 2) Clip the selected interval in the FrameDurations limits\n>\n\nYes, that is precisely what happens.\n\n\n>\n> This seems more than reasonable but then the first question I have is\n> how to make this consistent for application among different\n> platforms/IPA implementations and how to clearly document that.\n>\n\nThe control documentation? :-)\n\n\n>\n> This additional controls does not define that, and I think it's fair.\n> What we should avoid is going in a direction that makes it too hard to\n> backtrack, and I don't think there's anything that bad here.\n>\n> We just need more IPAs, to have a large enough base to prove these\n> concepts against a different implementation I guess.\n>\n> >\n> > Perhaps I should make those changes as part of this series so you get the\n> > full picture right now?\n>\n> The risk is to delay this more. I would do that on top if I got you\n> right and these are mostly improvements specific to the RPi\n> implementation.\n>\n\nAgreed, this is why I did not introduce it just yet.\n\n\n>\n> >\n> >\n> > >\n> > > I'm willing to ack this patch, but I think there are a few details I\n> > > would like to discuss:\n> > >\n> > > - Clipping. We need a (per-configuration, like the ScalerCrop\n> > >   rectangle) property to provide application limits and refer to it in\n> > >   this control description. I'll address this on top, but I would\n> > >   apreciate a:\n> > >\n> > >   \\todo Refer to the frame duration limits property to describe how\n> > >   application-provided values gets clipped or how to reset the control\n> > >   value to its default.\n> > >\n> >\n> > This is partially addressed in my point (2) above.  However, I did not\n> add\n> > a property, rather just did clipping based on VBLANK limits to the IPA.\n> So\n> > do you think we should have a \"properties::SensorMaxFramerate\" or similar\n> > that provides the application the maximum possible framerate for this\n> mode\n> > (derived from VBLANK control limits)?  As mentioned above, right now I\n> > simply clip the user request to what the sensor mode can achieve.\n>\n> As an implementation it's fine, but I think we need a mechanism to allow\n> application to access that information. At least, I know we need this\n> for Android, so something has to be added.\n>\n> And I think once we have that property is fair to mention here that\n> values outside the limits there reported will be clipped (and that's\n> something I think we can assume for all pipelines).\n>\n> Then if you want to support this, you will simply have to update the\n> Camera property to report the VBLANK limits.\n>\n> >\n> >\n> > >\n> > > - This is both a control and a metadata. I think the\n> > >   description is only about 'setting' the FrameDurations, not reading\n> > >   it. We have discussed in length the former but somewhat ignored the\n> > >   latter.\n> > >\n> > >   The 'reading' case could addressed by introducing a read-only control\n> > >   (ie FrameDuration) only to be used as metadata. But this might even\n> be\n> > >   more confusing as people will wonder why they have to use\n> > >   'Duration-s-' when they want a precise value and theres a 'Duration'\n> > >   (without 's') available. I'll propose an additional section but if\n> > >   you have ideas please suggest them. Also, feel free to leave this\n> > >   last part out if it turns out to be controversial and would delay the\n> > >   series any longer.\n> > >\n> >\n> > Yes, good point here.  Returning a \"FrameDuration\" may be a bit\n> redundant,\n> > as this information is conveyed in the FrameBuffer timestamps.  What do\n> > other folks think?\n>\n> That's a good point. We have timestamps and durations can be\n> calculated. I still think it would be nicer for application to\n> have a direct way to access this as part of the Request metadata. Or\n> we can expose the sensor timestamp from the Request and let them calculate\n> durations..\n>\n\nYes, any of these options would work, and should be easy to put in place.\nFor now, perhaps I keep \"FrameDurations\" metadata as what the limits are as\nper the user request.\n\n\n> >\n> >\n> > >\n> > > >\n> > > > +  - FrameDurations:\n> > > > +      type: float\n> > >\n> > > I understand this is meant to accommodate standard FPS like 29,97 FPS\n> > > (-.-) but won't expressing this as nanoseconds with a uin64_t\n> > > representation be capable of achieving the same. Floating point\n> > > arithmentic is generally a bit harder and clunky to handle when doing\n> > > calculations. I won't push if you think a float is better.\n> > >\n> >\n> > I only used float here as that is what we use in our IPA :) Happy to\n> change\n> > to int64_t based numbers.\n> >\n>\n> I would slightly prefer, but it's a gut feeling. I suspect you have a\n> reasons why you used float in your IPA I am missing. If that's the\n> case feel free to keep float here.\n>\n\nint64_t is fine, and it somewhat mirrors ExposureTime (int32_t) which is\nnice.  All of our controller code uses floats, so it is trivial for me to\ndo a conversion in our IPA.\n\n\n>\n> > >\n> > > > +      description: |\n> > >\n> > > I would:\n> > >         size: [2]\n> > >         description:\n> > >           The minimum and maximum (in that order) frame duration,\n> > >           expressed in micro-seconds.\n> > >\n> > >           When provided by applications the control specifies the\n> > >           sensor frame duration interval the pipeline has to use. This\n> > >           could also limit the largest exposure times the sensor can\n> > >           use. For example, if a maximum frame duration of 33ms is\n> > >           requested (corresponding to 30 frames per second), the\n> > >           sensor will not be able to raise the exposure time above\n> > >           33ms. A fixed frame duration is achieved by setting the\n> > >           minimum and maximum values to be the same.\n> > >\n> > >           \\todo Refer to the frame duration limits property to\n> describe how\n> > >           application-provided values gets clipped or how to reset the\n> > > control\n> > >           values to their defaults.\n> > >\n> > >           \\todo Better specify how the frame durations interact with\n> the\n> > >           exposure control algorithm.\n> > >           \\sa AeEnable\n> > >           \\sa AeExposureMode\n> > >           \\sa ExposureTime\n> > >\n> > >           -----------------8< you can cut here 8<--------------------\n> > >\n> > >           When reported by pipelines the control expresses the duration\n> > >           of the sensor frame used to produce streams part of the\n> completed\n> > >           Request. The minimum and maximum values shall then be the\n> same,\n> > > as the\n> > >           sensor frame duration is a fixed parameter. The sensor frame\n> > >           duration is one of the parameter that defines the capture\n> > >           frame rate but it does not alone provide enough information\n> > >           to fully calculate it as it does not account for pipeline\n> > >           processing delays.\n> > >\n> > >           \\todo Define how to calculate the capture frame rate by\n> > >           defining controls to report additional delays introduced by\n> > >           the capture pipeline or post-processing stages (ie JPEG\n> > >           conversion, frame scaling).\n> > >\n> >\n> > Sure, no problem.  I will use the above wording and perhaps expand a bit\n> > more on the AE interactions.\n>\n> Great thanks, we can hopefully ack this sooner than later then.\n>\n\nWill post an update with the new description text (and the other discussed\nchanges), and we can look to see if it is more appropriate.\n\nRegards,\nNaush\n\n\n\n>\n> Thanks\n>   j\n>\n> >\n> >\n> > >\n> > > > +        Specifies the minimum and maximum (in that order) allowable\n> > > frame\n> > > > +        duration, in micro-seconds, for the sensor to use. This\n> could\n> > > also limit\n> > > > +        the largest exposure times the sensor can use. For example,\n> if\n> > > a maximum\n> > > > +        frame duration of 33ms is requested (corresponding to 30\n> frames\n> > > per\n> > > > +        second), the sensor will not be able raise the exposure time\n> > > above 33ms.\n> > >\n> > > s/will not be able raise/will not be able to raise/\n> > > Fixed in the above suggestions.\n> > >\n> >\n> > Ack.\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > >\n> > > Thanks\n> > >   j\n> >\n> >\n> > > > +        A fixed frame duration is achieved by setting the minimum\n> and\n> > > maximum\n> > > > +        values to be the same. Note that the sensor may not always\n> be\n> > > able to\n> > > > +        provide the requested frame duration limits depending on its\n> > > mode\n> > > > +        configuration.\n> > >\n> > > > +        \\sa ExposureTime\n> > > > +      size: [2]\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> > >\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 7F3BABD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Dec 2020 16:21:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E62A867F79;\n\tThu, 10 Dec 2020 17:21:00 +0100 (CET)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7527567F6E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 17:20:59 +0100 (CET)","by mail-lj1-x235.google.com with SMTP id b10so5006152ljp.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Dec 2020 08:20:59 -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=\"m3pgsaqI\"; 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=xMtn0dq6RpCeQ/EKjM5G6o45G8FTwVQykJK3i0up8jw=;\n\tb=m3pgsaqI7dPHfduIbM2yZErZ9WiJMHPXcihCybjSv04AY1W/CDiScuG+/HmJTQLYwc\n\tM8trMM+9OSPDiYwSjg26aUZXrS+CQTXHvrZ/MnNOe3YhT4UctQif+EJbNaQxJJlfVS83\n\tfn+1JIEO8UvoDCnn1cYuLkIWL+8JeJiToWME6r9lYBYVta8YYnCIkNfYxAJbm3OzHPKY\n\tvSnC1zYJXzI/9Mf8GNp3Nk/Kq3wenD0UY+tpLuny9FPpMc03TaTTtsixrcarnF/3EKZK\n\tjgWhGHpklCCmw/rT88GOu9lfEiy7H0mbtGFrSLYDQPZJwZFMxBhtyu2rdvLHJNVN+OME\n\tB+OA==","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=xMtn0dq6RpCeQ/EKjM5G6o45G8FTwVQykJK3i0up8jw=;\n\tb=jD1ZkAA5f7s6o335tSyGWNLMBjnxx0+lv9rfi9N2Gns3wl5NEo4Ynk4Le2okOtpx9S\n\tCQWpMEG7qus9OkeQgoUOyhy67lleBSUEQmR6M8tsNny06o3IboTSKt/Gkmmb4kKNLBC7\n\tJW5SO/fSRgygUfzM633pgUEF9Tlg1NWhb70OD4zXQwM8auBDMb6QIU94GC/B3QvmqFcX\n\tvYNGUbf0m02gpUibArySgjU49uNk8YbDvuLCQ+M+xQKLinKwzfoKTw1Ay/C1r6bgTq2C\n\tkj8vFdkGpf41szZsHtQE/z4xXdQ6lQoYPHUEhG+42DUkVMNsiTFgarkDPA2zeJc9V4p+\n\tW/kw==","X-Gm-Message-State":"AOAM531yf41gVrCL/R1uRqiaoVTfV9vdreWf6lnynvcahCl+RqovZzsH\n\tMTdT2sITD1WQbq53cDa6hQ0gtoKR489E9y7Y+VECaA==","X-Google-Smtp-Source":"ABdhPJwuoEres1KFAeDm2F2uh399MAeK7C+m76EBXJE28L4SGcQ8FCYpWRn80ZCNQvjvJ/swfrPVXj9GK/UF7+pNn5M=","X-Received":"by 2002:a2e:a58b:: with SMTP id\n\tm11mr3207128ljp.329.1607617254298; \n\tThu, 10 Dec 2020 08:20:54 -0800 (PST)","MIME-Version":"1.0","References":"<20201210133426.206679-1-naush@raspberrypi.com>\n\t<20201210144811.oz3gziiqtswurvdd@uno.localdomain>\n\t<CAEmqJPqk8CfAranY+VbrK6ZmUa2y7_L1GKxZRCSvUy12_E5YhA@mail.gmail.com>\n\t<20201210160415.qgp7dcwail4jmjx7@uno.localdomain>","In-Reply-To":"<20201210160415.qgp7dcwail4jmjx7@uno.localdomain>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Thu, 10 Dec 2020 16:20:38 +0000","Message-ID":"<CAEmqJPofC5+pSCw-ntS0ovMqiUqVeqQiZeS5BnriEwzKr1_mrQ@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v5 1/3] libcamera: controls: Add frame\n\tduration control","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=\"===============0401640755429399390==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]