[{"id":4893,"web_url":"https://patchwork.libcamera.org/comment/4893/","msgid":"<20200524222302.GA6006@pendragon.ideasonboard.com>","date":"2020-05-24T22:23:02","subject":"Re: [libcamera-devel] [PATCH v2 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, May 13, 2020 at 10:11:18AM +0100, 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> ---\n>  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n>  1 file changed, 14 insertions(+)\n> \n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index 77ebc3f9..42694fc7 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -239,4 +239,18 @@ controls:\n>          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n>          control can only be returned in metadata.\n>        size: [4]\n> +\n> +  - FrameDurations:\n> +      type: float\n\nDo we need sub-microsecond precision, or would a uint32_t control work ?\nI'd rather have a fixed precision if possible.\n\n> +      description: |\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> +        Note that the sensor may not always be able to provide the requested\n> +        frame duration limits depending on its mode configuration.\n\nThis looks good to me, but I'd like to discuss the corner cases I can\nalready think about.\n\n- Are there use cases for an application to specify a minimum frame\n  duration only, or a maximum frame duration only (that is, any frame\n  rate lower than a limit, or any frame rate higher than a limit) ? If\n  so, how should that be specified ? We could set the minimum value to 0\n  to mean that the maximum frame rate is unbounded, and the maximum\n  value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n  Is there anything I'm overlooking ?\n\n- Is 0 an acceptable value ? Or should 1 be the minimum value ?\n\n- What happens if the requested frame duration isn't achievable ? Should\n  we specify that the camera will use a frame duration as close to the\n  requested range as possible ? Or could there be cases where a\n  different behaviour would be needed ?\n\n- Not something we need to address now, but do you see any future\n  relation between this control and anti-banding (50 or 60Hz flicker\n  avoidance) ? Anti-banding will mostly restrict possible exposure\n  times, and should only indirectly interact with the frame duration if\n  I'm not mistaken, is that correct ?\n\n> +\n> +        \\sa ExposureTime\n> +      size: [2]\n>  ...","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E88161012\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 May 2020 00:23:16 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A485A24D;\n\tMon, 25 May 2020 00:23:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"iWf+uNYN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1590358995;\n\tbh=bztDxyGsrEyVlJX0iEg3ICzieaxj2iyKMOfXo1lBGVU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iWf+uNYNTf1h9a556NERxEVQ0JFeMG9e9C+T1UFHcSPx+SBIpANdDgxVz/hge7EO9\n\t7xzPSh6xFfe/7bWl+dk7HvNcd5y/ZVtx8Iuqp4GB3RYQbLGzTiehSpaaCGpahuW2ky\n\tAQsNmwQIkPcVbUonfemzUgVXyk6rc4aztPmdaquU=","Date":"Mon, 25 May 2020 01:23:02 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200524222302.GA6006@pendragon.ideasonboard.com>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200513091120.1653-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Sun, 24 May 2020 22:23:16 -0000"}},{"id":4905,"web_url":"https://patchwork.libcamera.org/comment/4905/","msgid":"<CAEmqJPqBe+zB=ceSJebeejkToBoD5YaCOp-XO6BWzpoy0t7CBA@mail.gmail.com>","date":"2020-05-26T08:59:57","subject":"Re: [libcamera-devel] [PATCH v2 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 Laurent,\n\n\nOn Sun, 24 May 2020 at 23:23, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > ---\n> >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> >  1 file changed, 14 insertions(+)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index 77ebc3f9..42694fc7 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -239,4 +239,18 @@ controls:\n> >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> >          control can only be returned in metadata.\n> >        size: [4]\n> > +\n> > +  - FrameDurations:\n> > +      type: float\n>\n> Do we need sub-microsecond precision, or would a uint32_t control work ?\n> I'd rather have a fixed precision if possible.\n\nThe reason I chose floating point here was because of legacy standards\n(e.g. 29.97/59.94 fps for SMPTE).  It also does allow us to specify <\n1 fps if a user ever needs to (e.g. for a time lapse type capture use\ncase).\n\n> > +      description: |\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> > +        Note that the sensor may not always be able to provide the requested\n> > +        frame duration limits depending on its mode configuration.\n>\n> This looks good to me, but I'd like to discuss the corner cases I can\n> already think about.\n>\n> - Are there use cases for an application to specify a minimum frame\n>   duration only, or a maximum frame duration only (that is, any frame\n>   rate lower than a limit, or any frame rate higher than a limit) ? If\n>   so, how should that be specified ? We could set the minimum value to 0\n>   to mean that the maximum frame rate is unbounded, and the maximum\n>   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n>   Is there anything I'm overlooking ?\n\nGood point, I can see we might only want to set one limit.  In these\ncases, the sensor specified limit would be used.  Speaking of this, I\nwas going to address sensor reported framerate after this patch.  Do\nwe do need a way for the sensor driver to specify its framerate limits\nif the application wants to know?  Perhaps this should be part of\nCameraSensorInfo, although it will be mode specific?  Or perhaps we\njust use the VBLANK min/max to give us the limits?\n\n>\n> - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n\nIf we stick to floating point, we should use 0.0 as the min limit.\n\n>\n> - What happens if the requested frame duration isn't achievable ? Should\n>   we specify that the camera will use a frame duration as close to the\n>   requested range as possible ? Or could there be cases where a\n>   different behaviour would be needed ?\n\nI think the only sensible thing would be to truncate the requested\nvalues to the sensor limits.  See above on how we get the limits from\nthe sensor though.\n\n>\n> - Not something we need to address now, but do you see any future\n>   relation between this control and anti-banding (50 or 60Hz flicker\n>   avoidance) ? Anti-banding will mostly restrict possible exposure\n>   times, and should only indirectly interact with the frame duration if\n>   I'm not mistaken, is that correct ?\n\nThere should be minimal interaction with framerate control.  As you\nsaid, we will restrict use of certain exposure times for antibanding,\nbut will have more flexibility with framerate.\n\n>\n> > +\n> > +        \\sa ExposureTime\n> > +      size: [2]\n> >  ...\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nRegards,\nNaush","headers":{"Return-Path":"<naush@raspberrypi.com>","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11B3D603D1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 May 2020 11:00:14 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id d7so694433lfi.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 May 2020 02:00:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"tHKvoyWJ\"; 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=5QO4yyLJ90y7uYpMthxs4/ZcpKja0Lij8jcJR+BZCHs=;\n\tb=tHKvoyWJNL1c8nejdxiTq3ayfZPVEhkq9dZ+pptaIciNXRd/KpYIO92aALP9fRM6cB\n\tlYCrZ0tGm+TdBtevzcnUGiXdRPaWzCov6VH9HgBoYp/NCz0Em5sMI4/kBfMDPVvXZYEV\n\tHzbg8wmXuWqoU6bDARzBkNkb+TPWE1BDhae69/JkLhvR4TObXY1XGcTobgbrsg2pz6pu\n\tQAe11VOuoXRgy9X6/an9dqjIEFNE+EBiI0+9wRtcuAvcVMnI3Dxy4J5VKA9Kfvq8rljU\n\txN+W5TmHeHA8OKlToyJUy+P3Zog5Ugl8AnxVkGeUOrzZDFnyR0loSV8OCk00QX+Mw3BH\n\t8TBA==","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=5QO4yyLJ90y7uYpMthxs4/ZcpKja0Lij8jcJR+BZCHs=;\n\tb=FjVGokWGRzaJpJcE6bwGX1SK2q/YvsbEUpRQeizwnLf9y0qd27huUf/f0DUAil3Zq/\n\tPtwcloPcWc9I2FmYXbG3ETRtWRBqxcaxup55BtrV8dI9dfpAmTz27qG6+tjsM2j23Cbk\n\tdi6Pf9ZzSfiL9qwr3vGUHSFJVqxSATlPe9NotKtqqYYXpZQiCSbMjTwvVIxRn8IeAIjf\n\tOhDRmBDrEfDBjL8d85Ig6i/CV9NiZot5PfEytFyA8UYicn6hI85dBA8Yfc/i5qImVe3h\n\ty6GrcgR2QsmSLObHgm7D2sf8CCBA+5mvQAH/cw+SxtSKPJ7h99i/DQwuf4KMA7nek+4R\n\tHr1w==","X-Gm-Message-State":"AOAM530y5/YcTGONn9hQ+0Xvmbw8fRX2BxJa/W7mbDUtg0jYnQOPGZr8\n\t8XqBFrqtgFa3z5WEkWKAupNuO01X7aUl5KQUm9KKeg==","X-Google-Smtp-Source":"ABdhPJy+u1l9QctZ0iPtnrSdpF8bPkHx7y8WPGB3aYoWHdtcGJF5w5vbLVHYjeE1hPwZ2ImasEz+uXR1NtmIApBqQiU=","X-Received":"by 2002:ac2:44bb:: with SMTP id c27mr16771lfm.59.1590483613352; \n\tTue, 26 May 2020 02:00:13 -0700 (PDT)","MIME-Version":"1.0","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>","In-Reply-To":"<20200524222302.GA6006@pendragon.ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Tue, 26 May 2020 09:59:57 +0100","Message-ID":"<CAEmqJPqBe+zB=ceSJebeejkToBoD5YaCOp-XO6BWzpoy0t7CBA@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Tue, 26 May 2020 09:00:15 -0000"}},{"id":4914,"web_url":"https://patchwork.libcamera.org/comment/4914/","msgid":"<20200527214734.45xrc3etyks27lsz@uno.localdomain>","date":"2020-05-27T21:47:34","subject":"Re: [libcamera-devel] [PATCH v2 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, Laurent,\n\nOn Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > ---\n> >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> >  1 file changed, 14 insertions(+)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index 77ebc3f9..42694fc7 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -239,4 +239,18 @@ controls:\n> >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> >          control can only be returned in metadata.\n> >        size: [4]\n> > +\n> > +  - FrameDurations:\n> > +      type: float\n>\n> Do we need sub-microsecond precision, or would a uint32_t control work ?\n> I'd rather have a fixed precision if possible.\n>\n> > +      description: |\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\nIsn't it s/maximum frame duration/minimum frame duration/ ?\n\nIf at least 30fps are requested, the exposure time cannot be longer\nthan 33msec ? Am I reading this wrong ?\n\n> > +        Note that the sensor may not always be able to provide the requested\n> > +        frame duration limits depending on its mode configuration.\n\nI've been really confused by this, until I realized I was having an\nhard time trying to figure out how to use this for applications to\nexpress a request for a precise duration and how should the library\nhave used it to report the actual frame durtion in metadata. Until I\nrealized this could probably be better interpreted not as a frame\nduration request or result, but as an hint to the AE algorithm to\nclamp its calculation into certain duration limtis.\n\nAnd I agree a duration range might be a good hint to drive the AE\nroutine and if I got the rest of the series right, that's\nwhat happens in the IPA with these patches.\n\nIf that's actually the intention, what about making of this an\n\"AeFrameDurationLimits\" control, to make it clear it applies to AE and\nuse \"FrameDuration\" as an integer control to represent the requested\nand precise frame duration in requests and metadata respectively.\n\nOn \"FrameDuration\" itself: would it make sense to only considered it\nwhen running with AE off ? In that case it would be applications\nresponsibility to opportunely calculate the exposure time and the frame\nduration, but I think it's expected if you run in manual mode. And for\nreference, that's what Android does as well :)\n\nIt would also rule out the need to specify how the frameDurationLimits\nwould be used when running in manual mode and wanting to configure a\nprecise value there, as I guess you would have to give to min and max\nthe same value to express that.\n\nthis might open pandora's box, but: is the frame duration a property of\nthe camera, or of a single stream ?\n\n>\n> This looks good to me, but I'd like to discuss the corner cases I can\n> already think about.\n>\n> - Are there use cases for an application to specify a minimum frame\n>   duration only, or a maximum frame duration only (that is, any frame\n>   rate lower than a limit, or any frame rate higher than a limit) ? If\n>   so, how should that be specified ? We could set the minimum value to 0\n>   to mean that the maximum frame rate is unbounded, and the maximum\n>   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n>   Is there anything I'm overlooking ?\n\nIf this becames an AE-related control and you specify that, I think\nyou should provide both values. If you don't care about one of the two\njust use the values of the (forthcoming) property that reports the\nmin and max frame durations of a camera. If an applications does not\nspecify this control, the AE routine runs free, in the limits\nof the camera sensor capabilities.\n\nDoes it make sense ?\n\nThanks\n  j\n\n>\n> - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n>\n> - What happens if the requested frame duration isn't achievable ? Should\n>   we specify that the camera will use a frame duration as close to the\n>   requested range as possible ? Or could there be cases where a\n>   different behaviour would be needed ?\n>\n> - Not something we need to address now, but do you see any future\n>   relation between this control and anti-banding (50 or 60Hz flicker\n>   avoidance) ? Anti-banding will mostly restrict possible exposure\n>   times, and should only indirectly interact with the frame duration if\n>   I'm not mistaken, is that correct ?\n>\n> > +\n> > +        \\sa ExposureTime\n> > +      size: [2]\n> >  ...\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4878E603D1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 May 2020 23:44:15 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 93A3D240002;\n\tWed, 27 May 2020 21:44:14 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 27 May 2020 23:47:34 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200527214734.45xrc3etyks27lsz@uno.localdomain>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200524222302.GA6006@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Wed, 27 May 2020 21:44:15 -0000"}},{"id":4915,"web_url":"https://patchwork.libcamera.org/comment/4915/","msgid":"<20200528002400.GA4670@pendragon.ideasonboard.com>","date":"2020-05-28T00:24:00","subject":"Re: [libcamera-devel] [PATCH v2 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, May 26, 2020 at 09:59:57AM +0100, Naushir Patuck wrote:\n> On Sun, 24 May 2020 at 23:23, Laurent Pinchart wrote:\n> > On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > > ---\n> > >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> > >  1 file changed, 14 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > index 77ebc3f9..42694fc7 100644\n> > > --- a/src/libcamera/control_ids.yaml\n> > > +++ b/src/libcamera/control_ids.yaml\n> > > @@ -239,4 +239,18 @@ controls:\n> > >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> > >          control can only be returned in metadata.\n> > >        size: [4]\n> > > +\n> > > +  - FrameDurations:\n> > > +      type: float\n> >\n> > Do we need sub-microsecond precision, or would a uint32_t control work ?\n> > I'd rather have a fixed precision if possible.\n> \n> The reason I chose floating point here was because of legacy standards\n> (e.g. 29.97/59.94 fps for SMPTE).  It also does allow us to specify <\n> 1 fps if a user ever needs to (e.g. for a time lapse type capture use\n> case).\n\nBut we're now using a frame duration in µs. A value lower than one would\ncorrespond to a frame rate higher than 1Mfps, which is unlikely :-)\n\n> > > +      description: |\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> > > +        Note that the sensor may not always be able to provide the requested\n> > > +        frame duration limits depending on its mode configuration.\n> >\n> > This looks good to me, but I'd like to discuss the corner cases I can\n> > already think about.\n> >\n> > - Are there use cases for an application to specify a minimum frame\n> >   duration only, or a maximum frame duration only (that is, any frame\n> >   rate lower than a limit, or any frame rate higher than a limit) ? If\n> >   so, how should that be specified ? We could set the minimum value to 0\n> >   to mean that the maximum frame rate is unbounded, and the maximum\n> >   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n> >   Is there anything I'm overlooking ?\n> \n> Good point, I can see we might only want to set one limit.  In these\n> cases, the sensor specified limit would be used.\n\nHow should applications specify that ? By setting the other value to 0\n(for minimum) or UINT32_MAX (for maximum) ? Or do you think another\nmeans would be better ?\n\n> Speaking of this, I was going to address sensor reported framerate\n> after this patch.  Do we do need a way for the sensor driver to\n> specify its framerate limits if the application wants to know?\n\nI think reporting the minimum and maximum achievable frame durations is\nuseful. It could be done by specifying FrameDurations (or\nFrameDurationLimits, as proposed in the review of 2/2) as a property\n(Camera::properties()). The maximum won't be a problem, but the minimum\n(highest frame rate) may depend on the frame size, so this can be\ntricky.\n\n> Perhaps this should be part of CameraSensorInfo, although it will be\n> mode specific?  Or perhaps we just use the VBLANK min/max to give us\n> the limits?\n\nCameraSensorInfo is meant to convey information specific to the mode\nthat has been selected (or, more generally speaking, the sensor\nconfiguration that has been set). It won't be a good match to report\ndata that need to be set in camera static properties, at least when\npassed to IPAInterface::configure().\n\nThe maximum achievable frame rate depends on both the sensor and the\nISP, as the ISP bandwidth limit may be lower than what the sensor\nsupports. Could it also depend on the algorithms (and would thus need to\ninvolve the IPA), or would it be possible to always compute the limits\nin the pipeline handler ?\n\nWe need to figure out what information we need to calculate the limits,\nand identify what information needs to be retrieved from the kernel (if\nany), what static information about the sensor should come from a\nuserspace sensor database (similar to CamHelper, but moved to the\nlibcamera core or libipa), and how the pipeline handler and IPA should\ncollaborate to perform the computation. I can help designing all this,\nbut if you already have an idea regarding how it could be done (even if\nin a way that would be specific to Raspberry Pi) that would be useful as\na base for discussions.\n\n> > - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n> \n> If we stick to floating point, we should use 0.0 as the min limit.\n> \n> > - What happens if the requested frame duration isn't achievable ? Should\n> >   we specify that the camera will use a frame duration as close to the\n> >   requested range as possible ? Or could there be cases where a\n> >   different behaviour would be needed ?\n> \n> I think the only sensible thing would be to truncate the requested\n> values to the sensor limits.  See above on how we get the limits from\n> the sensor though.\n\nThat sounds good to me. Could you update the description to document\nthis ?\n\n> > - Not something we need to address now, but do you see any future\n> >   relation between this control and anti-banding (50 or 60Hz flicker\n> >   avoidance) ? Anti-banding will mostly restrict possible exposure\n> >   times, and should only indirectly interact with the frame duration if\n> >   I'm not mistaken, is that correct ?\n> \n> There should be minimal interaction with framerate control.  As you\n> said, we will restrict use of certain exposure times for antibanding,\n> but will have more flexibility with framerate.\n\nOK, good to have a confirmation that I shouldn't worry about it :-)\n\n> > > +\n> > > +        \\sa ExposureTime\n> > > +      size: [2]\n> > >  ...","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FB8C603CD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 May 2020 02:24:14 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EA01829F;\n\tThu, 28 May 2020 02:24:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CX7qC+2T\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1590625454;\n\tbh=UkFn/SglxDdsuVWg8dmvzFSmERiZ7VHdIKTk00fJ9z8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CX7qC+2TsoUGj2/oresus3QqABx9qctxTKOjH37g9IjT+xdR05a4wwXQYUCKOsR4t\n\thtjpX937hT/izNUg4YDc/HphonyUPFjwYkimXQaIPfDzjz5zhR/PY7sMFg9cMDMDeo\n\tixsk66WU8ETIs4TYiT3W30ahTC4nD/oxtayUKMqM=","Date":"Thu, 28 May 2020 03:24:00 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200528002400.GA4670@pendragon.ideasonboard.com>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<CAEmqJPqBe+zB=ceSJebeejkToBoD5YaCOp-XO6BWzpoy0t7CBA@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<CAEmqJPqBe+zB=ceSJebeejkToBoD5YaCOp-XO6BWzpoy0t7CBA@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Thu, 28 May 2020 00:24:15 -0000"}},{"id":4917,"web_url":"https://patchwork.libcamera.org/comment/4917/","msgid":"<20200528005541.GB4670@pendragon.ideasonboard.com>","date":"2020-05-28T00:55:41","subject":"Re: [libcamera-devel] [PATCH v2 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, May 27, 2020 at 11:47:34PM +0200, Jacopo Mondi wrote:\n> On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:\n> > On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > > ---\n> > >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> > >  1 file changed, 14 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > index 77ebc3f9..42694fc7 100644\n> > > --- a/src/libcamera/control_ids.yaml\n> > > +++ b/src/libcamera/control_ids.yaml\n> > > @@ -239,4 +239,18 @@ controls:\n> > >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> > >          control can only be returned in metadata.\n> > >        size: [4]\n> > > +\n> > > +  - FrameDurations:\n> > > +      type: float\n> >\n> > Do we need sub-microsecond precision, or would a uint32_t control work ?\n> > I'd rather have a fixed precision if possible.\n> >\n> > > +      description: |\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> \n> Isn't it s/maximum frame duration/minimum frame duration/ ?\n> \n> If at least 30fps are requested, the exposure time cannot be longer\n> than 33msec ? Am I reading this wrong ?\n\nI may read it wrong, but I think Naush is correct here. If a *maximum*\nframe duration of 33ms is requested [that is, a *minimum* frame rate of\n30fps], the sensor will not be able to *raise* the exposure time above\n33ms.\n\n> > > +        Note that the sensor may not always be able to provide the requested\n> > > +        frame duration limits depending on its mode configuration.\n> \n> I've been really confused by this, until I realized I was having an\n> hard time trying to figure out how to use this for applications to\n> express a request for a precise duration and how should the library\n> have used it to report the actual frame durtion in metadata. Until I\n> realized this could probably be better interpreted not as a frame\n> duration request or result, but as an hint to the AE algorithm to\n> clamp its calculation into certain duration limtis.\n> \n> And I agree a duration range might be a good hint to drive the AE\n> routine and if I got the rest of the series right, that's\n> what happens in the IPA with these patches.\n> \n> If that's actually the intention, what about making of this an\n> \"AeFrameDurationLimits\" control, to make it clear it applies to AE and\n> use \"FrameDuration\" as an integer control to represent the requested\n> and precise frame duration in requests and metadata respectively.\n\nI think (Ae)FrameDurationLimits makes sense as a control (and as a\nproperty too, with the caveat that the minimum frame duration - highest\nframe rate - can be dependent on the stream resolution), and\nFrameDuration makes sense for metadata.\n\nMore about FrameDuration as a control below.\n\n> On \"FrameDuration\" itself: would it make sense to only considered it\n> when running with AE off ? In that case it would be applications\n> responsibility to opportunely calculate the exposure time and the frame\n> duration, but I think it's expected if you run in manual mode. And for\n> reference, that's what Android does as well :)\n> \n> It would also rule out the need to specify how the frameDurationLimits\n> would be used when running in manual mode and wanting to configure a\n> precise value there, as I guess you would have to give to min and max\n> the same value to express that.\n\n\n> this might open pandora's box, but: is the frame duration a property of\n> the camera, or of a single stream ?\n\nI've opened the same box in my previous e-mail :-) I think the frame\nduration is a property of the camera, as all streams share the same\nsensor, and are thus slave to the same frame rate. Some streams could\ndrop frames, but I think I would then report that as a decimation factor\nper stream (not sure what the use cases would be though). This being\nsaid, the maximum frame rate (minimum frame duration) can depend on the\nconfiguration of the camera, due to bandwidth limitations. It shouldn't\naffect the (Ae)FrameDuration(Limits) control or metadata, but will\naffect the (Ae)FrameDurationLimits property. It's something we need to\naddress eventually, not necessarily as part of this series, we could\nstart with a limit that doesn't depend on the camera configuration and\nthen build additional features on top.\n\n> > This looks good to me, but I'd like to discuss the corner cases I can\n> > already think about.\n> >\n> > - Are there use cases for an application to specify a minimum frame\n> >   duration only, or a maximum frame duration only (that is, any frame\n> >   rate lower than a limit, or any frame rate higher than a limit) ? If\n> >   so, how should that be specified ? We could set the minimum value to 0\n> >   to mean that the maximum frame rate is unbounded, and the maximum\n> >   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n> >   Is there anything I'm overlooking ?\n> \n> If this becames an AE-related control and you specify that, I think\n> you should provide both values. If you don't care about one of the two\n> just use the values of the (forthcoming) property that reports the\n> min and max frame durations of a camera. If an applications does not\n> specify this control, the AE routine runs free, in the limits\n> of the camera sensor capabilities.\n> \n> Does it make sense ?\n\nYes that can work. Specifying anything below the minimum possible or\nabove the maximum possible value supported by the camera will be clamped\nto the camera limits anyway.\n\n> > - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n> >\n> > - What happens if the requested frame duration isn't achievable ? Should\n> >   we specify that the camera will use a frame duration as close to the\n> >   requested range as possible ? Or could there be cases where a\n> >   different behaviour would be needed ?\n> >\n> > - Not something we need to address now, but do you see any future\n> >   relation between this control and anti-banding (50 or 60Hz flicker\n> >   avoidance) ? Anti-banding will mostly restrict possible exposure\n> >   times, and should only indirectly interact with the frame duration if\n> >   I'm not mistaken, is that correct ?\n> >\n> > > +\n> > > +        \\sa ExposureTime\n> > > +      size: [2]\n> > >  ...","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54F46603CD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 May 2020 02:55:56 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ACE3A2A3;\n\tThu, 28 May 2020 02:55:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AKt7MLhl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1590627355;\n\tbh=+ErLxojVm3rIiKJt9hN77YDmZPHMuLjBA2CGkKcWfLQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AKt7MLhlLPK9o2VSxdUvJ2XbBzPmyFC2sqSIO5VxOcWBvTu48mVsZyUc3uv/2R0zL\n\t16yQ1ja0IQ0H/wJ2K6FmmoWVU2NX0vhnLz38raTwxd6BwX+e4G/STlnzCo19K/ldSi\n\t43pIBe1/wxTb+XYC8yibQ0v3HuSQIA5/C7qmeqyU=","Date":"Thu, 28 May 2020 03:55:41 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200528005541.GB4670@pendragon.ideasonboard.com>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200527214734.45xrc3etyks27lsz@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Thu, 28 May 2020 00:55:56 -0000"}},{"id":4920,"web_url":"https://patchwork.libcamera.org/comment/4920/","msgid":"<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>","date":"2020-05-28T08:52:51","subject":"Re: [libcamera-devel] [PATCH v2 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 Laurent and Jacopo,\n\nI'll address the comments from both emails jointly below:\n\nOn Thu, 28 May 2020 at 01:55, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Jacopo,\n>\n> On Wed, May 27, 2020 at 11:47:34PM +0200, Jacopo Mondi wrote:\n> > On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:\n> > > On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > > > ---\n> > > >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> > > >  1 file changed, 14 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > index 77ebc3f9..42694fc7 100644\n> > > > --- a/src/libcamera/control_ids.yaml\n> > > > +++ b/src/libcamera/control_ids.yaml\n> > > > @@ -239,4 +239,18 @@ controls:\n> > > >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> > > >          control can only be returned in metadata.\n> > > >        size: [4]\n> > > > +\n> > > > +  - FrameDurations:\n> > > > +      type: float\n> > >\n> > > Do we need sub-microsecond precision, or would a uint32_t control work ?\n> > > I'd rather have a fixed precision if possible.\n> > >\n> > > > +      description: |\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> >\n> > Isn't it s/maximum frame duration/minimum frame duration/ ?\n> >\n> > If at least 30fps are requested, the exposure time cannot be longer\n> > than 33msec ? Am I reading this wrong ?\n>\n> I may read it wrong, but I think Naush is correct here. If a *maximum*\n> frame duration of 33ms is requested [that is, a *minimum* frame rate of\n> 30fps], the sensor will not be able to *raise* the exposure time above\n> 33ms.\n>\n\nYes, my wording in the description matches Laurent's interpretation.\nI must admit, I do find it more difficult talking about frame\ndurations vs framerates :)\n\n> > > > +        Note that the sensor may not always be able to provide the requested\n> > > > +        frame duration limits depending on its mode configuration.\n> >\n> > I've been really confused by this, until I realized I was having an\n> > hard time trying to figure out how to use this for applications to\n> > express a request for a precise duration and how should the library\n> > have used it to report the actual frame durtion in metadata. Until I\n> > realized this could probably be better interpreted not as a frame\n> > duration request or result, but as an hint to the AE algorithm to\n> > clamp its calculation into certain duration limtis.\n> >\n> > And I agree a duration range might be a good hint to drive the AE\n> > routine and if I got the rest of the series right, that's\n> > what happens in the IPA with these patches.\n> >\n> > If that's actually the intention, what about making of this an\n> > \"AeFrameDurationLimits\" control, to make it clear it applies to AE and\n> > use \"FrameDuration\" as an integer control to represent the requested\n> > and precise frame duration in requests and metadata respectively.\n>\n> I think (Ae)FrameDurationLimits makes sense as a control (and as a\n> property too, with the caveat that the minimum frame duration - highest\n> frame rate - can be dependent on the stream resolution), and\n> FrameDuration makes sense for metadata.\n\nGood points here.  When doing this change, I purposely did not want to\ndirectly tie this control with AE for a few of reasons, even though\nthere is an interaction:\n\n1) Even though the FrameDurationLimits does restrict what the AE can\nask for, in our IPA, they are two distinct and separate operations.\nIn fact, our AE does not even see the frameduration limits that the\nuser set.  The results of AE just get clipped by the\nFrameDurationLimits.  This is possibly sub-optimal (see my last email\nfor the discussion), but still gives the desired exposure.  In future\nwe will probably optimise this, but given the complexity, will leave\nit for another change.\n2) AE does have a way to clip requested shutter speeds using the\n\"Exposure Profile\" tuning parameters which offers another degree of\nfreedom to control the interaction of shutter speed with analogue\ngain.  FrameDurationLimits only controls shutter speed which may not\nbe desirable.\n\n>\n> More about FrameDuration as a control below.\n>\n> > On \"FrameDuration\" itself: would it make sense to only considered it\n> > when running with AE off ? In that case it would be applications\n> > responsibility to opportunely calculate the exposure time and the frame\n> > duration, but I think it's expected if you run in manual mode. And for\n> > reference, that's what Android does as well :)\n> >\n> > It would also rule out the need to specify how the frameDurationLimits\n> > would be used when running in manual mode and wanting to configure a\n> > precise value there, as I guess you would have to give to min and max\n> > the same value to express that.\n\nFrameDurationLimits should only really apply with AE enabled.  AE\nvaries the shutter speed all over the place, that could cause\nframerate changes that we need to clip.  If AE is disabled and we have\na manual shutter speed set, that dictates the frame duration, and any\nlimits will not apply.  Perhaps I should add this in the description?\n\n>\n>\n> > this might open pandora's box, but: is the frame duration a property of\n> > the camera, or of a single stream ?\n>\n> I've opened the same box in my previous e-mail :-) I think the frame\n> duration is a property of the camera, as all streams share the same\n> sensor, and are thus slave to the same frame rate. Some streams could\n> drop frames, but I think I would then report that as a decimation factor\n> per stream (not sure what the use cases would be though). This being\n> said, the maximum frame rate (minimum frame duration) can depend on the\n> configuration of the camera, due to bandwidth limitations. It shouldn't\n> affect the (Ae)FrameDuration(Limits) control or metadata, but will\n> affect the (Ae)FrameDurationLimits property. It's something we need to\n> address eventually, not necessarily as part of this series, we could\n> start with a limit that doesn't depend on the camera configuration and\n> then build additional features on top.\n\nAgreed.\n\n>\n> > > This looks good to me, but I'd like to discuss the corner cases I can\n> > > already think about.\n> > >\n> > > - Are there use cases for an application to specify a minimum frame\n> > >   duration only, or a maximum frame duration only (that is, any frame\n> > >   rate lower than a limit, or any frame rate higher than a limit) ? If\n> > >   so, how should that be specified ? We could set the minimum value to 0\n> > >   to mean that the maximum frame rate is unbounded, and the maximum\n> > >   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n> > >   Is there anything I'm overlooking ?\n> >\n> > If this becames an AE-related control and you specify that, I think\n> > you should provide both values. If you don't care about one of the two\n> > just use the values of the (forthcoming) property that reports the\n> > min and max frame durations of a camera. If an applications does not\n> > specify this control, the AE routine runs free, in the limits\n> > of the camera sensor capabilities.\n> >\n> > Does it make sense ?\n>\n> Yes that can work. Specifying anything below the minimum possible or\n> above the maximum possible value supported by the camera will be clamped\n> to the camera limits anyway.\n>\n> > > - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n> > >\n> > > - What happens if the requested frame duration isn't achievable ? Should\n> > >   we specify that the camera will use a frame duration as close to the\n> > >   requested range as possible ? Or could there be cases where a\n> > >   different behaviour would be needed ?\n> > >\n> > > - Not something we need to address now, but do you see any future\n> > >   relation between this control and anti-banding (50 or 60Hz flicker\n> > >   avoidance) ? Anti-banding will mostly restrict possible exposure\n> > >   times, and should only indirectly interact with the frame duration if\n> > >   I'm not mistaken, is that correct ?\n> > >\n> > > > +\n> > > > +        \\sa ExposureTime\n> > > > +      size: [2]\n> > > >  ...\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nRegards,\nNaush","headers":{"Return-Path":"<naush@raspberrypi.com>","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A2EE603CF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 May 2020 10:53:09 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id b6so32427984ljj.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 May 2020 01:53:09 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"o/6Pv7dJ\"; 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=k5EeYr2zebAA3z8z2BMWvS0/JimmEG3j/ScKLXpw1q0=;\n\tb=o/6Pv7dJAo/SiM2YJe/UXCaSr4LO1iZjG4QRWE948D5j0a24eD0Re5b7dolPR7Cc8g\n\tPp1l3eIi+fCHTUlq6anWLeB9LQQG4ytXL93AAk4OUr/DRsLsoBFzeP+w2PjY/7tQrWtr\n\t10ODH2aDuzeaPfoX/2i1tabwuxXqDcI7mFyvbe6cnd+UNZ8A2uSeqKNBXHXFXFJwRPbb\n\tE68YICbzDOSZ2BHuNbd9BATrlFTbcdjAN11bQTbDjKVXipd4WrLvDP6cESOdxa/zK8i9\n\tyW+/tFa7/pV0TKpF699JiVBTarVYt5IsXZbgD4pPJGu8as/i97pVXw9Mczc5oq43+xdL\n\tTXcg==","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=k5EeYr2zebAA3z8z2BMWvS0/JimmEG3j/ScKLXpw1q0=;\n\tb=dY6Ux8WuDEjbQm5zhgQRrAGF/VxK8hXHphMP5js/BNxNgArPUhvwoER562GpOtimjK\n\tWVqlplsNZ5W9WJm2L32a/oUGmZVUrrhhGHVcx0X7na1XRfKTIGJESYwkyUIYz91CD2O6\n\tY2EYvT4Un3xEWpNJYHN9JMo3PNcfKvtHObc50QqbuWdxkIXsu/qJHdk7I2UcYpTTEteK\n\t3hCeWoSMpogkE5uUY1XJLm0+UwkcRLp8E5NdAksLU4ppl3q0gwWfZ4CBu+ZsT/sTPb3+\n\tePg4RP56YYOmLbdnrlQzX5PUrqd6JluVfTmO1+cnMcR7aIIjjLt8jdH9KDEQGq+p4QHf\n\tUHHQ==","X-Gm-Message-State":"AOAM530kscW93b8XhC2n71df+cYiGthEXyFNGMjj6rXKu8l9PWkFxsGp\n\tdSJz7uHwHlH6RBxjT3m5kix/Vo1wbNDARA5W+7EO6Q==","X-Google-Smtp-Source":"ABdhPJw0FpRypPjA4vb73C789lBv+MY/XKUd67TnfExqyuBJR3LsTBriMrG4DPzcX/PQTHanRJvf0htxNGWDWuAe35I=","X-Received":"by 2002:a05:651c:1208:: with SMTP id\n\ti8mr1010101lja.103.1590655988120; \n\tThu, 28 May 2020 01:53:08 -0700 (PDT)","MIME-Version":"1.0","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>","In-Reply-To":"<20200528005541.GB4670@pendragon.ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Thu, 28 May 2020 09:52:51 +0100","Message-ID":"<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Thu, 28 May 2020 08:53:09 -0000"}},{"id":4927,"web_url":"https://patchwork.libcamera.org/comment/4927/","msgid":"<20200528100632.r2u6jpdgeukbzdml@uno.localdomain>","date":"2020-05-28T10:06:32","subject":"Re: [libcamera-devel] [PATCH v2 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 Laurent,\n\nOn Thu, May 28, 2020 at 03:55:41AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, May 27, 2020 at 11:47:34PM +0200, Jacopo Mondi wrote:\n> > On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:\n> > > On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > > > ---\n> > > >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> > > >  1 file changed, 14 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > index 77ebc3f9..42694fc7 100644\n> > > > --- a/src/libcamera/control_ids.yaml\n> > > > +++ b/src/libcamera/control_ids.yaml\n> > > > @@ -239,4 +239,18 @@ controls:\n> > > >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> > > >          control can only be returned in metadata.\n> > > >        size: [4]\n> > > > +\n> > > > +  - FrameDurations:\n> > > > +      type: float\n> > >\n> > > Do we need sub-microsecond precision, or would a uint32_t control work ?\n> > > I'd rather have a fixed precision if possible.\n> > >\n> > > > +      description: |\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> >\n> > Isn't it s/maximum frame duration/minimum frame duration/ ?\n> >\n> > If at least 30fps are requested, the exposure time cannot be longer\n> > than 33msec ? Am I reading this wrong ?\n>\n> I may read it wrong, but I think Naush is correct here. If a *maximum*\n> frame duration of 33ms is requested [that is, a *minimum* frame rate of\n> 30fps], the sensor will not be able to *raise* the exposure time above\n> 33ms.\n\nOuch, correct, sorry for the noise.\n\n>\n> > > > +        Note that the sensor may not always be able to provide the requested\n> > > > +        frame duration limits depending on its mode configuration.\n> >\n> > I've been really confused by this, until I realized I was having an\n> > hard time trying to figure out how to use this for applications to\n> > express a request for a precise duration and how should the library\n> > have used it to report the actual frame durtion in metadata. Until I\n> > realized this could probably be better interpreted not as a frame\n> > duration request or result, but as an hint to the AE algorithm to\n> > clamp its calculation into certain duration limtis.\n> >\n> > And I agree a duration range might be a good hint to drive the AE\n> > routine and if I got the rest of the series right, that's\n> > what happens in the IPA with these patches.\n> >\n> > If that's actually the intention, what about making of this an\n> > \"AeFrameDurationLimits\" control, to make it clear it applies to AE and\n> > use \"FrameDuration\" as an integer control to represent the requested\n> > and precise frame duration in requests and metadata respectively.\n>\n> I think (Ae)FrameDurationLimits makes sense as a control (and as a\n> property too, with the caveat that the minimum frame duration - highest\n> frame rate - can be dependent on the stream resolution), and\n> FrameDuration makes sense for metadata.\n>\n> More about FrameDuration as a control below.\n>\n> > On \"FrameDuration\" itself: would it make sense to only considered it\n> > when running with AE off ? In that case it would be applications\n> > responsibility to opportunely calculate the exposure time and the frame\n> > duration, but I think it's expected if you run in manual mode. And for\n> > reference, that's what Android does as well :)\n> >\n> > It would also rule out the need to specify how the frameDurationLimits\n> > would be used when running in manual mode and wanting to configure a\n> > precise value there, as I guess you would have to give to min and max\n> > the same value to express that.\n>\n>\n> > this might open pandora's box, but: is the frame duration a property of\n> > the camera, or of a single stream ?\n>\n> I've opened the same box in my previous e-mail :-) I think the frame\n> duration is a property of the camera, as all streams share the same\n> sensor, and are thus slave to the same frame rate. Some streams could\n> drop frames, but I think I would then report that as a decimation factor\n> per stream (not sure what the use cases would be though). This being\n\nI was actually more concerned about processing that might stall the\npipeline, like JPEG encoding.\n\n> said, the maximum frame rate (minimum frame duration) can depend on the\n> configuration of the camera, due to bandwidth limitations. It shouldn't\n> affect the (Ae)FrameDuration(Limits) control or metadata, but will\n> affect the (Ae)FrameDurationLimits property. It's something we need to\n> address eventually, not necessarily as part of this series, we could\n> start with a limit that doesn't depend on the camera configuration and\n> then build additional features on top.\n>\n\nI might actually need to fill that gap very soon as I need a way to\nreport the per-configuration frame duration interval. This is\nsomething not part of this series, but it's good if we at least\nstabilize on naming.\n\n> > > This looks good to me, but I'd like to discuss the corner cases I can\n> > > already think about.\n> > >\n> > > - Are there use cases for an application to specify a minimum frame\n> > >   duration only, or a maximum frame duration only (that is, any frame\n> > >   rate lower than a limit, or any frame rate higher than a limit) ? If\n> > >   so, how should that be specified ? We could set the minimum value to 0\n> > >   to mean that the maximum frame rate is unbounded, and the maximum\n> > >   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n> > >   Is there anything I'm overlooking ?\n> >\n> > If this becames an AE-related control and you specify that, I think\n> > you should provide both values. If you don't care about one of the two\n> > just use the values of the (forthcoming) property that reports the\n> > min and max frame durations of a camera. If an applications does not\n> > specify this control, the AE routine runs free, in the limits\n> > of the camera sensor capabilities.\n> >\n> > Does it make sense ?\n>\n> Yes that can work. Specifying anything below the minimum possible or\n> above the maximum possible value supported by the camera will be clamped\n> to the camera limits anyway.\n>\n\nTrue, we can clamp to those limits indeed\n\n> > > - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n> > >\n> > > - What happens if the requested frame duration isn't achievable ? Should\n> > >   we specify that the camera will use a frame duration as close to the\n> > >   requested range as possible ? Or could there be cases where a\n> > >   different behaviour would be needed ?\n> > >\n> > > - Not something we need to address now, but do you see any future\n> > >   relation between this control and anti-banding (50 or 60Hz flicker\n> > >   avoidance) ? Anti-banding will mostly restrict possible exposure\n> > >   times, and should only indirectly interact with the frame duration if\n> > >   I'm not mistaken, is that correct ?\n> > >\n> > > > +\n> > > > +        \\sa ExposureTime\n> > > > +      size: [2]\n> > > >  ...\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54C52603D0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 May 2020 12:03:15 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 9FE9940004;\n\tThu, 28 May 2020 10:03:14 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 28 May 2020 12:06:32 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200528100632.r2u6jpdgeukbzdml@uno.localdomain>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200528005541.GB4670@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Thu, 28 May 2020 10:03:15 -0000"}},{"id":4949,"web_url":"https://patchwork.libcamera.org/comment/4949/","msgid":"<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>","date":"2020-06-01T15:43:22","subject":"Re: [libcamera-devel] [PATCH v2 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, May 28, 2020 at 09:52:51AM +0100, Naushir Patuck wrote:\n> Hi Laurent and Jacopo,\n>\n> I'll address the comments from both emails jointly below:\n>\n> On Thu, 28 May 2020 at 01:55, Laurent Pinchart\n> <laurent.pinchart@ideasonboard.com> wrote:\n> >\n> > Hi Jacopo,\n> >\n> > On Wed, May 27, 2020 at 11:47:34PM +0200, Jacopo Mondi wrote:\n> > > On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:\n> > > > On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > > > > ---\n> > > > >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> > > > >  1 file changed, 14 insertions(+)\n> > > > >\n> > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > > index 77ebc3f9..42694fc7 100644\n> > > > > --- a/src/libcamera/control_ids.yaml\n> > > > > +++ b/src/libcamera/control_ids.yaml\n> > > > > @@ -239,4 +239,18 @@ controls:\n> > > > >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> > > > >          control can only be returned in metadata.\n> > > > >        size: [4]\n> > > > > +\n> > > > > +  - FrameDurations:\n> > > > > +      type: float\n> > > >\n> > > > Do we need sub-microsecond precision, or would a uint32_t control work ?\n> > > > I'd rather have a fixed precision if possible.\n> > > >\n> > > > > +      description: |\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> > >\n> > > Isn't it s/maximum frame duration/minimum frame duration/ ?\n> > >\n> > > If at least 30fps are requested, the exposure time cannot be longer\n> > > than 33msec ? Am I reading this wrong ?\n> >\n> > I may read it wrong, but I think Naush is correct here. If a *maximum*\n> > frame duration of 33ms is requested [that is, a *minimum* frame rate of\n> > 30fps], the sensor will not be able to *raise* the exposure time above\n> > 33ms.\n> >\n>\n> Yes, my wording in the description matches Laurent's interpretation.\n> I must admit, I do find it more difficult talking about frame\n> durations vs framerates :)\n>\n> > > > > +        Note that the sensor may not always be able to provide the requested\n> > > > > +        frame duration limits depending on its mode configuration.\n> > >\n> > > I've been really confused by this, until I realized I was having an\n> > > hard time trying to figure out how to use this for applications to\n> > > express a request for a precise duration and how should the library\n> > > have used it to report the actual frame durtion in metadata. Until I\n> > > realized this could probably be better interpreted not as a frame\n> > > duration request or result, but as an hint to the AE algorithm to\n> > > clamp its calculation into certain duration limtis.\n> > >\n> > > And I agree a duration range might be a good hint to drive the AE\n> > > routine and if I got the rest of the series right, that's\n> > > what happens in the IPA with these patches.\n> > >\n> > > If that's actually the intention, what about making of this an\n> > > \"AeFrameDurationLimits\" control, to make it clear it applies to AE and\n> > > use \"FrameDuration\" as an integer control to represent the requested\n> > > and precise frame duration in requests and metadata respectively.\n> >\n> > I think (Ae)FrameDurationLimits makes sense as a control (and as a\n> > property too, with the caveat that the minimum frame duration - highest\n> > frame rate - can be dependent on the stream resolution), and\n> > FrameDuration makes sense for metadata.\n>\n> Good points here.  When doing this change, I purposely did not want to\n> directly tie this control with AE for a few of reasons, even though\n> there is an interaction:\n>\n> 1) Even though the FrameDurationLimits does restrict what the AE can\n> ask for, in our IPA, they are two distinct and separate operations.\n> In fact, our AE does not even see the frameduration limits that the\n> user set.  The results of AE just get clipped by the\n> FrameDurationLimits.  This is possibly sub-optimal (see my last email\n> for the discussion), but still gives the desired exposure.  In future\n> we will probably optimise this, but given the complexity, will leave\n> it for another change.\n\nNot sure I got this: \"AE just get clipped by the FrameDuration\"\ndoesn't mean that this is an AE-tuning parameter ?\n\nIdeally, I think we should have both an \"AE frame duration limits\" and\nan \"AE exposure limits\" to allow applications (with the help of some\npre-cooked profile possibily) to implement the canonical\nshutter-priority and exposure-priority modes..\n\n> 2) AE does have a way to clip requested shutter speeds using the\n> \"Exposure Profile\" tuning parameters which offers another degree of\n> freedom to control the interaction of shutter speed with analogue\n> gain.  FrameDurationLimits only controls shutter speed which may not\n> be desirable.\n\nCould this be addressed with an AE-exposure-priority control ?\n\n>\n\nI'm not sure, based on your last points, what's your view on this\ncontrol future developments. Should this stay \"frameDuration\" or\nshould this become AeFrameDurationLimit (or AeTargetDuration or\nwhatever else appropriate) ?\n\n> >\n> > More about FrameDuration as a control below.\n> >\n> > > On \"FrameDuration\" itself: would it make sense to only considered it\n> > > when running with AE off ? In that case it would be applications\n> > > responsibility to opportunely calculate the exposure time and the frame\n> > > duration, but I think it's expected if you run in manual mode. And for\n> > > reference, that's what Android does as well :)\n> > >\n> > > It would also rule out the need to specify how the frameDurationLimits\n> > > would be used when running in manual mode and wanting to configure a\n> > > precise value there, as I guess you would have to give to min and max\n> > > the same value to express that.\n>\n> FrameDurationLimits should only really apply with AE enabled.  AE\n> varies the shutter speed all over the place, that could cause\n> framerate changes that we need to clip.  If AE is disabled and we have\n> a manual shutter speed set, that dictates the frame duration, and any\n> limits will not apply.  Perhaps I should add this in the description?\n>\n\nIt really depends what developments you foresee here, if the control\nbecomes an AE-relatd one I think it's not necessary to specify that.\n\nAll in all, to me it seems like \"frameDuration\" should be reserved for\nmanual duration control and to report the actual duration of the\ncaptured frames.\n\nAs I'm working on reporting the camera frame duration limits, synching\non naming and semantic associated with the control could help both of\nus progressing knowing we won't step on each other toes...\n\nThanks\n  j\n\n\n> >\n> >\n> > > this might open pandora's box, but: is the frame duration a property of\n> > > the camera, or of a single stream ?\n> >\n> > I've opened the same box in my previous e-mail :-) I think the frame\n> > duration is a property of the camera, as all streams share the same\n> > sensor, and are thus slave to the same frame rate. Some streams could\n> > drop frames, but I think I would then report that as a decimation factor\n> > per stream (not sure what the use cases would be though). This being\n> > said, the maximum frame rate (minimum frame duration) can depend on the\n> > configuration of the camera, due to bandwidth limitations. It shouldn't\n> > affect the (Ae)FrameDuration(Limits) control or metadata, but will\n> > affect the (Ae)FrameDurationLimits property. It's something we need to\n> > address eventually, not necessarily as part of this series, we could\n> > start with a limit that doesn't depend on the camera configuration and\n> > then build additional features on top.\n>\n> Agreed.\n>\n> >\n> > > > This looks good to me, but I'd like to discuss the corner cases I can\n> > > > already think about.\n> > > >\n> > > > - Are there use cases for an application to specify a minimum frame\n> > > >   duration only, or a maximum frame duration only (that is, any frame\n> > > >   rate lower than a limit, or any frame rate higher than a limit) ? If\n> > > >   so, how should that be specified ? We could set the minimum value to 0\n> > > >   to mean that the maximum frame rate is unbounded, and the maximum\n> > > >   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n> > > >   Is there anything I'm overlooking ?\n> > >\n> > > If this becames an AE-related control and you specify that, I think\n> > > you should provide both values. If you don't care about one of the two\n> > > just use the values of the (forthcoming) property that reports the\n> > > min and max frame durations of a camera. If an applications does not\n> > > specify this control, the AE routine runs free, in the limits\n> > > of the camera sensor capabilities.\n> > >\n> > > Does it make sense ?\n> >\n> > Yes that can work. Specifying anything below the minimum possible or\n> > above the maximum possible value supported by the camera will be clamped\n> > to the camera limits anyway.\n> >\n> > > > - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n> > > >\n> > > > - What happens if the requested frame duration isn't achievable ? Should\n> > > >   we specify that the camera will use a frame duration as close to the\n> > > >   requested range as possible ? Or could there be cases where a\n> > > >   different behaviour would be needed ?\n> > > >\n> > > > - Not something we need to address now, but do you see any future\n> > > >   relation between this control and anti-banding (50 or 60Hz flicker\n> > > >   avoidance) ? Anti-banding will mostly restrict possible exposure\n> > > >   times, and should only indirectly interact with the frame duration if\n> > > >   I'm not mistaken, is that correct ?\n> > > >\n> > > > > +\n> > > > > +        \\sa ExposureTime\n> > > > > +      size: [2]\n> > > > >  ...\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n>\n> Regards,\n> Naush","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56893603CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jun 2020 17:40:04 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 1C4D14000B;\n\tMon,  1 Jun 2020 15:40:02 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 1 Jun 2020 17:43:22 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>\n\t<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Mon, 01 Jun 2020 15:40:04 -0000"}},{"id":4955,"web_url":"https://patchwork.libcamera.org/comment/4955/","msgid":"<CAEmqJPokD1n3J78SoxRX3D_0bjCWm4cMK5aC_E4j=R6ACyB6ug@mail.gmail.com>","date":"2020-06-02T09:38:41","subject":"Re: [libcamera-devel] [PATCH v2 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 Mon, 1 Jun 2020 at 16:40, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi Naush,\n>\n> On Thu, May 28, 2020 at 09:52:51AM +0100, Naushir Patuck wrote:\n> > Hi Laurent and Jacopo,\n> >\n> > I'll address the comments from both emails jointly below:\n> >\n> > On Thu, 28 May 2020 at 01:55, Laurent Pinchart\n> > <laurent.pinchart@ideasonboard.com> wrote:\n> > >\n> > > Hi Jacopo,\n> > >\n> > > On Wed, May 27, 2020 at 11:47:34PM +0200, Jacopo Mondi wrote:\n> > > > On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:\n> > > > > On Wed, May 13, 2020 at 10:11:18AM +0100, 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> > > > > > ---\n> > > > > >  src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> > > > > >  1 file changed, 14 insertions(+)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > > > index 77ebc3f9..42694fc7 100644\n> > > > > > --- a/src/libcamera/control_ids.yaml\n> > > > > > +++ b/src/libcamera/control_ids.yaml\n> > > > > > @@ -239,4 +239,18 @@ controls:\n> > > > > >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels\n> > > > > >          control can only be returned in metadata.\n> > > > > >        size: [4]\n> > > > > > +\n> > > > > > +  - FrameDurations:\n> > > > > > +      type: float\n> > > > >\n> > > > > Do we need sub-microsecond precision, or would a uint32_t control work ?\n> > > > > I'd rather have a fixed precision if possible.\n> > > > >\n> > > > > > +      description: |\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> > > >\n> > > > Isn't it s/maximum frame duration/minimum frame duration/ ?\n> > > >\n> > > > If at least 30fps are requested, the exposure time cannot be longer\n> > > > than 33msec ? Am I reading this wrong ?\n> > >\n> > > I may read it wrong, but I think Naush is correct here. If a *maximum*\n> > > frame duration of 33ms is requested [that is, a *minimum* frame rate of\n> > > 30fps], the sensor will not be able to *raise* the exposure time above\n> > > 33ms.\n> > >\n> >\n> > Yes, my wording in the description matches Laurent's interpretation.\n> > I must admit, I do find it more difficult talking about frame\n> > durations vs framerates :)\n> >\n> > > > > > +        Note that the sensor may not always be able to provide the requested\n> > > > > > +        frame duration limits depending on its mode configuration.\n> > > >\n> > > > I've been really confused by this, until I realized I was having an\n> > > > hard time trying to figure out how to use this for applications to\n> > > > express a request for a precise duration and how should the library\n> > > > have used it to report the actual frame durtion in metadata. Until I\n> > > > realized this could probably be better interpreted not as a frame\n> > > > duration request or result, but as an hint to the AE algorithm to\n> > > > clamp its calculation into certain duration limtis.\n> > > >\n> > > > And I agree a duration range might be a good hint to drive the AE\n> > > > routine and if I got the rest of the series right, that's\n> > > > what happens in the IPA with these patches.\n> > > >\n> > > > If that's actually the intention, what about making of this an\n> > > > \"AeFrameDurationLimits\" control, to make it clear it applies to AE and\n> > > > use \"FrameDuration\" as an integer control to represent the requested\n> > > > and precise frame duration in requests and metadata respectively.\n> > >\n> > > I think (Ae)FrameDurationLimits makes sense as a control (and as a\n> > > property too, with the caveat that the minimum frame duration - highest\n> > > frame rate - can be dependent on the stream resolution), and\n> > > FrameDuration makes sense for metadata.\n> >\n> > Good points here.  When doing this change, I purposely did not want to\n> > directly tie this control with AE for a few of reasons, even though\n> > there is an interaction:\n> >\n> > 1) Even though the FrameDurationLimits does restrict what the AE can\n> > ask for, in our IPA, they are two distinct and separate operations.\n> > In fact, our AE does not even see the frameduration limits that the\n> > user set.  The results of AE just get clipped by the\n> > FrameDurationLimits.  This is possibly sub-optimal (see my last email\n> > for the discussion), but still gives the desired exposure.  In future\n> > we will probably optimise this, but given the complexity, will leave\n> > it for another change.\n>\n> Not sure I got this: \"AE just get clipped by the FrameDuration\"\n> doesn't mean that this is an AE-tuning parameter ?\n\nRoughly speaking, the following sequence of events occur in the RPi\nIPA (per frame) for AE calculations:\n\n1) AE gets given the current frame shutter speed and gain.\n2) If the current exposure does not have the required exposure for the\ntarget brightness, it sets up a digital gain in the ISP to make up the\ndifference.\n3) It also generates a new shutter speed and gain value to program the\nsensor to achieve the target brightness without digital gain.  This is\ndone with no knowledge of what the maximum achievable exposure is\n(based on the FrameDuration).\n4) In the IPA, we look at this shutter speed and gain value requested\nby the AE.  We clip the shutter speed based on what user requested\nFrameDuration was provided (if any).\n\nSo even though AE results do get clipped by the FrameDuration setting,\nAE has no knowledge about this.  In the case of manual exposures (see\nbelow for a bit more context), the FrameDuration clipping will still\napply exactly like above.  Hence my choosing to call this control\nFrameDuration to decouple it from what the AE does.  However, I am\nhappy to change things and rename FrameDuration -> AeFrameDuration (or\nAeFrameDurationLimits?) if the consensus is there.\n\n>\n> Ideally, I think we should have both an \"AE frame duration limits\" and\n> an \"AE exposure limits\" to allow applications (with the help of some\n> pre-cooked profile possibily) to implement the canonical\n> shutter-priority and exposure-priority modes..\n>\n> > 2) AE does have a way to clip requested shutter speeds using the\n> > \"Exposure Profile\" tuning parameters which offers another degree of\n> > freedom to control the interaction of shutter speed with analogue\n> > gain.  FrameDurationLimits only controls shutter speed which may not\n> > be desirable.\n>\n> Could this be addressed with an AE-exposure-priority control ?\n\nYou are right that we do need controls for both.  The RPi IPA does\nhave both controls.  For  \"AE frame duration limits\", we use\nFrameDuration (or AeFrameDuration depending on what we decide).  For\n\"AE exposure limits\", we use the control AeExposureMode for exactly\nthis.\n\n>\n> >\n>\n> I'm not sure, based on your last points, what's your view on this\n> control future developments. Should this stay \"frameDuration\" or\n> should this become AeFrameDurationLimit (or AeTargetDuration or\n> whatever else appropriate) ?\n\nSee below.\n\n>\n> > >\n> > > More about FrameDuration as a control below.\n> > >\n> > > > On \"FrameDuration\" itself: would it make sense to only considered it\n> > > > when running with AE off ? In that case it would be applications\n> > > > responsibility to opportunely calculate the exposure time and the frame\n> > > > duration, but I think it's expected if you run in manual mode. And for\n> > > > reference, that's what Android does as well :)\n> > > >\n> > > > It would also rule out the need to specify how the frameDurationLimits\n> > > > would be used when running in manual mode and wanting to configure a\n> > > > precise value there, as I guess you would have to give to min and max\n> > > > the same value to express that.\n> >\n> > FrameDurationLimits should only really apply with AE enabled.  AE\n> > varies the shutter speed all over the place, that could cause\n> > framerate changes that we need to clip.  If AE is disabled and we have\n> > a manual shutter speed set, that dictates the frame duration, and any\n> > limits will not apply.  Perhaps I should add this in the description?\n> >\n\nI have to correct myself here.  FrameDurationLimits will also apply\nwith manual shutter speeds when AE is disabled.  We could, say, have a\nmanual shutter speed request of 25 ms, but still want to run at 30fps.\nThere is a question on what happens if they clash, e.g. request a\nmanual shutter speed of 66ms, and a FrameDuration of 30fps?  What gets\nprioritised?  Currently, in our implementation, FrameDuration will\nalways be prioritised.\n\n>\n> It really depends what developments you foresee here, if the control\n> becomes an AE-relatd one I think it's not necessary to specify that.\n>\n> All in all, to me it seems like \"frameDuration\" should be reserved for\n> manual duration control and to report the actual duration of the\n> captured frames.\n\nSo circling back to your earlier question:\n> Should this stay \"frameDuration\" or\n> should this become AeFrameDurationLimit (or AeTargetDuration or\n> whatever else appropriate) ?\n\nGiven that I think FrameDurationLimits might apply in both AE and\nnon-AE operations equally, I think we should keep the name as-is.\nHowever, again, if the consensus is to change, I am more than happy to\nmake the change.\n\n>\n> As I'm working on reporting the camera frame duration limits, synching\n> on naming and semantic associated with the control could help both of\n> us progressing knowing we won't step on each other toes...\n\nAgreed, we should be consistent on what we decide.  Please do let me\nknow what the consensus is and I will be happy to follow it.\n\nRegards,\nNaush\n\n\n> Thanks\n>   j\n>\n>\n> > >\n> > >\n> > > > this might open pandora's box, but: is the frame duration a property of\n> > > > the camera, or of a single stream ?\n> > >\n> > > I've opened the same box in my previous e-mail :-) I think the frame\n> > > duration is a property of the camera, as all streams share the same\n> > > sensor, and are thus slave to the same frame rate. Some streams could\n> > > drop frames, but I think I would then report that as a decimation factor\n> > > per stream (not sure what the use cases would be though). This being\n> > > said, the maximum frame rate (minimum frame duration) can depend on the\n> > > configuration of the camera, due to bandwidth limitations. It shouldn't\n> > > affect the (Ae)FrameDuration(Limits) control or metadata, but will\n> > > affect the (Ae)FrameDurationLimits property. It's something we need to\n> > > address eventually, not necessarily as part of this series, we could\n> > > start with a limit that doesn't depend on the camera configuration and\n> > > then build additional features on top.\n> >\n> > Agreed.\n> >\n> > >\n> > > > > This looks good to me, but I'd like to discuss the corner cases I can\n> > > > > already think about.\n> > > > >\n> > > > > - Are there use cases for an application to specify a minimum frame\n> > > > >   duration only, or a maximum frame duration only (that is, any frame\n> > > > >   rate lower than a limit, or any frame rate higher than a limit) ? If\n> > > > >   so, how should that be specified ? We could set the minimum value to 0\n> > > > >   to mean that the maximum frame rate is unbounded, and the maximum\n> > > > >   value to UINT32_MAX to mean that the minimum frame rate is unbounded.\n> > > > >   Is there anything I'm overlooking ?\n> > > >\n> > > > If this becames an AE-related control and you specify that, I think\n> > > > you should provide both values. If you don't care about one of the two\n> > > > just use the values of the (forthcoming) property that reports the\n> > > > min and max frame durations of a camera. If an applications does not\n> > > > specify this control, the AE routine runs free, in the limits\n> > > > of the camera sensor capabilities.\n> > > >\n> > > > Does it make sense ?\n> > >\n> > > Yes that can work. Specifying anything below the minimum possible or\n> > > above the maximum possible value supported by the camera will be clamped\n> > > to the camera limits anyway.\n> > >\n> > > > > - Is 0 an acceptable value ? Or should 1 be the minimum value ?\n> > > > >\n> > > > > - What happens if the requested frame duration isn't achievable ? Should\n> > > > >   we specify that the camera will use a frame duration as close to the\n> > > > >   requested range as possible ? Or could there be cases where a\n> > > > >   different behaviour would be needed ?\n> > > > >\n> > > > > - Not something we need to address now, but do you see any future\n> > > > >   relation between this control and anti-banding (50 or 60Hz flicker\n> > > > >   avoidance) ? Anti-banding will mostly restrict possible exposure\n> > > > >   times, and should only indirectly interact with the frame duration if\n> > > > >   I'm not mistaken, is that correct ?\n> > > > >\n> > > > > > +\n> > > > > > +        \\sa ExposureTime\n> > > > > > +      size: [2]\n> > > > > >  ...\n> > >\n> > > --\n> > > Regards,\n> > >\n> > > Laurent Pinchart\n> >\n> > Regards,\n> > Naush","headers":{"Return-Path":"<naush@raspberrypi.com>","Received":["from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 895A6603CA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Jun 2020 11:39:00 +0200 (CEST)","by mail-lf1-x134.google.com with SMTP id 82so5760894lfh.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 02 Jun 2020 02:39:00 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"Qsh7iV87\"; 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=FcXkWXUZNqwPyfxX88m+P5L/dFYjkIXY2fbudgy7ASc=;\n\tb=Qsh7iV876HyHJjVTLZoMFRE2Mx/VcSo5sGug0/cz1/AoeuIlj9pzLGDOuXEKjHJozS\n\tBITtwmxsY0DacUKE1MKod8pSqJndn/RP6BaUvnIT1ewGAGNs2n9kqFVLwM8//2BZky03\n\tFZXqyhNSE2F37kuMeW7S54tq3xuhDy1UyGIevX/uNHaTpCl1cvMKOVfYh4u769g1oROO\n\tESQ05k9vIkQONe1oUiVNQ7KWzRZZHiRceghYj+PGEWfgQ2e91488e4CKYd5mXYjMt8+s\n\tUhqXx7jaWwHeP4U/JAZarEYOO5+iCNTdshpC61+aSDQ7wL+JBo1NiRckLp3tHdRTZ8BH\n\tEywA==","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=FcXkWXUZNqwPyfxX88m+P5L/dFYjkIXY2fbudgy7ASc=;\n\tb=jYBfagd/D7ip9c0VdxeI4Uapih6UvpJuvbA+gyPGNjNHZVICj97X4hP8qQEm3vaKhj\n\tFW42iz11zwefEAFVbrSYZyJWck1WF3LRc2444JIigsstzI6DefUqGzC55DX7C3VEy9Kg\n\t+8NhbG6JW8DtH9JqG/RCMEjysz9j6yrYU1BWk4eGBttI3h9Rp/rUGLq2zXSDKje2PZTG\n\tdUnoeZyk0twPJp4mdInFLIuEAq/H4pqew3UuliOf0M7QvA8IdTD/rgcD8uWnHO14fV/r\n\tey1iRlHV/S2yNafdkjnb+nZ8Ezn1XJcT+9KTRGpYdDqaWX6mh5a7fC969UlqTrjwPWZk\n\tQ21A==","X-Gm-Message-State":"AOAM53133aS9xKU/yMrUXV/jKPtXZfseZ97cRqMrgA+srk602Z0FqYYp\n\tbNcyfCG/h6VXoA733G06K7f0WA1ZIZb25I7PrI/OFw==","X-Google-Smtp-Source":"ABdhPJy8+K6Yv67GoxwFjAr5a7OXmSKax8nzWRLxYcW2qneRCBFSyxzkCLwVWqpTzSweLUAHuZbohUNkywaF0I7nk0I=","X-Received":"by 2002:ac2:44bb:: with SMTP id\n\tc27mr13909273lfm.59.1591090739366; \n\tTue, 02 Jun 2020 02:38:59 -0700 (PDT)","MIME-Version":"1.0","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>\n\t<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>\n\t<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>","In-Reply-To":"<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Tue, 2 Jun 2020 10:38:41 +0100","Message-ID":"<CAEmqJPokD1n3J78SoxRX3D_0bjCWm4cMK5aC_E4j=R6ACyB6ug@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Tue, 02 Jun 2020 09:39:00 -0000"}},{"id":4978,"web_url":"https://patchwork.libcamera.org/comment/4978/","msgid":"<20200603084535.ljtwgwsgieonicxi@uno.localdomain>","date":"2020-06-03T08:45:35","subject":"Re: [libcamera-devel] [PATCH v2 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\n    I won't go into details here as I would like Laurent to chime in\nand express his view on this topic, but I would have a question in the\nmeantime.\n\nOn Tue, Jun 02, 2020 at 10:38:41AM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n>\n> On Mon, 1 Jun 2020 at 16:40, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > Hi Naush,\n> >\n> > On Thu, May 28, 2020 at 09:52:51AM +0100, Naushir Patuck wrote:\n\n[snip]\n\n> > > > > On \"FrameDuration\" itself: would it make sense to only considered it\n> > > > > when running with AE off ? In that case it would be applications\n> > > > > responsibility to opportunely calculate the exposure time and the frame\n> > > > > duration, but I think it's expected if you run in manual mode. And for\n> > > > > reference, that's what Android does as well :)\n> > > > >\n> > > > > It would also rule out the need to specify how the frameDurationLimits\n> > > > > would be used when running in manual mode and wanting to configure a\n> > > > > precise value there, as I guess you would have to give to min and max\n> > > > > the same value to express that.\n> > >\n> > > FrameDurationLimits should only really apply with AE enabled.  AE\n> > > varies the shutter speed all over the place, that could cause\n> > > framerate changes that we need to clip.  If AE is disabled and we have\n> > > a manual shutter speed set, that dictates the frame duration, and any\n> > > limits will not apply.  Perhaps I should add this in the description?\n> > >\n>\n> I have to correct myself here.  FrameDurationLimits will also apply\n> with manual shutter speeds when AE is disabled.  We could, say, have a\n> manual shutter speed request of 25 ms, but still want to run at 30fps.\n> There is a question on what happens if they clash, e.g. request a\n> manual shutter speed of 66ms, and a FrameDuration of 30fps?  What gets\n> prioritised?  Currently, in our implementation, FrameDuration will\n> always be prioritised.\n\nIs there a rationle behind this decision ? I'm asking as I see Android\ngoing in the other direction, with the exposure time always overriding\nthe requested frame duration.\n\nIt is specified in their documentation that the maximum frame duration\na camera reports (lower frame rate) shall be at least the maximum\nallowed exposure time, so that all possible exposure values are known\nto be in the requested FPS range. I guess this should be enforced even\nif we give frame duration priority, or should we cap the maximum frame\nduration to the maximum available exposure time ?\n\nJust trying to get more data points to make an informed decision, as\nyour question on what has to be prioritized is quite sensible...\n\nThanks\n  j","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 38369603C9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jun 2020 10:42:17 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 514DA200020;\n\tWed,  3 Jun 2020 08:42:15 +0000 (UTC)"],"Date":"Wed, 3 Jun 2020 10:45:35 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200603084535.ljtwgwsgieonicxi@uno.localdomain>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>\n\t<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>\n\t<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>\n\t<CAEmqJPokD1n3J78SoxRX3D_0bjCWm4cMK5aC_E4j=R6ACyB6ug@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPokD1n3J78SoxRX3D_0bjCWm4cMK5aC_E4j=R6ACyB6ug@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Wed, 03 Jun 2020 08:42:17 -0000"}},{"id":4979,"web_url":"https://patchwork.libcamera.org/comment/4979/","msgid":"<CAEmqJPoJrQnVGzsnm9a+N1aHuTaekWMv-Fzo8FEZGsTeoXLiwg@mail.gmail.com>","date":"2020-06-03T09:44:18","subject":"Re: [libcamera-devel] [PATCH v2 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\nOn Wed, 3 Jun 2020 at 09:42, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi Naush,\n>\n>     I won't go into details here as I would like Laurent to chime in\n> and express his view on this topic, but I would have a question in the\n> meantime.\n>\n> On Tue, Jun 02, 2020 at 10:38:41AM +0100, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> >\n> > On Mon, 1 Jun 2020 at 16:40, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > >\n> > > Hi Naush,\n> > >\n> > > On Thu, May 28, 2020 at 09:52:51AM +0100, Naushir Patuck wrote:\n>\n> [snip]\n>\n> > > > > > On \"FrameDuration\" itself: would it make sense to only considered it\n> > > > > > when running with AE off ? In that case it would be applications\n> > > > > > responsibility to opportunely calculate the exposure time and the frame\n> > > > > > duration, but I think it's expected if you run in manual mode. And for\n> > > > > > reference, that's what Android does as well :)\n> > > > > >\n> > > > > > It would also rule out the need to specify how the frameDurationLimits\n> > > > > > would be used when running in manual mode and wanting to configure a\n> > > > > > precise value there, as I guess you would have to give to min and max\n> > > > > > the same value to express that.\n> > > >\n> > > > FrameDurationLimits should only really apply with AE enabled.  AE\n> > > > varies the shutter speed all over the place, that could cause\n> > > > framerate changes that we need to clip.  If AE is disabled and we have\n> > > > a manual shutter speed set, that dictates the frame duration, and any\n> > > > limits will not apply.  Perhaps I should add this in the description?\n> > > >\n> >\n> > I have to correct myself here.  FrameDurationLimits will also apply\n> > with manual shutter speeds when AE is disabled.  We could, say, have a\n> > manual shutter speed request of 25 ms, but still want to run at 30fps.\n> > There is a question on what happens if they clash, e.g. request a\n> > manual shutter speed of 66ms, and a FrameDuration of 30fps?  What gets\n> > prioritised?  Currently, in our implementation, FrameDuration will\n> > always be prioritised.\n>\n> Is there a rationle behind this decision ? I'm asking as I see Android\n> going in the other direction, with the exposure time always overriding\n> the requested frame duration.\n\nThis was an entirely arbitrary choice on my part - based on what was\neasiest to do at the time :)\n\n> It is specified in their documentation that the mximum frame duration\n> a camera reports (lower frame rate) shall be at least the maximum\n> allowed exposure time, so that all possible exposure values are known\n> to be in the requested FPS range. I guess this should be enforced even\n> if we give frame duration priority, or should we cap the maximum frame\n> duration to the maximum available exposure time ?\n\nI have not looked at the Android docs in ages, but do they have two\nsets of frame duration limits, one that the sensor will report (sounds\nlike this is what you are working on?) and one that the user requests\n(of course, bounded by the former)?  This was where my thinking was\ngoing, and with frame duration priority, exposure time is capped by\nthe user request (if given), or the sensor limits.  But as always, if\nthe consensus is to behave differently, I am happy to change things\naround.\n\nRegards,\nNaush\n\n\n>\n> Just trying to get more data points to make an informed decision, as\n> your question on what has to be prioritized is quite sensible...\n>\n> Thanks\n>   j","headers":{"Return-Path":"<naush@raspberrypi.com>","Received":["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 8CBC5603C9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jun 2020 11:44:36 +0200 (CEST)","by mail-lj1-x235.google.com with SMTP id b6so1908641ljj.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 03 Jun 2020 02:44:36 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"QSRqDQak\"; 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=KnZ15ny4xZSNG8/V8NDzDZmS9Afjn09aAHK9C8uOVVc=;\n\tb=QSRqDQakJjNXE+o4HJXlWFgc9PVCy+amwIjq53yp2wM8HLlvAEucqL7TgSeAAlICLY\n\tAJWYljpL7+9zLznL3S5a+DQR3brzDVytvj4N4cX84YjLBGEKfwF4CHwT/ICAxK/F33zf\n\txGaRrn5OQ0RkJJLRotOMr499K7Nk/lBCslDa5SfoI+DT1ILvCP/0Tgg3zQYrxNurXnw0\n\tkGr5A5U4lsEqJb5CHtOQLNZ7w98SALl4A9eZMSpoPcJUf2tZHIFiizXtWq+/LojYndf+\n\tg+VdiT4RgO1tt0n4UsAH14cpVtCg9jps51mpuytebixnRbiePZoPKJwSfBWXeAowvDn0\n\tG25w==","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=KnZ15ny4xZSNG8/V8NDzDZmS9Afjn09aAHK9C8uOVVc=;\n\tb=hwut9BUVoN02GhC5XliH+vvhujBlLzvLX5AjqxqGRWf06HdIzlbUbq1XxAiuE5K034\n\t9UZD7pyBlezlbNzYcQfpBw/obmqXUhoXfblBELaoecaeco+sn5Je1nsANyrTzEd+agta\n\tupBGAENJ138jUOrAfhCsluwsyLwJ6XNcpALMV/xPYxXSCzjs+ca7iyWkWhSkNr5bfBfY\n\tRHmgEuY2cFKiZ3Ep7udtmBQIYbuDEZg9GMx/iAmD8xVZ8Wi77VDRmHpSjdSpwpZ6bmDE\n\tlxKYQqrQOPH7RtuTTA52D/dpHnB3VnhOpBnT5A28h4gEuZ/N0rhXJMIZfW3F327XuImx\n\tFUKQ==","X-Gm-Message-State":"AOAM533R1/V7Xub4GnUdZoHlj7bjoydzeOjv9muZFRClGO3wkQ+51zzr\n\tgtcMIq3IjsHqdiw3Fn9pWS2jFeRzzWFTOpIofcrs/uoA6dU=","X-Google-Smtp-Source":"ABdhPJypYSjEEEEOeCp+dmdHA1YISnFHLNos+y/s9x8xa5hYS+G5cXWvWQ5j+WIDxrFCgs3UD474wvYRI6ZTT3vbgiI=","X-Received":"by 2002:a2e:98d4:: with SMTP id s20mr1023432ljj.83.1591177475534;\n\tWed, 03 Jun 2020 02:44:35 -0700 (PDT)","MIME-Version":"1.0","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>\n\t<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>\n\t<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>\n\t<CAEmqJPokD1n3J78SoxRX3D_0bjCWm4cMK5aC_E4j=R6ACyB6ug@mail.gmail.com>\n\t<20200603084535.ljtwgwsgieonicxi@uno.localdomain>","In-Reply-To":"<20200603084535.ljtwgwsgieonicxi@uno.localdomain>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Wed, 3 Jun 2020 10:44:18 +0100","Message-ID":"<CAEmqJPoJrQnVGzsnm9a+N1aHuTaekWMv-Fzo8FEZGsTeoXLiwg@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Wed, 03 Jun 2020 09:44:36 -0000"}},{"id":4980,"web_url":"https://patchwork.libcamera.org/comment/4980/","msgid":"<20200603101429.6xqtp2xsvgoeuel3@uno.localdomain>","date":"2020-06-03T10:14:29","subject":"Re: [libcamera-devel] [PATCH v2 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 Wed, Jun 03, 2020 at 10:44:18AM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> On Wed, 3 Jun 2020 at 09:42, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > Hi Naush,\n> >\n> >     I won't go into details here as I would like Laurent to chime in\n> > and express his view on this topic, but I would have a question in the\n> > meantime.\n> >\n> > On Tue, Jun 02, 2020 at 10:38:41AM +0100, Naushir Patuck wrote:\n> > > Hi Jacopo,\n> > >\n> > >\n> > > On Mon, 1 Jun 2020 at 16:40, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > > >\n> > > > Hi Naush,\n> > > >\n> > > > On Thu, May 28, 2020 at 09:52:51AM +0100, Naushir Patuck wrote:\n> >\n> > [snip]\n> >\n> > > > > > > On \"FrameDuration\" itself: would it make sense to only considered it\n> > > > > > > when running with AE off ? In that case it would be applications\n> > > > > > > responsibility to opportunely calculate the exposure time and the frame\n> > > > > > > duration, but I think it's expected if you run in manual mode. And for\n> > > > > > > reference, that's what Android does as well :)\n> > > > > > >\n> > > > > > > It would also rule out the need to specify how the frameDurationLimits\n> > > > > > > would be used when running in manual mode and wanting to configure a\n> > > > > > > precise value there, as I guess you would have to give to min and max\n> > > > > > > the same value to express that.\n> > > > >\n> > > > > FrameDurationLimits should only really apply with AE enabled.  AE\n> > > > > varies the shutter speed all over the place, that could cause\n> > > > > framerate changes that we need to clip.  If AE is disabled and we have\n> > > > > a manual shutter speed set, that dictates the frame duration, and any\n> > > > > limits will not apply.  Perhaps I should add this in the description?\n> > > > >\n> > >\n> > > I have to correct myself here.  FrameDurationLimits will also apply\n> > > with manual shutter speeds when AE is disabled.  We could, say, have a\n> > > manual shutter speed request of 25 ms, but still want to run at 30fps.\n> > > There is a question on what happens if they clash, e.g. request a\n> > > manual shutter speed of 66ms, and a FrameDuration of 30fps?  What gets\n> > > prioritised?  Currently, in our implementation, FrameDuration will\n> > > always be prioritised.\n> >\n> > Is there a rationle behind this decision ? I'm asking as I see Android\n> > going in the other direction, with the exposure time always overriding\n> > the requested frame duration.\n>\n> This was an entirely arbitrary choice on my part - based on what was\n> easiest to do at the time :)\n>\n> > It is specified in their documentation that the mximum frame duration\n> > a camera reports (lower frame rate) shall be at least the maximum\n> > allowed exposure time, so that all possible exposure values are known\n> > to be in the requested FPS range. I guess this should be enforced even\n> > if we give frame duration priority, or should we cap the maximum frame\n> > duration to the maximum available exposure time ?\n>\n> I have not looked at the Android docs in ages, but do they have two\n> sets of frame duration limits, one that the sensor will report (sounds\n> like this is what you are working on?) and one that the user requests\n\nyes, there's a static property that reports the minFrameDurations for\neach supported stream configuration (including the ISP introduced\ndealys)\nhttps://jmondi.org/android_metadata_tags/docs.html#static_android.scaler.availableMinFrameDurations\n\nAnd two controls for the sensor frame durations, one static property\nhttps://jmondi.org/android_metadata_tags/docs.html#static_android.sensor.info.maxFrameDuration\n\nand a control\nhttps://jmondi.org/android_metadata_tags/docs.html#controls_android.sensor.frameDuration\n\n> (of course, bounded by the former)?  This was where my thinking was\n> going, and with frame duration priority, exposure time is capped by\n> the user request (if given), or the sensor limits.  But as always, if\n> the consensus is to behave differently, I am happy to change things\n> around.\n\nI don't have preferences, nor reasons that make me think one is better\nthan the other. I wonder if we should enforce this at all or let pipeline\nhandler/IPA decide that.\n\nThanks\n  j\n\n>\n> Regards,\n> Naush\n>\n>\n> >\n> > Just trying to get more data points to make an informed decision, as\n> > your question on what has to be prioritized is quite sensible...\n> >\n> > Thanks\n> >   j","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C089603CA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jun 2020 12:11:10 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id B9AE7100007;\n\tWed,  3 Jun 2020 10:11:09 +0000 (UTC)"],"Date":"Wed, 3 Jun 2020 12:14:29 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200603101429.6xqtp2xsvgoeuel3@uno.localdomain>","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>\n\t<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>\n\t<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>\n\t<CAEmqJPokD1n3J78SoxRX3D_0bjCWm4cMK5aC_E4j=R6ACyB6ug@mail.gmail.com>\n\t<20200603084535.ljtwgwsgieonicxi@uno.localdomain>\n\t<CAEmqJPoJrQnVGzsnm9a+N1aHuTaekWMv-Fzo8FEZGsTeoXLiwg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPoJrQnVGzsnm9a+N1aHuTaekWMv-Fzo8FEZGsTeoXLiwg@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Wed, 03 Jun 2020 10:11:10 -0000"}},{"id":4981,"web_url":"https://patchwork.libcamera.org/comment/4981/","msgid":"<CAPY8ntAkOL18rp+iXhN=ouObQE=+ckxdoJWGUwN5mgop2Mj9+w@mail.gmail.com>","date":"2020-06-03T11:53:48","subject":"Re: [libcamera-devel] [PATCH v2 1/3] libcamera: controls: Add frame\n\tduration control","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"Hi Jacopo\n\nOn Wed, 3 Jun 2020 at 11:11, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi Naush,\n>\n> On Wed, Jun 03, 2020 at 10:44:18AM +0100, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> > On Wed, 3 Jun 2020 at 09:42, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > >\n> > > Hi Naush,\n> > >\n> > >     I won't go into details here as I would like Laurent to chime in\n> > > and express his view on this topic, but I would have a question in the\n> > > meantime.\n> > >\n> > > On Tue, Jun 02, 2020 at 10:38:41AM +0100, Naushir Patuck wrote:\n> > > > Hi Jacopo,\n> > > >\n> > > >\n> > > > On Mon, 1 Jun 2020 at 16:40, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > > > >\n> > > > > Hi Naush,\n> > > > >\n> > > > > On Thu, May 28, 2020 at 09:52:51AM +0100, Naushir Patuck wrote:\n> > >\n> > > [snip]\n> > >\n> > > > > > > > On \"FrameDuration\" itself: would it make sense to only considered it\n> > > > > > > > when running with AE off ? In that case it would be applications\n> > > > > > > > responsibility to opportunely calculate the exposure time and the frame\n> > > > > > > > duration, but I think it's expected if you run in manual mode. And for\n> > > > > > > > reference, that's what Android does as well :)\n> > > > > > > >\n> > > > > > > > It would also rule out the need to specify how the frameDurationLimits\n> > > > > > > > would be used when running in manual mode and wanting to configure a\n> > > > > > > > precise value there, as I guess you would have to give to min and max\n> > > > > > > > the same value to express that.\n> > > > > >\n> > > > > > FrameDurationLimits should only really apply with AE enabled.  AE\n> > > > > > varies the shutter speed all over the place, that could cause\n> > > > > > framerate changes that we need to clip.  If AE is disabled and we have\n> > > > > > a manual shutter speed set, that dictates the frame duration, and any\n> > > > > > limits will not apply.  Perhaps I should add this in the description?\n> > > > > >\n> > > >\n> > > > I have to correct myself here.  FrameDurationLimits will also apply\n> > > > with manual shutter speeds when AE is disabled.  We could, say, have a\n> > > > manual shutter speed request of 25 ms, but still want to run at 30fps.\n> > > > There is a question on what happens if they clash, e.g. request a\n> > > > manual shutter speed of 66ms, and a FrameDuration of 30fps?  What gets\n> > > > prioritised?  Currently, in our implementation, FrameDuration will\n> > > > always be prioritised.\n> > >\n> > > Is there a rationle behind this decision ? I'm asking as I see Android\n> > > going in the other direction, with the exposure time always overriding\n> > > the requested frame duration.\n> >\n> > This was an entirely arbitrary choice on my part - based on what was\n> > easiest to do at the time :)\n> >\n> > > It is specified in their documentation that the mximum frame duration\n> > > a camera reports (lower frame rate) shall be at least the maximum\n> > > allowed exposure time, so that all possible exposure values are known\n> > > to be in the requested FPS range. I guess this should be enforced even\n> > > if we give frame duration priority, or should we cap the maximum frame\n> > > duration to the maximum available exposure time ?\n> >\n> > I have not looked at the Android docs in ages, but do they have two\n> > sets of frame duration limits, one that the sensor will report (sounds\n> > like this is what you are working on?) and one that the user requests\n>\n> yes, there's a static property that reports the minFrameDurations for\n> each supported stream configuration (including the ISP introduced\n> dealys)\n> https://jmondi.org/android_metadata_tags/docs.html#static_android.scaler.availableMinFrameDurations\n>\n> And two controls for the sensor frame durations, one static property\n> https://jmondi.org/android_metadata_tags/docs.html#static_android.sensor.info.maxFrameDuration\n>\n> and a control\n> https://jmondi.org/android_metadata_tags/docs.html#controls_android.sensor.frameDuration\n>\n> > (of course, bounded by the former)?  This was where my thinking was\n> > going, and with frame duration priority, exposure time is capped by\n> > the user request (if given), or the sensor limits.  But as always, if\n> > the consensus is to behave differently, I am happy to change things\n> > around.\n>\n> I don't have preferences, nor reasons that make me think one is better\n> than the other. I wonder if we should enforce this at all or let pipeline\n> handler/IPA decide that.\n\nV4L2 has it that VBLANK (ie frame rate) clips the max exposure time,\ntherefore if libcamera doesn't follow that then you have to be very\nconscious of doing things in the correct order - reset VBLANK / frame\nrate first to reset exposure ranges, and then set your desired\nexposure time.\nI'm trying to think whether I've encountered sensors that won't allow\nyou to adjust the blanking periods whilst streaming, as those would\nlimit the exposure time without stopping and restarting. None are\njumping to mind, but others may have encountered such.\n\nFor AE modes, there is the override control\nV4L2_CID_EXPOSURE_AUTO_PRIORITY [1], but otherwise frame rate all\nother references appear to say frame rate dictates max exposure.\n\nFor performance tests it could be considered useful if the specified\nframe rate was guaranteed to be delivered. Then again you could claim\nit was a flawed test if it set the exposure time to a value that\ncontradicted the frame rate it was trying to measure.\n\n  Dave\n\n[1] https://www.kernel.org/doc/html/latest/media/uapi/v4l/ext-ctrls-camera.html\n\n> Thanks\n>   j\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > >\n> > > Just trying to get more data points to make an informed decision, as\n> > > your question on what has to be prioritized is quite sensible...\n> > >\n> > > Thanks\n> > >   j\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<dave.stevenson@raspberrypi.com>","Received":["from mail-wr1-x435.google.com (mail-wr1-x435.google.com\n\t[IPv6:2a00:1450:4864:20::435])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42E2761027\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jun 2020 13:54:05 +0200 (CEST)","by mail-wr1-x435.google.com with SMTP id j10so2031325wrw.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 03 Jun 2020 04:54:05 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"MVgWXilz\"; 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=tSn284Bv7Uad1gYTS2WW1CdAR2kv3m1AKfZBNqpFCEo=;\n\tb=MVgWXilzWWx/isu6XKjxEiAB1HuFHZwLyFy0LzpEnPSGoor6uK1Hd1lXQk5AOUlZS4\n\ttqCqFYITDUk/6vdseZscpbNoO5I+odQjX5cuOAjFA3iOjv3tDVS0TDBJR5IxzZb6RyZi\n\ttvK6BwDc8un2lPFFyYK5HWzu11qwTg8JV48uOVqWkMGq/FlBO4js9dISNd9z3/CpOcCL\n\tjE14kQTiRe79zNJyarTzpFASn1GodzZqm6dC9I08BQCP0TMoblIP6+nKyBjmF/ZkNLrW\n\tO/iTRInxjVKNi8v7uxvrRdchWfA+HSSQb5ibiuz6usnBW5Msn2t8bWZLDqikqI2Zo/ro\n\tg8PA==","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=tSn284Bv7Uad1gYTS2WW1CdAR2kv3m1AKfZBNqpFCEo=;\n\tb=pCAdjzYOWek43klC4qI6JO5QitPpAXNdt889EeV4L0Dzhq21XwkSFh0GDw30smC0Ov\n\tPdEGmIAEftCZc5QqIeOQzBjeDgf/CzAKaHq47vrWaHPXhbvsRZOR/mMNRbNGh2f6kylr\n\t3MqHSv10sJFQx6sC9VK8Fd4NB00kXs2n8QQXH0EV/mZodplbwEP1e9VIbdcCRtkvr7eb\n\tkzoBbiSqXHMrLkAC4glHGpI2YiwomFmQIob8Rrj+6UCsWOPJUXOibhDd1nAL2Vnx3MYg\n\t0Ol4aMWBhNip8OL67x4RVn6U07jtFpfw6YZdxMaNpLlIoXCEESQblcc8w5gUbwQGufF4\n\txtLA==","X-Gm-Message-State":"AOAM532GdYl06oFHHQigTMr2m0RqBj1YeaI9voBJZVurrkLxHd0R8kX2\n\tlFfPNWVKznuewCY9Pd7BqQMaMMNpoToTCOrNAZ23Cg==","X-Google-Smtp-Source":"ABdhPJwovXyqSUqWSQ2nZeSyMsRtTCFBDH9QCd2CewPyxQ6Fn28PxccVBZGeTDNHEFtF7S9eYLee6TBJ/n19YBF/ewc=","X-Received":"by 2002:a5d:6986:: with SMTP id g6mr31760818wru.27.1591185244673;\n\tWed, 03 Jun 2020 04:54:04 -0700 (PDT)","MIME-Version":"1.0","References":"<20200513091120.1653-1-naush@raspberrypi.com>\n\t<20200513091120.1653-2-naush@raspberrypi.com>\n\t<20200524222302.GA6006@pendragon.ideasonboard.com>\n\t<20200527214734.45xrc3etyks27lsz@uno.localdomain>\n\t<20200528005541.GB4670@pendragon.ideasonboard.com>\n\t<CAEmqJPo7=BqjGGZzQNXwmvS2W4WE10S0yEmUofNrkFv35UuB3A@mail.gmail.com>\n\t<20200601154322.f4t3iuvb7tquvbro@uno.localdomain>\n\t<CAEmqJPokD1n3J78SoxRX3D_0bjCWm4cMK5aC_E4j=R6ACyB6ug@mail.gmail.com>\n\t<20200603084535.ljtwgwsgieonicxi@uno.localdomain>\n\t<CAEmqJPoJrQnVGzsnm9a+N1aHuTaekWMv-Fzo8FEZGsTeoXLiwg@mail.gmail.com>\n\t<20200603101429.6xqtp2xsvgoeuel3@uno.localdomain>","In-Reply-To":"<20200603101429.6xqtp2xsvgoeuel3@uno.localdomain>","From":"Dave Stevenson <dave.stevenson@raspberrypi.com>","Date":"Wed, 3 Jun 2020 12:53:48 +0100","Message-ID":"<CAPY8ntAkOL18rp+iXhN=ouObQE=+ckxdoJWGUwN5mgop2Mj9+w@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 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>","X-List-Received-Date":"Wed, 03 Jun 2020 11:54:05 -0000"}}]