| Message ID | 20250828-flash-support-v1-1-4c5dc674a05b@emfend.at |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Matthias Fend (2025-08-28 14:09:38) > Define a set of controls to control camera flash devices. > > Signed-off-by: Matthias Fend <matthias.fend@emfend.at> > --- > src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml > index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 > --- a/src/libcamera/control_ids_draft.yaml > +++ b/src/libcamera/control_ids_draft.yaml Please target real controls not draft controls. We should really try to remove this draft file... > @@ -294,4 +294,73 @@ controls: > Currently identical to ANDROID_STATISTICS_FACE_IDS. > size: [n] > > + - FlashMode: > + type: int32_t > + direction: inout > + description: | > + Flash mode. > + enum: > + - name: FlashModeNone > + value: 0 > + description: | > + None. > + - name: FlashModeFlash > + value: 1 > + description: | > + Flash. > + - name: FlashModeTorch > + value: 2 > + description: | > + Torch. And I think we could expand all of those descriptions somehow. > + > + - FlashIntensity: > + type: int32_t > + direction: inout > + description: | > + Flash intensity in mA. > + > + - FlashTimeout: > + type: int32_t > + direction: inout > + description: | > + Flash timeout in us. > + > + - FlashStrobeSource: > + type: int32_t > + direction: inout > + description: | > + Flash mode. Flash mode ? Or Flash source? With all the descriptions expanded, these can target mainline controls. No need to go to draft. We'll need to convey relationships in the documentation too.. like perhaps it could be documented how FlashTimeout I suspect interacts only with FlashStrobeSourceSoftware ? ... > + enum: > + - name: FlashStrobeSourceSoftware > + value: 0 > + description: | > + Software. > + - name: FlashStrobeSourceExternal > + value: 1 > + description: | > + External. > + > + - FlashStrobe: > + type: int32_t > + direction: in > + description: | > + Start/stop flash strobe. > + > + enum: > + - name: FlashStrobeStart > + value: 0 > + description: | > + Start flash strobe. > + > + - name: FlashStrobeStop > + value: 1 > + description: | > + Stop flash strobe. > + > + - FlashTorchIntensity: Do we need this in addition to FlashIntensity ? Will we have different intensity limits depending on the mode between flash+torch? > + type: int32_t > + direction: inout > + description: | > + Torch intensity in mA. I guess we get mA from the kernel drivers? > + > ... > > -- > 2.34.1 >
Hi Kieran, thank you for your feedback. Am 30.08.2025 um 12:21 schrieb Kieran Bingham: > Quoting Matthias Fend (2025-08-28 14:09:38) >> Define a set of controls to control camera flash devices. >> >> Signed-off-by: Matthias Fend <matthias.fend@emfend.at> >> --- >> src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 69 insertions(+) >> >> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml >> index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 >> --- a/src/libcamera/control_ids_draft.yaml >> +++ b/src/libcamera/control_ids_draft.yaml > > Please target real controls not draft controls. We should really try to > remove this draft file... Okay. I'll move them. > > >> @@ -294,4 +294,73 @@ controls: >> Currently identical to ANDROID_STATISTICS_FACE_IDS. >> size: [n] >> >> + - FlashMode: >> + type: int32_t >> + direction: inout >> + description: | >> + Flash mode. >> + enum: >> + - name: FlashModeNone >> + value: 0 >> + description: | >> + None. >> + - name: FlashModeFlash >> + value: 1 >> + description: | >> + Flash. >> + - name: FlashModeTorch >> + value: 2 >> + description: | >> + Torch. > > And I think we could expand all of those descriptions somehow. ACK. > >> + >> + - FlashIntensity: >> + type: int32_t >> + direction: inout >> + description: | >> + Flash intensity in mA. >> + >> + - FlashTimeout: >> + type: int32_t >> + direction: inout >> + description: | >> + Flash timeout in us. >> + >> + - FlashStrobeSource: >> + type: int32_t >> + direction: inout >> + description: | >> + Flash mode. > > Flash mode ? Or Flash source? True. Seems that all descriptions need some TLC... > > With all the descriptions expanded, these can target mainline controls. > No need to go to draft. > > We'll need to convey relationships in the documentation too.. like > perhaps it could be documented how FlashTimeout I suspect interacts only > with FlashStrobeSourceSoftware ? ... FlashTimeout is the maximum permissible flash on-time and is guaranteed by the hardware. This applies to both the software and hardware (external) triggers. It is also possible to generate shorter flash times using the triggers. The timeout is essentially a protective mechanism that limits the maximum duration. > > >> + enum: >> + - name: FlashStrobeSourceSoftware >> + value: 0 >> + description: | >> + Software. >> + - name: FlashStrobeSourceExternal >> + value: 1 >> + description: | >> + External. >> + >> + - FlashStrobe: >> + type: int32_t >> + direction: in >> + description: | >> + Start/stop flash strobe. >> + >> + enum: >> + - name: FlashStrobeStart >> + value: 0 >> + description: | >> + Start flash strobe. >> + >> + - name: FlashStrobeStop >> + value: 1 >> + description: | >> + Stop flash strobe. >> + >> + - FlashTorchIntensity: > > Do we need this in addition to FlashIntensity ? > > Will we have different intensity limits depending on the mode between > flash+torch? Yes, exactly. Depending on the hardware setup, the maximum currents in the two modes can differ. Continuous operation is possible in torch mode, which typically allows a lower maximum current (often for thermal reasons). > >> + type: int32_t >> + direction: inout >> + description: | >> + Torch intensity in mA. > > I guess we get mA from the kernel drivers? Yes, the API documentation [1] for V4L2_CID_FLASH_INTENSITY says: "[..] The unit should be milliamps (mA) if possible. [..]" Best regards ~Matthias [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-flash.html > >> + >> ... >> >> -- >> 2.34.1 >>
Hi, On Sun, 31 Aug 2025 at 16:19, Matthias Fend <matthias.fend@emfend.at> wrote: > > Hi Kieran, > > thank you for your feedback. > > Am 30.08.2025 um 12:21 schrieb Kieran Bingham: > > Quoting Matthias Fend (2025-08-28 14:09:38) > >> Define a set of controls to control camera flash devices. > >> > >> Signed-off-by: Matthias Fend <matthias.fend@emfend.at> > >> --- > >> src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 69 insertions(+) > >> > >> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml > >> index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 > >> --- a/src/libcamera/control_ids_draft.yaml > >> +++ b/src/libcamera/control_ids_draft.yaml > > > > Please target real controls not draft controls. We should really try to > > remove this draft file... > > Okay. I'll move them. > > > > > > >> @@ -294,4 +294,73 @@ controls: > >> Currently identical to ANDROID_STATISTICS_FACE_IDS. > >> size: [n] > >> > >> + - FlashMode: > >> + type: int32_t > >> + direction: inout > >> + description: | > >> + Flash mode. > >> + enum: > >> + - name: FlashModeNone > >> + value: 0 > >> + description: | > >> + None. > >> + - name: FlashModeFlash > >> + value: 1 > >> + description: | > >> + Flash. > >> + - name: FlashModeTorch > >> + value: 2 > >> + description: | > >> + Torch. > > > > And I think we could expand all of those descriptions somehow. > > ACK. > > > > >> + > >> + - FlashIntensity: > >> + type: int32_t > >> + direction: inout > >> + description: | > >> + Flash intensity in mA. > >> + > >> + - FlashTimeout: > >> + type: int32_t > >> + direction: inout > >> + description: | > >> + Flash timeout in us. > >> + > >> + - FlashStrobeSource: > >> + type: int32_t > >> + direction: inout > >> + description: | > >> + Flash mode. > > > > Flash mode ? Or Flash source? > > True. Seems that all descriptions need some TLC... > > > > > With all the descriptions expanded, these can target mainline controls. > > No need to go to draft. > > > > We'll need to convey relationships in the documentation too.. like > > perhaps it could be documented how FlashTimeout I suspect interacts only > > with FlashStrobeSourceSoftware ? ... > > FlashTimeout is the maximum permissible flash on-time and is guaranteed > by the hardware. This applies to both the software and hardware > (external) triggers. It is also possible to generate shorter flash times > using the triggers. The timeout is essentially a protective mechanism > that limits the maximum duration. > > > > > > >> + enum: > >> + - name: FlashStrobeSourceSoftware > >> + value: 0 > >> + description: | > >> + Software. > >> + - name: FlashStrobeSourceExternal > >> + value: 1 > >> + description: | > >> + External. > >> + > >> + - FlashStrobe: > >> + type: int32_t > >> + direction: in > >> + description: | > >> + Start/stop flash strobe. > >> + > >> + enum: > >> + - name: FlashStrobeStart > >> + value: 0 > >> + description: | > >> + Start flash strobe. > >> + > >> + - name: FlashStrobeStop > >> + value: 1 > >> + description: | > >> + Stop flash strobe. > >> + > >> + - FlashTorchIntensity: > > > > Do we need this in addition to FlashIntensity ? > > > > Will we have different intensity limits depending on the mode between > > flash+torch? > > Yes, exactly. Depending on the hardware setup, the maximum currents in > the two modes can differ. Continuous operation is possible in torch > mode, which typically allows a lower maximum current (often for thermal > reasons). > > > >> + type: int32_t > >> + direction: inout > >> + description: | > >> + Torch intensity in mA. > > > > I guess we get mA from the kernel drivers? > > Yes, the API documentation [1] for V4L2_CID_FLASH_INTENSITY says: "[..] > The unit should be milliamps (mA) if possible. [..]" Should we consider changing this to, say, a percentage? The IPA can handle conversion from this to mA when setting the V4L2 ctrl. That way, applications do not have to do anything different when dealing with different devices with different limits. Regards, Naush > > Best regards > ~Matthias > > [1] > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-flash.html > > > > >> + > >> ... > >> > >> -- > >> 2.34.1 > >> >
Hi Thanks for posting some proposals for flash control. On Mon, 1 Sept 2025 at 09:41, Naushir Patuck <naush@raspberrypi.com> wrote: > > Hi, > > On Sun, 31 Aug 2025 at 16:19, Matthias Fend <matthias.fend@emfend.at> wrote: > > > > Hi Kieran, > > > > thank you for your feedback. > > > > Am 30.08.2025 um 12:21 schrieb Kieran Bingham: > > > Quoting Matthias Fend (2025-08-28 14:09:38) > > >> Define a set of controls to control camera flash devices. > > >> > > >> Signed-off-by: Matthias Fend <matthias.fend@emfend.at> > > >> --- > > >> src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ > > >> 1 file changed, 69 insertions(+) > > >> > > >> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml > > >> index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 > > >> --- a/src/libcamera/control_ids_draft.yaml > > >> +++ b/src/libcamera/control_ids_draft.yaml > > > > > > Please target real controls not draft controls. We should really try to > > > remove this draft file... > > > > Okay. I'll move them. > > > > > > > > > > >> @@ -294,4 +294,73 @@ controls: > > >> Currently identical to ANDROID_STATISTICS_FACE_IDS. > > >> size: [n] > > >> > > >> + - FlashMode: > > >> + type: int32_t > > >> + direction: inout > > >> + description: | > > >> + Flash mode. > > >> + enum: > > >> + - name: FlashModeNone > > >> + value: 0 > > >> + description: | > > >> + None. > > >> + - name: FlashModeFlash > > >> + value: 1 > > >> + description: | > > >> + Flash. > > >> + - name: FlashModeTorch > > >> + value: 2 > > >> + description: | > > >> + Torch. > > > > > > And I think we could expand all of those descriptions somehow. > > > > ACK. Yes, obviously more explanation is needed. I wasn't clear whether the "flash" flash mode is targeting firing the flash for a still image capture. Do there need to be some options so that an application can ask for a pre-flash? Pre-flash can be relatively complicated as there are different ways to do it, and it may require specific support in the AGC/AEC algorithm. The AGC/AEC might be deciding exactly when to trigger it, and then doing calculations by comparing frames with/without the flash in order to calculate the optimal level for the final capture. Just want to be sure we're thinking ahead about these possibilities! > > > > > > > > >> + > > >> + - FlashIntensity: > > >> + type: int32_t > > >> + direction: inout > > >> + description: | > > >> + Flash intensity in mA. I'm dubious about using mA here, it feels a bit close to the hardware to me! Would a percentage of the maximum be more application friendly? Then there might also be (for example) a camera property that gives you some idea of the absolute brightness (in lux?) of the flash at maximum intensity. > > >> + > > >> + - FlashTimeout: > > >> + type: int32_t > > >> + direction: inout > > >> + description: | > > >> + Flash timeout in us. > > >> + > > >> + - FlashStrobeSource: > > >> + type: int32_t > > >> + direction: inout > > >> + description: | > > >> + Flash mode. > > > > > > Flash mode ? Or Flash source? > > > > True. Seems that all descriptions need some TLC... > > > > > > > > With all the descriptions expanded, these can target mainline controls. > > > No need to go to draft. > > > > > > We'll need to convey relationships in the documentation too.. like > > > perhaps it could be documented how FlashTimeout I suspect interacts only > > > with FlashStrobeSourceSoftware ? ... > > > > FlashTimeout is the maximum permissible flash on-time and is guaranteed > > by the hardware. This applies to both the software and hardware > > (external) triggers. It is also possible to generate shorter flash times > > using the triggers. The timeout is essentially a protective mechanism > > that limits the maximum duration. > > > > > > > > > > >> + enum: > > >> + - name: FlashStrobeSourceSoftware > > >> + value: 0 > > >> + description: | > > >> + Software. > > >> + - name: FlashStrobeSourceExternal > > >> + value: 1 > > >> + description: | > > >> + External. > > >> + > > >> + - FlashStrobe: > > >> + type: int32_t > > >> + direction: in > > >> + description: | > > >> + Start/stop flash strobe. > > >> + > > >> + enum: > > >> + - name: FlashStrobeStart > > >> + value: 0 > > >> + description: | > > >> + Start flash strobe. > > >> + > > >> + - name: FlashStrobeStop > > >> + value: 1 > > >> + description: | > > >> + Stop flash strobe. > > >> + > > >> + - FlashTorchIntensity: > > > > > > Do we need this in addition to FlashIntensity ? > > > > > > Will we have different intensity limits depending on the mode between > > > flash+torch? > > > > Yes, exactly. Depending on the hardware setup, the maximum currents in > > the two modes can differ. Continuous operation is possible in torch > > mode, which typically allows a lower maximum current (often for thermal > > reasons). > > > > > >> + type: int32_t > > >> + direction: inout > > >> + description: | > > >> + Torch intensity in mA. > > > > > > I guess we get mA from the kernel drivers? > > > > Yes, the API documentation [1] for V4L2_CID_FLASH_INTENSITY says: "[..] > > The unit should be milliamps (mA) if possible. [..]" > > Should we consider changing this to, say, a percentage? The IPA can > handle conversion from this to mA when setting the V4L2 ctrl. That > way, applications do not have to do anything different when dealing > with different devices with different limits. Agree - see my remarks above! > > Regards, > Naush > > > > > Best regards > > ~Matthias Thanks! David > > > > [1] > > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-flash.html > > > > > > > >> + > > >> ... > > >> > > >> -- > > >> 2.34.1 > > >> > >
Hi Naushir, thanks for your comments. Am 01.09.2025 um 10:41 schrieb Naushir Patuck: > Hi, > > On Sun, 31 Aug 2025 at 16:19, Matthias Fend <matthias.fend@emfend.at> wrote: >> >> Hi Kieran, >> >> thank you for your feedback. >> >> Am 30.08.2025 um 12:21 schrieb Kieran Bingham: >>> Quoting Matthias Fend (2025-08-28 14:09:38) >>>> Define a set of controls to control camera flash devices. >>>> >>>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at> >>>> --- >>>> src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 69 insertions(+) >>>> >>>> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml >>>> index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 >>>> --- a/src/libcamera/control_ids_draft.yaml >>>> +++ b/src/libcamera/control_ids_draft.yaml >>> >>> Please target real controls not draft controls. We should really try to >>> remove this draft file... >> >> Okay. I'll move them. >> >>> >>> >>>> @@ -294,4 +294,73 @@ controls: >>>> Currently identical to ANDROID_STATISTICS_FACE_IDS. >>>> size: [n] >>>> >>>> + - FlashMode: >>>> + type: int32_t >>>> + direction: inout >>>> + description: | >>>> + Flash mode. >>>> + enum: >>>> + - name: FlashModeNone >>>> + value: 0 >>>> + description: | >>>> + None. >>>> + - name: FlashModeFlash >>>> + value: 1 >>>> + description: | >>>> + Flash. >>>> + - name: FlashModeTorch >>>> + value: 2 >>>> + description: | >>>> + Torch. >>> >>> And I think we could expand all of those descriptions somehow. >> >> ACK. >> >>> >>>> + >>>> + - FlashIntensity: >>>> + type: int32_t >>>> + direction: inout >>>> + description: | >>>> + Flash intensity in mA. >>>> + >>>> + - FlashTimeout: >>>> + type: int32_t >>>> + direction: inout >>>> + description: | >>>> + Flash timeout in us. >>>> + >>>> + - FlashStrobeSource: >>>> + type: int32_t >>>> + direction: inout >>>> + description: | >>>> + Flash mode. >>> >>> Flash mode ? Or Flash source? >> >> True. Seems that all descriptions need some TLC... >> >>> >>> With all the descriptions expanded, these can target mainline controls. >>> No need to go to draft. >>> >>> We'll need to convey relationships in the documentation too.. like >>> perhaps it could be documented how FlashTimeout I suspect interacts only >>> with FlashStrobeSourceSoftware ? ... >> >> FlashTimeout is the maximum permissible flash on-time and is guaranteed >> by the hardware. This applies to both the software and hardware >> (external) triggers. It is also possible to generate shorter flash times >> using the triggers. The timeout is essentially a protective mechanism >> that limits the maximum duration. >> >>> >>> >>>> + enum: >>>> + - name: FlashStrobeSourceSoftware >>>> + value: 0 >>>> + description: | >>>> + Software. >>>> + - name: FlashStrobeSourceExternal >>>> + value: 1 >>>> + description: | >>>> + External. >>>> + >>>> + - FlashStrobe: >>>> + type: int32_t >>>> + direction: in >>>> + description: | >>>> + Start/stop flash strobe. >>>> + >>>> + enum: >>>> + - name: FlashStrobeStart >>>> + value: 0 >>>> + description: | >>>> + Start flash strobe. >>>> + >>>> + - name: FlashStrobeStop >>>> + value: 1 >>>> + description: | >>>> + Stop flash strobe. >>>> + >>>> + - FlashTorchIntensity: >>> >>> Do we need this in addition to FlashIntensity ? >>> >>> Will we have different intensity limits depending on the mode between >>> flash+torch? >> >> Yes, exactly. Depending on the hardware setup, the maximum currents in >> the two modes can differ. Continuous operation is possible in torch >> mode, which typically allows a lower maximum current (often for thermal >> reasons). >>> >>>> + type: int32_t >>>> + direction: inout >>>> + description: | >>>> + Torch intensity in mA. >>> >>> I guess we get mA from the kernel drivers? >> >> Yes, the API documentation [1] for V4L2_CID_FLASH_INTENSITY says: "[..] >> The unit should be milliamps (mA) if possible. [..]" > > Should we consider changing this to, say, a percentage? The IPA can > handle conversion from this to mA when setting the V4L2 ctrl. That > way, applications do not have to do anything different when dealing > with different devices with different limits. Yes, perhaps. That would also be easy to implement. However, it is also practical if the value has a real, usable unit. With milliamps, at least it is clear what that means. With a percentage, the question arises again of what. A percentage of the maximum current (which is how large) or a percentage of the maximum luminous flux (what does not have to be equivalent to the current) ? Having the sensor gain in dB or the exposure time in seconds is somehow also more helpful than in percent. If an application wants to implement a generic slider with percent values, for example, it wouldn't be really difficult to simply multiply the maximum current value by it. Therefore, in my opinion, the absolute current makes sense somehow. But I'm happy to be convinced otherwise :) Thanks ~Matthias > > Regards, > Naush > >> >> Best regards >> ~Matthias >> >> [1] >> https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-flash.html >> >>> >>>> + >>>> ... >>>> >>>> -- >>>> 2.34.1 >>>> >>
Hi David, thanks for your feedback. Am 01.09.2025 um 11:31 schrieb David Plowman: > Hi > > Thanks for posting some proposals for flash control. > > On Mon, 1 Sept 2025 at 09:41, Naushir Patuck <naush@raspberrypi.com> wrote: >> >> Hi, >> >> On Sun, 31 Aug 2025 at 16:19, Matthias Fend <matthias.fend@emfend.at> wrote: >>> >>> Hi Kieran, >>> >>> thank you for your feedback. >>> >>> Am 30.08.2025 um 12:21 schrieb Kieran Bingham: >>>> Quoting Matthias Fend (2025-08-28 14:09:38) >>>>> Define a set of controls to control camera flash devices. >>>>> >>>>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at> >>>>> --- >>>>> src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 69 insertions(+) >>>>> >>>>> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml >>>>> index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 >>>>> --- a/src/libcamera/control_ids_draft.yaml >>>>> +++ b/src/libcamera/control_ids_draft.yaml >>>> >>>> Please target real controls not draft controls. We should really try to >>>> remove this draft file... >>> >>> Okay. I'll move them. >>> >>>> >>>> >>>>> @@ -294,4 +294,73 @@ controls: >>>>> Currently identical to ANDROID_STATISTICS_FACE_IDS. >>>>> size: [n] >>>>> >>>>> + - FlashMode: >>>>> + type: int32_t >>>>> + direction: inout >>>>> + description: | >>>>> + Flash mode. >>>>> + enum: >>>>> + - name: FlashModeNone >>>>> + value: 0 >>>>> + description: | >>>>> + None. >>>>> + - name: FlashModeFlash >>>>> + value: 1 >>>>> + description: | >>>>> + Flash. >>>>> + - name: FlashModeTorch >>>>> + value: 2 >>>>> + description: | >>>>> + Torch. >>>> >>>> And I think we could expand all of those descriptions somehow. >>> >>> ACK. > > Yes, obviously more explanation is needed. I wasn't clear whether the > "flash" flash mode is targeting firing the flash for a still image > capture. Do there need to be some options so that an application can > ask for a pre-flash? > > Pre-flash can be relatively complicated as there are different ways to > do it, and it may require specific support in the AGC/AEC algorithm. > The AGC/AEC might be deciding exactly when to trigger it, and then > doing calculations by comparing frames with/without the flash in order > to calculate the optimal level for the final capture. Just want to be > sure we're thinking ahead about these possibilities! If I understand correctly, there are no sequences for still image capture yet, right? This means that the application has to trigger the algorithms (AE, AWB, AF, etc.), wait until all settings are complete or stable, and then take a picture. Therefore, the application would also have to take care of controlling the flash—with everything that goes with it. I see the controls that are then used for such sequences at a higher level and independent of these flash controls. Admittedly, I certainly lack the necessary overall overview to participate in such strategic decisions in a qualified manner. > > > >>> >>>> >>>>> + >>>>> + - FlashIntensity: >>>>> + type: int32_t >>>>> + direction: inout >>>>> + description: | >>>>> + Flash intensity in mA. > > I'm dubious about using mA here, it feels a bit close to the hardware > to me! Would a percentage of the maximum be more application friendly? > > Then there might also be (for example) a camera property that gives > you some idea of the absolute brightness (in lux?) of the flash at > maximum intensity. > >>>>> + >>>>> + - FlashTimeout: >>>>> + type: int32_t >>>>> + direction: inout >>>>> + description: | >>>>> + Flash timeout in us. >>>>> + >>>>> + - FlashStrobeSource: >>>>> + type: int32_t >>>>> + direction: inout >>>>> + description: | >>>>> + Flash mode. >>>> >>>> Flash mode ? Or Flash source? >>> >>> True. Seems that all descriptions need some TLC... >>> >>>> >>>> With all the descriptions expanded, these can target mainline controls. >>>> No need to go to draft. >>>> >>>> We'll need to convey relationships in the documentation too.. like >>>> perhaps it could be documented how FlashTimeout I suspect interacts only >>>> with FlashStrobeSourceSoftware ? ... >>> >>> FlashTimeout is the maximum permissible flash on-time and is guaranteed >>> by the hardware. This applies to both the software and hardware >>> (external) triggers. It is also possible to generate shorter flash times >>> using the triggers. The timeout is essentially a protective mechanism >>> that limits the maximum duration. >>> >>>> >>>> >>>>> + enum: >>>>> + - name: FlashStrobeSourceSoftware >>>>> + value: 0 >>>>> + description: | >>>>> + Software. >>>>> + - name: FlashStrobeSourceExternal >>>>> + value: 1 >>>>> + description: | >>>>> + External. >>>>> + >>>>> + - FlashStrobe: >>>>> + type: int32_t >>>>> + direction: in >>>>> + description: | >>>>> + Start/stop flash strobe. >>>>> + >>>>> + enum: >>>>> + - name: FlashStrobeStart >>>>> + value: 0 >>>>> + description: | >>>>> + Start flash strobe. >>>>> + >>>>> + - name: FlashStrobeStop >>>>> + value: 1 >>>>> + description: | >>>>> + Stop flash strobe. >>>>> + >>>>> + - FlashTorchIntensity: >>>> >>>> Do we need this in addition to FlashIntensity ? >>>> >>>> Will we have different intensity limits depending on the mode between >>>> flash+torch? >>> >>> Yes, exactly. Depending on the hardware setup, the maximum currents in >>> the two modes can differ. Continuous operation is possible in torch >>> mode, which typically allows a lower maximum current (often for thermal >>> reasons). >>>> >>>>> + type: int32_t >>>>> + direction: inout >>>>> + description: | >>>>> + Torch intensity in mA. >>>> >>>> I guess we get mA from the kernel drivers? >>> >>> Yes, the API documentation [1] for V4L2_CID_FLASH_INTENSITY says: "[..] >>> The unit should be milliamps (mA) if possible. [..]" >> >> Should we consider changing this to, say, a percentage? The IPA can >> handle conversion from this to mA when setting the V4L2 ctrl. That >> way, applications do not have to do anything different when dealing >> with different devices with different limits. > > Agree - see my remarks above! See also my comment in response to Naushir. Thanks ~Matthias > >> >> Regards, >> Naush >> >>> >>> Best regards >>> ~Matthias > > Thanks! > > David > >>> >>> [1] >>> https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-flash.html >>> >>>> >>>>> + >>>>> ... >>>>> >>>>> -- >>>>> 2.34.1 >>>>> >>>
Hi Matthias On Mon, 1 Sept 2025 at 11:40, Matthias Fend <matthias.fend@emfend.at> wrote: > > Hi David, > > thanks for your feedback. > > Am 01.09.2025 um 11:31 schrieb David Plowman: > > Hi > > > > Thanks for posting some proposals for flash control. > > > > On Mon, 1 Sept 2025 at 09:41, Naushir Patuck <naush@raspberrypi.com> wrote: > >> > >> Hi, > >> > >> On Sun, 31 Aug 2025 at 16:19, Matthias Fend <matthias.fend@emfend.at> wrote: > >>> > >>> Hi Kieran, > >>> > >>> thank you for your feedback. > >>> > >>> Am 30.08.2025 um 12:21 schrieb Kieran Bingham: > >>>> Quoting Matthias Fend (2025-08-28 14:09:38) > >>>>> Define a set of controls to control camera flash devices. > >>>>> > >>>>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at> > >>>>> --- > >>>>> src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ > >>>>> 1 file changed, 69 insertions(+) > >>>>> > >>>>> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml > >>>>> index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 > >>>>> --- a/src/libcamera/control_ids_draft.yaml > >>>>> +++ b/src/libcamera/control_ids_draft.yaml > >>>> > >>>> Please target real controls not draft controls. We should really try to > >>>> remove this draft file... > >>> > >>> Okay. I'll move them. > >>> > >>>> > >>>> > >>>>> @@ -294,4 +294,73 @@ controls: > >>>>> Currently identical to ANDROID_STATISTICS_FACE_IDS. > >>>>> size: [n] > >>>>> > >>>>> + - FlashMode: > >>>>> + type: int32_t > >>>>> + direction: inout > >>>>> + description: | > >>>>> + Flash mode. > >>>>> + enum: > >>>>> + - name: FlashModeNone > >>>>> + value: 0 > >>>>> + description: | > >>>>> + None. > >>>>> + - name: FlashModeFlash > >>>>> + value: 1 > >>>>> + description: | > >>>>> + Flash. > >>>>> + - name: FlashModeTorch > >>>>> + value: 2 > >>>>> + description: | > >>>>> + Torch. > >>>> > >>>> And I think we could expand all of those descriptions somehow. > >>> > >>> ACK. > > > > Yes, obviously more explanation is needed. I wasn't clear whether the > > "flash" flash mode is targeting firing the flash for a still image > > capture. Do there need to be some options so that an application can > > ask for a pre-flash? > > > > Pre-flash can be relatively complicated as there are different ways to > > do it, and it may require specific support in the AGC/AEC algorithm. > > The AGC/AEC might be deciding exactly when to trigger it, and then > > doing calculations by comparing frames with/without the flash in order > > to calculate the optimal level for the final capture. Just want to be > > sure we're thinking ahead about these possibilities! > > If I understand correctly, there are no sequences for still image > capture yet, right? > This means that the application has to trigger the algorithms (AE, AWB, > AF, etc.), wait until all settings are complete or stable, and then take > a picture. > Therefore, the application would also have to take care of controlling > the flash—with everything that goes with it. Not 100% sure what you mean by "sequences for still image capture", but you're right it's worth thinking about what happens with all these algorithms and how things will be driven. In this respect it's the "pre-flash" case for still image capture that's most interesting. It will have to be closely integrated with AGC/AEC which will want to meter a "no flash" frame and then a "with flash" frame. The "with flash" frame needs to be completely illuminated, meaning the flash was turned on before the first pixel even starts being exposed. (Note I think we will need per frame metadata that tells us whether the flash was "off", "on" - wholly illuminating every pixel, or "in-between".) All this possibly affects which flash "modes" are useful in practice, so there's probably more to think about there too. > > I see the controls that are then used for such sequences at a higher > level and independent of these flash controls. Yes, I think this is probably the case if we can figure all these controls out! > > Admittedly, I certainly lack the necessary overall overview to > participate in such strategic decisions in a qualified manner. > > > > > > > >>> > >>>> > >>>>> + > >>>>> + - FlashIntensity: > >>>>> + type: int32_t > >>>>> + direction: inout > >>>>> + description: | > >>>>> + Flash intensity in mA. > > > > I'm dubious about using mA here, it feels a bit close to the hardware > > to me! Would a percentage of the maximum be more application friendly? > > > > Then there might also be (for example) a camera property that gives > > you some idea of the absolute brightness (in lux?) of the flash at > > maximum intensity. > > > >>>>> + > >>>>> + - FlashTimeout: > >>>>> + type: int32_t > >>>>> + direction: inout > >>>>> + description: | > >>>>> + Flash timeout in us. > >>>>> + > >>>>> + - FlashStrobeSource: > >>>>> + type: int32_t > >>>>> + direction: inout > >>>>> + description: | > >>>>> + Flash mode. > >>>> > >>>> Flash mode ? Or Flash source? > >>> > >>> True. Seems that all descriptions need some TLC... > >>> > >>>> > >>>> With all the descriptions expanded, these can target mainline controls. > >>>> No need to go to draft. > >>>> > >>>> We'll need to convey relationships in the documentation too.. like > >>>> perhaps it could be documented how FlashTimeout I suspect interacts only > >>>> with FlashStrobeSourceSoftware ? ... > >>> > >>> FlashTimeout is the maximum permissible flash on-time and is guaranteed > >>> by the hardware. This applies to both the software and hardware > >>> (external) triggers. It is also possible to generate shorter flash times > >>> using the triggers. The timeout is essentially a protective mechanism > >>> that limits the maximum duration. > >>> > >>>> > >>>> > >>>>> + enum: > >>>>> + - name: FlashStrobeSourceSoftware > >>>>> + value: 0 > >>>>> + description: | > >>>>> + Software. > >>>>> + - name: FlashStrobeSourceExternal > >>>>> + value: 1 > >>>>> + description: | > >>>>> + External. > >>>>> + > >>>>> + - FlashStrobe: > >>>>> + type: int32_t > >>>>> + direction: in > >>>>> + description: | > >>>>> + Start/stop flash strobe. > >>>>> + > >>>>> + enum: > >>>>> + - name: FlashStrobeStart > >>>>> + value: 0 > >>>>> + description: | > >>>>> + Start flash strobe. > >>>>> + > >>>>> + - name: FlashStrobeStop > >>>>> + value: 1 > >>>>> + description: | > >>>>> + Stop flash strobe. > >>>>> + > >>>>> + - FlashTorchIntensity: > >>>> > >>>> Do we need this in addition to FlashIntensity ? > >>>> > >>>> Will we have different intensity limits depending on the mode between > >>>> flash+torch? > >>> > >>> Yes, exactly. Depending on the hardware setup, the maximum currents in > >>> the two modes can differ. Continuous operation is possible in torch > >>> mode, which typically allows a lower maximum current (often for thermal > >>> reasons). > >>>> > >>>>> + type: int32_t > >>>>> + direction: inout > >>>>> + description: | > >>>>> + Torch intensity in mA. > >>>> > >>>> I guess we get mA from the kernel drivers? > >>> > >>> Yes, the API documentation [1] for V4L2_CID_FLASH_INTENSITY says: "[..] > >>> The unit should be milliamps (mA) if possible. [..]" > >> > >> Should we consider changing this to, say, a percentage? The IPA can > >> handle conversion from this to mA when setting the V4L2 ctrl. That > >> way, applications do not have to do anything different when dealing > >> with different devices with different limits. > > > > Agree - see my remarks above! > > See also my comment in response to Naushir. Yes, to be fair, I don't feel super-strongly so long as applications have the information they really need. This probably means knowing what the real physical effect of the maximum flash is (in terms of light produced), and being able to calculate a proportion of it that gives you a linear response. So that you could, for example, ask for 50% illumination, which increases pixel brightness by only half the maximum compared to no flash at all. (Note that this may require calibration of the driver current vs. illumination response, so maybe "flash_helpers" as well as "cam_helpers"??) David > > Thanks > ~Matthias > > > > >> > >> Regards, > >> Naush > >> > >>> > >>> Best regards > >>> ~Matthias > > > > Thanks! > > > > David > > > >>> > >>> [1] > >>> https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-flash.html > >>> > >>>> > >>>>> + > >>>>> ... > >>>>> > >>>>> -- > >>>>> 2.34.1 > >>>>> > >>> >
diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml index 03309eeac34fa76eee4bb5d1c87d6467b890c9a7..c10b774bfd59b26980475bb9706fffa6961b3b1b 100644 --- a/src/libcamera/control_ids_draft.yaml +++ b/src/libcamera/control_ids_draft.yaml @@ -294,4 +294,73 @@ controls: Currently identical to ANDROID_STATISTICS_FACE_IDS. size: [n] + - FlashMode: + type: int32_t + direction: inout + description: | + Flash mode. + enum: + - name: FlashModeNone + value: 0 + description: | + None. + - name: FlashModeFlash + value: 1 + description: | + Flash. + - name: FlashModeTorch + value: 2 + description: | + Torch. + + - FlashIntensity: + type: int32_t + direction: inout + description: | + Flash intensity in mA. + + - FlashTimeout: + type: int32_t + direction: inout + description: | + Flash timeout in us. + + - FlashStrobeSource: + type: int32_t + direction: inout + description: | + Flash mode. + enum: + - name: FlashStrobeSourceSoftware + value: 0 + description: | + Software. + - name: FlashStrobeSourceExternal + value: 1 + description: | + External. + + - FlashStrobe: + type: int32_t + direction: in + description: | + Start/stop flash strobe. + + enum: + - name: FlashStrobeStart + value: 0 + description: | + Start flash strobe. + + - name: FlashStrobeStop + value: 1 + description: | + Stop flash strobe. + + - FlashTorchIntensity: + type: int32_t + direction: inout + description: | + Torch intensity in mA. + ...
Define a set of controls to control camera flash devices. Signed-off-by: Matthias Fend <matthias.fend@emfend.at> --- src/libcamera/control_ids_draft.yaml | 69 ++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)