[{"id":35801,"web_url":"https://patchwork.libcamera.org/comment/35801/","msgid":"<CAHW6GYKgd1DWD9aUUco2Bs2drOZixhK3rfjWU6DMd0MTsM1W1A@mail.gmail.com>","date":"2025-09-12T10:21:54","subject":"Re: [PATCH v2 1/5] libcamera: control_ids_core: Add flash controls","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Matthias\n\nThanks for the updated version.\n\nOn Fri, 12 Sept 2025 at 08:13, Matthias Fend <matthias.fend@emfend.at> wrote:\n>\n> Define a set of controls to control camera flash devices.\n>\n> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>\n> ---\n>  src/libcamera/control_ids_core.yaml | 78 +++++++++++++++++++++++++++++++++++++\n>  1 file changed, 78 insertions(+)\n>\n> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\n> index eec4b4f937ee6a2d751bb747e3b2d79dc16b7a3a..ff002accd771918b2618694ab14cda386997bc12 100644\n> --- a/src/libcamera/control_ids_core.yaml\n> +++ b/src/libcamera/control_ids_core.yaml\n> @@ -1284,4 +1284,82 @@ controls:\n>\n>          The FrameWallClock control can only be returned in metadata.\n>\n> +  - FlashMode:\n> +      type: int32_t\n> +      direction: inout\n> +      description: |\n> +        Flash operation mode.\n> +      enum:\n> +        - name: FlashModeNone\n> +          value: 0\n> +          description: |\n> +            Flash is off and inactive.\n> +        - name: FlashModeFlash\n> +          value: 1\n> +          description: |\n> +            The flash is active, but will only be switched on for a short time\n> +            by means of a trigger (software or external strobe). The maximum\n> +            switch-on time is limited by the FlashTimeout setting.\n> +        - name: FlashModeTorch\n> +          value: 2\n> +          description: |\n> +            The flash is continuously on and active. Commonly referred to as\n> +            torch or video light mode.\n\nI wonder what the FlashMode should be when the AGC/AEC is driving the\nflash - which I expect to be a common scenario in real life. Generally\nI'm feeling there's a kind of \"auto/manual\" distinction, a bit like we\nhave elsewhere. When the flash is in \"auto\" mode, then it's the\nAGC/AEC that's going to drive it. When it's in \"manual\" mode, you get\nto set all these parameters, turn the flash on or off yourself. What\ndo you think?\n\n> +\n> +  - FlashIntensity:\n> +      type: int32_t\n> +      direction: inout\n> +      description: |\n> +        Flash intensity in mA. Used when the flash is operated in flash mode.\n\nStill struggling a bit with the mA here!\n\nI think that, as a user of the flash, my minimum requirement is that I\nwould have linear control over the flash intensity. That is, if I\ndouble the intensity then I get double the pixel brightness (when\nthere is no ambient light, or double the increase in brightness when\nthere is ambient light). This allows you to fire the flash (at a\nlow-ish level), measure the response, and calculate what level of\nflash you really need.\n\nMy belief is that the use of mA would preclude that. So I'm still\nthinking I'd rather have an intensity that's (for example) a\npercentage of maximum, that has been calibrated (in the driver, or a\nflash_helper...) to behave linearly. Things like max current and so on\nshould still appear in the camera properties (or somewhere like that).\n\nNit-picking a little, mA is not a unit of intensity - perhaps if\nsomeone wants direct control of the current, a FlashCurrent control\nwould be better? Though I'm not convinced by the need for a\nFlashCurrent control if you already have FlashIntensity, though am\nopen to persuasion there!\n\n> +\n> +  - FlashTimeout:\n> +      type: int32_t\n> +      direction: inout\n> +      description: |\n> +        Flash timeout in us. Determines the maximum flash switch-on time. After\n> +        this time has elapsed, the flash is switched off by the hardware, even\n> +        if a strobe is still active.\n> +\n> +  - FlashStrobeSource:\n> +      type: int32_t\n> +      direction: inout\n> +      description: |\n> +        Flash strobe source.\n> +      enum:\n> +        - name: FlashStrobeSourceSoftware\n> +          value: 0\n> +          description: |\n> +            The strobe signal is controlled via software using the FlashStrobe\n> +            control.\n> +        - name: FlashStrobeSourceExternal\n> +          value: 1\n> +          description: |\n> +            The strobe signal is controlled by an external source. Typically,\n> +            this is done on the hardware by connecting the strobe source\n> +            directly to the flash controller.\n> +\n> +  - FlashStrobe:\n> +      type: int32_t\n> +      direction: in\n> +      description: |\n> +        Start/stop flash strobe. Only possible if FlashStrobeSourceSoftware is\n> +        selcted as FlashStrobeSource.\n> +\n> +      enum:\n> +        - name: FlashStrobeStart\n> +          value: 0\n> +          description: |\n> +            Start flash strobe.\n> +\n> +        - name: FlashStrobeStop\n> +          value: 1\n> +          description: |\n> +            Stop flash strobe.\n> +\n> +  - FlashTorchIntensity:\n> +      type: int32_t\n> +      direction: inout\n> +      description: |\n> +        Torch intensity in mA. Used when the flash is operated in torch mode.\n> +\n\nI'm also still wondering if it's possible to return metadata that\ntells us about the flash status for any completed request. That is,\nwas the flash on or off, or \"partially on\" - does that sound possible?\nI guess to some extent you could dead-reckon this sort of stuff if you\nhave a reasonable timestamp for when the flash goes on/off, but it's\nstill a bit fiddly (you have to worry about not only the frame's\ntimestamp, but the time before that when it started exposing, as well\nas the read-out time of the final pixel, which may be earlier than the\nframe length, of course). Maybe we should be nice and figure this out\nfor the users? Or is this a topic for another time?\n\nSorry for all the questions!\n\nBest regards\nDavid\n\n>  ...\n>\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 06AA7C324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Sep 2025 10:22:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 90FCF6936F;\n\tFri, 12 Sep 2025 12:22:09 +0200 (CEST)","from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com\n\t[IPv6:2607:f8b0:4864:20::72f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2261769367\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Sep 2025 12:22:07 +0200 (CEST)","by mail-qk1-x72f.google.com with SMTP id\n\taf79cd13be357-8112c7d196eso177488585a.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Sep 2025 03:22:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"a/xJDgz5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1757672526; x=1758277326;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=1DnoZ3/Lag39M6FcOHSiQ3MYegXpuvmpAipAkza/cx4=;\n\tb=a/xJDgz5lULVElIjiPcZH6wha6xD++V4BRzEpIfFhGVtIHVEhTeF4BU96H7iSYiLvb\n\tdA+M7kwZ7gTkUGQmoa3EW+E3IxZF4ICqfDwpgeTqCAzbpKIRaiWL1UfXyp8D5Tfw6uG+\n\tG5pC89XySTXqpjwn191hAyfWvBlRloa2RJbm4ATkQy4i20cuEnGNADIYENvizv3JOWWc\n\tPQi2D8NU5DyJYdt+T3KzxZM5PyNSXNSllW8ejAx/8yaDGwmKLE0KBfevulj2YIDbTq+b\n\tb++F5KwbZW/9v+VDDWZqdFgI2UBCAGKtD5oe84BwNY76EbCTB/oN4T3YfJStNB3kh9gZ\n\t69ZA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1757672526; x=1758277326;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=1DnoZ3/Lag39M6FcOHSiQ3MYegXpuvmpAipAkza/cx4=;\n\tb=S6+pUIncGWFbTJ16w8O2W0R2s2HYqiRi41emxXz9hD6KTsbRFCYfPDHBnBrowd6HYc\n\tmvlFV/yKmw7Vexfplk1+HHGcXLF2aCztLTKBOiYfJEmXWUPnXqbiq+Hp8Z41Zuj+V6JZ\n\txg0fN/Gj4e40TYQekAE2qyTP9tjkXfwqX07dZGeDUwDtnqRAKahLbB6Al0XTEoNuVjKJ\n\tTQVsr/QFuraEGtayDbGXHvAg8zQEyceUfW+XPgSg+eT/C282UnsP6cloahI5wn0pa28J\n\tAvnMS/vu20vrzzMFSKM0LqzoqsbrYP07J7Q/1CuxkERL+XCc+AmF3Sy3Y747LF11vTy4\n\ts8Ag==","X-Gm-Message-State":"AOJu0YwnvodtjqwHNfua70ABTXuvNKjYj2rYmwXlBl3WVvlX2wttqxXc\n\tG9BVrjNunSslj8UDwq9HAlXBVz0oBAsoAYnMVvuCIGGOF5s0P8y+xRy1XoyIYbPEjUmg5sImIgx\n\ti2mLTNo+CjAmADA0KtYnDBNJRsZrtadkKjc4zRLkhx2QtKAOG2ZNLD1I=","X-Gm-Gg":"ASbGncsBNdeAiKByl7JmPe4NNO+DlNF5IiyoTuzZ8jenqLRTDIlwmOH8MutSxzunyu7\n\tIOzdFFipbnNoRslK3eW7Rq6VH8LU931g7BNFMsrR1V02noJaY3BgrlI7fGM+fx3v60a422ixOfs\n\t/DqM7kGa+qiK5/pJvxc1vCrthXesmOqfjqMMSRkbmwnyap57itefBDGZ0heDmqKthdr6u3uo2vT\n\tAAbZmKP8dZSTv08AfcMWkJgd4yfqjLMVUkGnLD8yr5YIpbzgyzjXfgHvlsdPzyJCkSZFRSgcwxn\n\t0unkzgA/eRqO0wZBI8GflpV5","X-Google-Smtp-Source":"AGHT+IFGgNLvGTeHFrmZerp41CC6s2Olk8+j9o8erimtSCddpcfC5s86yE38PIWg9iE0EWCWc9AE2SUU4Vdu9gJrauQ=","X-Received":"by 2002:a05:620a:199b:b0:824:6bd7:e780 with SMTP id\n\taf79cd13be357-8246bd7e81dmr161284385a.15.1757672525559;\n\tFri, 12 Sep 2025 03:22:05 -0700 (PDT)","MIME-Version":"1.0","References":"<20250912-flash_reco-v2-0-d5bb80a2e619@emfend.at>\n\t<20250912-flash_reco-v2-1-d5bb80a2e619@emfend.at>","In-Reply-To":"<20250912-flash_reco-v2-1-d5bb80a2e619@emfend.at>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Fri, 12 Sep 2025 11:21:54 +0100","X-Gm-Features":"Ac12FXw5BwaoYRo4txG83kRcrpZ8kibcxclwsWLRmfTH_t1QTeYDBb2-iJF2zog","Message-ID":"<CAHW6GYKgd1DWD9aUUco2Bs2drOZixhK3rfjWU6DMd0MTsM1W1A@mail.gmail.com>","Subject":"Re: [PATCH v2 1/5] libcamera: control_ids_core: Add flash controls","To":"Matthias Fend <matthias.fend@emfend.at>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35808,"web_url":"https://patchwork.libcamera.org/comment/35808/","msgid":"<d440bbeb-e53a-4d99-b10e-2c61dfb7d923@emfend.at>","date":"2025-09-12T14:09:47","subject":"Re: [PATCH v2 1/5] libcamera: control_ids_core: Add flash controls","submitter":{"id":134,"url":"https://patchwork.libcamera.org/api/people/134/","name":"Matthias Fend","email":"matthias.fend@emfend.at"},"content":"Hi David,\n\nthanks for your comments, questions, and thoughts on this.\n\nJust to emphasize the note in the cover letter again (in case it got \nlost). Since the proposed API is considered too low-level, it will most \nlikely not make it into libcamera in this form.\n\nAm 12.09.2025 um 12:21 schrieb David Plowman:\n> Hi Matthias\n> \n> Thanks for the updated version.\n> \n> On Fri, 12 Sept 2025 at 08:13, Matthias Fend <matthias.fend@emfend.at> wrote:\n>>\n>> Define a set of controls to control camera flash devices.\n>>\n>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>\n>> ---\n>>   src/libcamera/control_ids_core.yaml | 78 +++++++++++++++++++++++++++++++++++++\n>>   1 file changed, 78 insertions(+)\n>>\n>> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\n>> index eec4b4f937ee6a2d751bb747e3b2d79dc16b7a3a..ff002accd771918b2618694ab14cda386997bc12 100644\n>> --- a/src/libcamera/control_ids_core.yaml\n>> +++ b/src/libcamera/control_ids_core.yaml\n>> @@ -1284,4 +1284,82 @@ controls:\n>>\n>>           The FrameWallClock control can only be returned in metadata.\n>>\n>> +  - FlashMode:\n>> +      type: int32_t\n>> +      direction: inout\n>> +      description: |\n>> +        Flash operation mode.\n>> +      enum:\n>> +        - name: FlashModeNone\n>> +          value: 0\n>> +          description: |\n>> +            Flash is off and inactive.\n>> +        - name: FlashModeFlash\n>> +          value: 1\n>> +          description: |\n>> +            The flash is active, but will only be switched on for a short time\n>> +            by means of a trigger (software or external strobe). The maximum\n>> +            switch-on time is limited by the FlashTimeout setting.\n>> +        - name: FlashModeTorch\n>> +          value: 2\n>> +          description: |\n>> +            The flash is continuously on and active. Commonly referred to as\n>> +            torch or video light mode.\n> \n> I wonder what the FlashMode should be when the AGC/AEC is driving the\n> flash - which I expect to be a common scenario in real life. Generally\n> I'm feeling there's a kind of \"auto/manual\" distinction, a bit like we\n> have elsewhere. When the flash is in \"auto\" mode, then it's the\n> AGC/AEC that's going to drive it. When it's in \"manual\" mode, you get\n> to set all these parameters, turn the flash on or off yourself. What\n> do you think?\n\nAll of these controls target the flash hardware device and not a higher \nlevel of flash support, such as automatically taking still images with \nflash.\nBasically, you can do the same things as an application that uses the \nflash-v4l2 device. Currently, a libcamera application needs to handle a \nflash device separately via v4l2 controls.\nWith this series, libcamera automatically detects the correct v4l2 flash \ndevice, and the application can use libcamera controls. You no longer \nneed to deal with v4l2. No more, no less.\n\nWhen we talk about settings and modes that actually take care of all the \nflash handling and still image capture for an application, there are \nmany more possibilities and details to consider. So just a few thoughts.\n\nFor videos (streaming or recording), only torch mode is actually \nsuitable. Whether this should be used (and at what intensity) or not can \nreally only be decided by the application.\n\nThe actual torch, as we know it from our cell phones, can also be a use \ncase.\n\nFor still image capture, I also see various selectable modes (torch, \nflash with pre-flash, flash with multi-exposure, etc.), each of which \nhas further configurations for things like intensity.\n\n> \n>> +\n>> +  - FlashIntensity:\n>> +      type: int32_t\n>> +      direction: inout\n>> +      description: |\n>> +        Flash intensity in mA. Used when the flash is operated in flash mode.\n> \n> Still struggling a bit with the mA here!\n> \n> I think that, as a user of the flash, my minimum requirement is that I\n> would have linear control over the flash intensity. That is, if I\n> double the intensity then I get double the pixel brightness (when\n> there is no ambient light, or double the increase in brightness when\n> there is ambient light). This allows you to fire the flash (at a\n> low-ish level), measure the response, and calculate what level of\n> flash you really need.\n> \n> My belief is that the use of mA would preclude that. So I'm still\n> thinking I'd rather have an intensity that's (for example) a\n> percentage of maximum, that has been calibrated (in the driver, or a\n> flash_helper...) to behave linearly. Things like max current and so on\n> should still appear in the camera properties (or somewhere like that).\n> \n> Nit-picking a little, mA is not a unit of intensity - perhaps if\n> someone wants direct control of the current, a FlashCurrent control\n> would be better? Though I'm not convinced by the need for a\n> FlashCurrent control if you already have FlashIntensity, though am\n> open to persuasion there!\n\nThis control also refers to a low level and corresponds fairly closely \nto the corresponding v4l2 control.\nIf you want to have a corresponding high-level control here that is \nsupposed to reflect the actual intensity, then you will probably need a \nmapping in the tuning file.\nI don't think that either the driver or a device helper (per driver as \ndone for image sensor) has the necessary information to know how the \ncurrent actually affects the intensity. At least for LEDs, this is \ntypically not linear.\n\nAnother factor that should not be overlooked here is the influence of \nthe scene.\nIf you take a picture with flash intensity set to 50% and 100% \nrespectively, depending on the scene, the resulting image brightness \nwill change as expected, remain completely the same, or somewhere in \nbetween.\n\n> \n>> +\n>> +  - FlashTimeout:\n>> +      type: int32_t\n>> +      direction: inout\n>> +      description: |\n>> +        Flash timeout in us. Determines the maximum flash switch-on time. After\n>> +        this time has elapsed, the flash is switched off by the hardware, even\n>> +        if a strobe is still active.\n>> +\n>> +  - FlashStrobeSource:\n>> +      type: int32_t\n>> +      direction: inout\n>> +      description: |\n>> +        Flash strobe source.\n>> +      enum:\n>> +        - name: FlashStrobeSourceSoftware\n>> +          value: 0\n>> +          description: |\n>> +            The strobe signal is controlled via software using the FlashStrobe\n>> +            control.\n>> +        - name: FlashStrobeSourceExternal\n>> +          value: 1\n>> +          description: |\n>> +            The strobe signal is controlled by an external source. Typically,\n>> +            this is done on the hardware by connecting the strobe source\n>> +            directly to the flash controller.\n>> +\n>> +  - FlashStrobe:\n>> +      type: int32_t\n>> +      direction: in\n>> +      description: |\n>> +        Start/stop flash strobe. Only possible if FlashStrobeSourceSoftware is\n>> +        selcted as FlashStrobeSource.\n>> +\n>> +      enum:\n>> +        - name: FlashStrobeStart\n>> +          value: 0\n>> +          description: |\n>> +            Start flash strobe.\n>> +\n>> +        - name: FlashStrobeStop\n>> +          value: 1\n>> +          description: |\n>> +            Stop flash strobe.\n>> +\n>> +  - FlashTorchIntensity:\n>> +      type: int32_t\n>> +      direction: inout\n>> +      description: |\n>> +        Torch intensity in mA. Used when the flash is operated in torch mode.\n>> +\n> \n> I'm also still wondering if it's possible to return metadata that\n> tells us about the flash status for any completed request. That is,\n> was the flash on or off, or \"partially on\" - does that sound possible?\n> I guess to some extent you could dead-reckon this sort of stuff if you\n> have a reasonable timestamp for when the flash goes on/off, but it's\n> still a bit fiddly (you have to worry about not only the frame's\n> timestamp, but the time before that when it started exposing, as well\n> as the read-out time of the final pixel, which may be earlier than the\n> frame length, of course). Maybe we should be nice and figure this out\n> for the users? Or is this a topic for another time?\n\nThere is a corresponding v4l2 control to at least read the current \nstrobe status (V4L2_CID_FLASH_STROBE_STATUS). This status could be added \nto the metadata.\nHowever, I am not sure whether all drivers consistently provide the same \ninformation here (e.g., what is returned when the strobe signal is still \nactive but a timeout has occurred) and whether the timing is reliable \nenough.\n\nPerhaps it is also possible to estimate the actual lightning activity \nsomehow from the brightness values of the frames.\n\n> \n> Sorry for all the questions!\n\nNo, thank you very much for them. It's good to see that there is at \nleast some discussion here.\n\nBest regards\n  ~Matthias\n\n> \n> Best regards\n> David\n> \n>>   ...\n>>\n>> --\n>> 2.34.1\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 92176BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Sep 2025 14:09:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5459169371;\n\tFri, 12 Sep 2025 16:09:50 +0200 (CEST)","from rmisp-mx-out2.tele.net (rmisp-mx-out2.tele.net\n\t[194.208.23.37])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 618636936A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Sep 2025 16:09:48 +0200 (CEST)","from [192.168.0.207] (194-208-208-245.tele.net [194.208.208.245])\n\tby rmisp-mx-out2.tele.net (Postfix) with ESMTPA id E107F10E3CA7;\n\tFri, 12 Sep 2025 16:09:47 +0200 (CEST)"],"Message-ID":"<d440bbeb-e53a-4d99-b10e-2c61dfb7d923@emfend.at>","Date":"Fri, 12 Sep 2025 16:09:47 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/5] libcamera: control_ids_core: Add flash controls","To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250912-flash_reco-v2-0-d5bb80a2e619@emfend.at>\n\t<20250912-flash_reco-v2-1-d5bb80a2e619@emfend.at>\n\t<CAHW6GYKgd1DWD9aUUco2Bs2drOZixhK3rfjWU6DMd0MTsM1W1A@mail.gmail.com>","Content-Language":"de-DE","From":"Matthias Fend <matthias.fend@emfend.at>","In-Reply-To":"<CAHW6GYKgd1DWD9aUUco2Bs2drOZixhK3rfjWU6DMd0MTsM1W1A@mail.gmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35835,"web_url":"https://patchwork.libcamera.org/comment/35835/","msgid":"<175801403063.99637.6053669968853672600@isaac-ThinkPad-T16-Gen-2>","date":"2025-09-16T09:13:50","subject":"Re: [PATCH v2 1/5] libcamera: control_ids_core: Add flash controls","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi all,\n\nThanks for the patch!\n\nQuoting David Plowman (2025-09-12 11:21:54)\n> Hi Matthias\n> \n> Thanks for the updated version.\n> \n> On Fri, 12 Sept 2025 at 08:13, Matthias Fend <matthias.fend@emfend.at> wrote:\n> >\n> > Define a set of controls to control camera flash devices.\n> >\n> > Signed-off-by: Matthias Fend <matthias.fend@emfend.at>\n> > ---\n> >  src/libcamera/control_ids_core.yaml | 78 +++++++++++++++++++++++++++++++++++++\n> >  1 file changed, 78 insertions(+)\n> >\n> > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\n> > index eec4b4f937ee6a2d751bb747e3b2d79dc16b7a3a..ff002accd771918b2618694ab14cda386997bc12 100644\n> > --- a/src/libcamera/control_ids_core.yaml\n> > +++ b/src/libcamera/control_ids_core.yaml\n> > @@ -1284,4 +1284,82 @@ controls:\n> >\n> >          The FrameWallClock control can only be returned in metadata.\n> >\n> > +  - FlashMode:\n> > +      type: int32_t\n> > +      direction: inout\n> > +      description: |\n> > +        Flash operation mode.\n> > +      enum:\n> > +        - name: FlashModeNone\n> > +          value: 0\n> > +          description: |\n> > +            Flash is off and inactive.\n> > +        - name: FlashModeFlash\n> > +          value: 1\n> > +          description: |\n> > +            The flash is active, but will only be switched on for a short time\n> > +            by means of a trigger (software or external strobe). The maximum\n> > +            switch-on time is limited by the FlashTimeout setting.\n> > +        - name: FlashModeTorch\n> > +          value: 2\n> > +          description: |\n> > +            The flash is continuously on and active. Commonly referred to as\n> > +            torch or video light mode.\n> \n> I wonder what the FlashMode should be when the AGC/AEC is driving the\n> flash - which I expect to be a common scenario in real life. Generally\n> I'm feeling there's a kind of \"auto/manual\" distinction, a bit like we\n> have elsewhere. When the flash is in \"auto\" mode, then it's the\n> AGC/AEC that's going to drive it. When it's in \"manual\" mode, you get\n> to set all these parameters, turn the flash on or off yourself. What\n> do you think?\n> \n> > +\n> > +  - FlashIntensity:\n> > +      type: int32_t\n> > +      direction: inout\n> > +      description: |\n> > +        Flash intensity in mA. Used when the flash is operated in flash mode.\n> \n> Still struggling a bit with the mA here!\n> \n> I think that, as a user of the flash, my minimum requirement is that I\n> would have linear control over the flash intensity. That is, if I\n> double the intensity then I get double the pixel brightness (when\n> there is no ambient light, or double the increase in brightness when\n> there is ambient light). This allows you to fire the flash (at a\n> low-ish level), measure the response, and calculate what level of\n> flash you really need.\n> \n> My belief is that the use of mA would preclude that. So I'm still\n> thinking I'd rather have an intensity that's (for example) a\n> percentage of maximum, that has been calibrated (in the driver, or a\n> flash_helper...) to behave linearly. Things like max current and so on\n> should still appear in the camera properties (or somewhere like that).\n> \n> Nit-picking a little, mA is not a unit of intensity - perhaps if\n> someone wants direct control of the current, a FlashCurrent control\n> would be better? Though I'm not convinced by the need for a\n> FlashCurrent control if you already have FlashIntensity, though am\n> open to persuasion there!\n> \n\nI agree here - in my experience controls for functions like torches can\nalso use regulators which use selectors, so perhaps a float or double\nto use as a multiplier? I've also seen that LED brightnesses are often\nset by an enum between 0 - 255.\n\nThe way I've tended to go is set a flash intensity in the driver as a\npercentage (a float 0.0 - 1.0), and pass that to the led_brightness enum in the\ndriver for the regulator that drives the LED (although I realise that\nmay just be my use case, I haven't implemented a strobe yet).\n\n> > +\n> > +  - FlashTimeout:\n> > +      type: int32_t\n> > +      direction: inout\n> > +      description: |\n> > +        Flash timeout in us. Determines the maximum flash switch-on time. After\n> > +        this time has elapsed, the flash is switched off by the hardware, even\n> > +        if a strobe is still active.\n> > +\n> > +  - FlashStrobeSource:\n> > +      type: int32_t\n> > +      direction: inout\n> > +      description: |\n> > +        Flash strobe source.\n> > +      enum:\n> > +        - name: FlashStrobeSourceSoftware\n> > +          value: 0\n> > +          description: |\n> > +            The strobe signal is controlled via software using the FlashStrobe\n> > +            control.\n> > +        - name: FlashStrobeSourceExternal\n> > +          value: 1\n> > +          description: |\n> > +            The strobe signal is controlled by an external source. Typically,\n> > +            this is done on the hardware by connecting the strobe source\n> > +            directly to the flash controller.\n> > +\n> > +  - FlashStrobe:\n> > +      type: int32_t\n> > +      direction: in\n> > +      description: |\n> > +        Start/stop flash strobe. Only possible if FlashStrobeSourceSoftware is\n> > +        selcted as FlashStrobeSource.\n> > +\n> > +      enum:\n> > +        - name: FlashStrobeStart\n> > +          value: 0\n> > +          description: |\n> > +            Start flash strobe.\n> > +\n> > +        - name: FlashStrobeStop\n> > +          value: 1\n> > +          description: |\n> > +            Stop flash strobe.\n> > +\n> > +  - FlashTorchIntensity:\n> > +      type: int32_t\n> > +      direction: inout\n> > +      description: |\n> > +        Torch intensity in mA. Used when the flash is operated in torch mode.\n> > +\n> \n> I'm also still wondering if it's possible to return metadata that\n> tells us about the flash status for any completed request. That is,\n> was the flash on or off, or \"partially on\" - does that sound possible?\n> I guess to some extent you could dead-reckon this sort of stuff if you\n> have a reasonable timestamp for when the flash goes on/off, but it's\n> still a bit fiddly (you have to worry about not only the frame's\n> timestamp, but the time before that when it started exposing, as well\n> as the read-out time of the final pixel, which may be earlier than the\n> frame length, of course). Maybe we should be nice and figure this out\n> for the users? Or is this a topic for another time?\n> \n> Sorry for all the questions!\n> \n> Best regards\n> David\n> \n\nBest wishes,\n\nIsaac\n\n> >  ...\n> >\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5AB00C328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 16 Sep 2025 09:13:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 836126936F;\n\tTue, 16 Sep 2025 11:13:56 +0200 (CEST)","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 95ABC62C3B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 16 Sep 2025 11:13:54 +0200 (CEST)","from thinkpad.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EDA57DF3;\n\tTue, 16 Sep 2025 11:12:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vv+SCPHm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758013956;\n\tbh=NLl9zFze44ZmOE//NOrvAIl09JckOKVHEymCifr0IZ0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=vv+SCPHmK8yaCXcz3fTUhvH8wpKITGijZbYOYt1Qjd3O2srBfAjdI+qxL9Gmtg2Xq\n\td64hABcCvS4j0pBHn3NnjEnqPzX01B1IOKfUm469PwpZ8LbVHjd070ADyHMwQ5fEZY\n\tuZMKa/IfjuC6fGaL/+wIt84RIZB8+COv+W7rFEJ8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAHW6GYKgd1DWD9aUUco2Bs2drOZixhK3rfjWU6DMd0MTsM1W1A@mail.gmail.com>","References":"<20250912-flash_reco-v2-0-d5bb80a2e619@emfend.at>\n\t<20250912-flash_reco-v2-1-d5bb80a2e619@emfend.at>\n\t<CAHW6GYKgd1DWD9aUUco2Bs2drOZixhK3rfjWU6DMd0MTsM1W1A@mail.gmail.com>","Subject":"Re: [PATCH v2 1/5] libcamera: control_ids_core: Add flash controls","From":"Isaac Scott <isaac.scott@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","To":"David Plowman <david.plowman@raspberrypi.com>,\n\tMatthias Fend <matthias.fend@emfend.at>","Date":"Tue, 16 Sep 2025 10:13:50 +0100","Message-ID":"<175801403063.99637.6053669968853672600@isaac-ThinkPad-T16-Gen-2>","User-Agent":"alot/0.10","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]