| Message ID | 20220621150337.47839-4-jacopo@jmondi.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for the patch ! On 21/06/2022 17:03, Jacopo Mondi via libcamera-devel wrote: > Introduce the enumeration of internal controls in > internal_control_ids.yaml. > > The list of controls currently defines 4 draft controls which mirror the > definition of the V4L2 control they map to. > > Plumb in the build system the command to generate the definition of > internal controls by re-using the same mechanism used for public > controls to make it easy to extend it to also handle internal properties > in future. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/internal/meson.build | 18 +++++++++ > src/libcamera/internal_control_ids.yaml | 54 +++++++++++++++++++++++++ > src/libcamera/meson.build | 16 ++++++++ > 3 files changed, 88 insertions(+) > create mode 100644 src/libcamera/internal_control_ids.yaml > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 7a780d48ee57..569940b0d2e8 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -44,3 +44,21 @@ libcamera_internal_headers = files([ > 'v4l2_videodevice.h', > 'yaml_parser.h', > ]) > + > +# Generate the list of internal controls identifiers > +internal_control_source_files = ['control_ids'] > + > +internal_control_headers = [] > + > +foreach header : internal_control_source_files > + input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\ > + '../' + header + '.h.in') > + internal_control_headers += custom_target('internal_' + header + '_h', > + input : input_files, > + output : header + '.h', > + command : [gen_controls, '--internal=True',\ > + '-o', '@OUTPUT@',\ > + '@INPUT@'], > + install : false) > +endforeach > +libcamera_internal_headers += internal_control_headers > diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml > new file mode 100644 > index 000000000000..e69e0d30657c > --- /dev/null > +++ b/src/libcamera/internal_control_ids.yaml > @@ -0,0 +1,54 @@ > +# SPDX-License-Identifier: LGPL-2.1-or-later > +# > +# Copyright (C) 2022, Google Inc. > +# > +%YAML 1.2 > +--- > +# Enumeration of internal libcamera controls > +# Not exposed to application, for library use only > + > +controls: > + > + # ---------------------------------------------------------------------------- > + # Draft controls section > + > + - VBlank: > + type: int32_t > + draft: true > + description: | > + Vertical blanking. The idle period after every frame during which no > + image data is produced. The unit of vertical blanking is a line. Every > + line has length of the image width plus horizontal blanking at the pixel > + rate defined by V4L2_CID_PIXEL_RATE control in the same sub-device. > + > + Currently identical to V4L2_CID_VBLANK. > + > + - HBlank: > + type: int32_t > + draft: true > + description: | > + Horizontal blanking. The idle period after every line of image data > + during which no image data is produced. The unit of horizontal blanking > + is pixels. > + > + Currently identical to V4L2_CID_HBLANK. > + > + - SensorAnalogueGain: > + type: int32_t > + draft: true > + description: | > + Analogue gain is gain affecting all colour components in the pixel > + matrix. The gain operation is performed in the analogue domain before > + A/D conversion > + > + Currently identical to V4L2_CID_ANALOGUE_GAIN. > + > + - SensorExposure: > + type: int32_t > + draft: true > + description: | > + Exposure time, expressed in frame lines. > + > + Currently identical to V4L2_CID_EXPOSURE. I have a general comment here, could probably be on top of the series. I think we should convey the SensorAnalogueGain as a double value and let the CameraSensor call the helper to convert it in a gain code. Thus, the control would be Gain, and the CameraSensor class (or a V4L2 specific version of it ;-)) would then convert this gain into Analogue Gain + digital gain. Same would be true for the SensorExposure control, as we could convey the exposure time we want, and it would be converted accordingly into VBLANK + exposure (and then would deal with VBLANK beeing called before calculating the exposure value in lines with the updated maximum). Thus, VBLANK would not be a control for IPA at all, and we would have Gain and ExposureTime (for instance) only ? I don't think it should be done on this series though, as I said, so with or without: Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> > + > +... > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index b57bee7ef6ca..4564d3dbb835 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -99,6 +99,7 @@ if not libyaml.found() > libyaml = libyaml_wrap.dependency('yaml') > endif > > +# Generate control_ids.cpp and property_ids.cpp > control_sources = [] > > foreach source : control_source_files > @@ -111,6 +112,21 @@ endforeach > > libcamera_sources += control_sources > > +# Generate internal_control_ids.cpp > +internal_control_source_files = ['control_ids'] > +internal_control_sources = [] > + > +foreach source : internal_control_source_files > + input_files = files('internal_' + source +'.yaml', source + '.cpp.in') > + internal_control_sources += custom_target('internal_' + source + '_cpp', > + input : input_files, > + output : 'internal_' + source + '.cpp', > + command : [gen_controls, '--internal=True',\ > + '-o', '@OUTPUT@',\ > + '@INPUT@']) > +endforeach > +libcamera_sources += internal_control_sources > + > gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > > # Use vcs_tag() and not configure_file() or run_command(), to ensure that the
Hi Jean-Michel, On Tue, Jun 21, 2022 at 08:20:27PM +0200, Jean-Michel Hautbois wrote: > Hi Jacopo, > > Thanks for the patch ! > > On 21/06/2022 17:03, Jacopo Mondi via libcamera-devel wrote: > > Introduce the enumeration of internal controls in > > internal_control_ids.yaml. > > > > The list of controls currently defines 4 draft controls which mirror the > > definition of the V4L2 control they map to. > > > > Plumb in the build system the command to generate the definition of > > internal controls by re-using the same mechanism used for public > > controls to make it easy to extend it to also handle internal properties > > in future. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > include/libcamera/internal/meson.build | 18 +++++++++ > > src/libcamera/internal_control_ids.yaml | 54 +++++++++++++++++++++++++ > > src/libcamera/meson.build | 16 ++++++++ > > 3 files changed, 88 insertions(+) > > create mode 100644 src/libcamera/internal_control_ids.yaml > > > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > index 7a780d48ee57..569940b0d2e8 100644 > > --- a/include/libcamera/internal/meson.build > > +++ b/include/libcamera/internal/meson.build > > @@ -44,3 +44,21 @@ libcamera_internal_headers = files([ > > 'v4l2_videodevice.h', > > 'yaml_parser.h', > > ]) > > + > > +# Generate the list of internal controls identifiers > > +internal_control_source_files = ['control_ids'] > > + > > +internal_control_headers = [] > > + > > +foreach header : internal_control_source_files > > + input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\ > > + '../' + header + '.h.in') > > + internal_control_headers += custom_target('internal_' + header + '_h', > > + input : input_files, > > + output : header + '.h', > > + command : [gen_controls, '--internal=True',\ > > + '-o', '@OUTPUT@',\ > > + '@INPUT@'], > > + install : false) > > +endforeach > > +libcamera_internal_headers += internal_control_headers > > diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml > > new file mode 100644 > > index 000000000000..e69e0d30657c > > --- /dev/null > > +++ b/src/libcamera/internal_control_ids.yaml > > @@ -0,0 +1,54 @@ > > +# SPDX-License-Identifier: LGPL-2.1-or-later > > +# > > +# Copyright (C) 2022, Google Inc. > > +# > > +%YAML 1.2 > > +--- > > +# Enumeration of internal libcamera controls > > +# Not exposed to application, for library use only > > + > > +controls: > > + > > + # ---------------------------------------------------------------------------- > > + # Draft controls section > > + > > + - VBlank: > > + type: int32_t > > + draft: true > > + description: | > > + Vertical blanking. The idle period after every frame during which no > > + image data is produced. The unit of vertical blanking is a line. Every > > + line has length of the image width plus horizontal blanking at the pixel > > + rate defined by V4L2_CID_PIXEL_RATE control in the same sub-device. > > + > > + Currently identical to V4L2_CID_VBLANK. > > + > > + - HBlank: > > + type: int32_t > > + draft: true > > + description: | > > + Horizontal blanking. The idle period after every line of image data > > + during which no image data is produced. The unit of horizontal blanking > > + is pixels. > > + > > + Currently identical to V4L2_CID_HBLANK. > > + > > + - SensorAnalogueGain: > > + type: int32_t > > + draft: true > > + description: | > > + Analogue gain is gain affecting all colour components in the pixel > > + matrix. The gain operation is performed in the analogue domain before > > + A/D conversion > > + > > + Currently identical to V4L2_CID_ANALOGUE_GAIN. > > + > > + - SensorExposure: > > + type: int32_t > > + draft: true > > + description: | > > + Exposure time, expressed in frame lines. > > + > > + Currently identical to V4L2_CID_EXPOSURE. > > I have a general comment here, could probably be on top of the series. I > think we should convey the SensorAnalogueGain as a double value and let the > CameraSensor call the helper to convert it in a gain code. Thus, the control > would be Gain, and the CameraSensor class (or a V4L2 specific version of it > ;-)) would then convert this gain into Analogue Gain + digital gain. Ah! I copied the type from the V4L2 control definition! I see your point, but doesn't IPA have access to the CameraSensorHelper class exactly for this purpose (convert the gain value into gain code) ? Now, I would dare sayin that the IPA could be even detached from any sensor-related knowledge and CameraSensorHelpers moved to the PH/CameraSensor side, so that IPAs only calculate the "analogue" value and the CameraSensor class computes the gain code. Is this the direction where we want to go ? > > Same would be true for the SensorExposure control, as we could convey the > exposure time we want, and it would be converted accordingly into > VBLANK + exposure (and then would deal with VBLANK beeing called before > calculating the exposure value in lines with the updated maximum). The same reasoning applies here, see CamHelperImx477::GetVBlanking(). There's a camera-sensor specific part that would require moving camera sensor helper on the PH side. To be honest this does make sense to me (without too much thinking, I admit, there might be drawbacks I'm not seeing atm), but it's a rather big change indeed! > > Thus, VBLANK would not be a control for IPA at all, and we would have Gain > and ExposureTime (for instance) only ? > > I don't think it should be done on this series though, as I said, so with or > without: > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> > > > + > > +... > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index b57bee7ef6ca..4564d3dbb835 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -99,6 +99,7 @@ if not libyaml.found() > > libyaml = libyaml_wrap.dependency('yaml') > > endif > > +# Generate control_ids.cpp and property_ids.cpp > > control_sources = [] > > foreach source : control_source_files > > @@ -111,6 +112,21 @@ endforeach > > libcamera_sources += control_sources > > +# Generate internal_control_ids.cpp > > +internal_control_source_files = ['control_ids'] > > +internal_control_sources = [] > > + > > +foreach source : internal_control_source_files > > + input_files = files('internal_' + source +'.yaml', source + '.cpp.in') > > + internal_control_sources += custom_target('internal_' + source + '_cpp', > > + input : input_files, > > + output : 'internal_' + source + '.cpp', > > + command : [gen_controls, '--internal=True',\ > > + '-o', '@OUTPUT@',\ > > + '@INPUT@']) > > +endforeach > > +libcamera_sources += internal_control_sources > > + > > gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > > # Use vcs_tag() and not configure_file() or run_command(), to ensure that the
Hi Jacopo, On 22/06/2022 09:06, Jacopo Mondi wrote: > Hi Jean-Michel, > > On Tue, Jun 21, 2022 at 08:20:27PM +0200, Jean-Michel Hautbois wrote: >> Hi Jacopo, >> >> Thanks for the patch ! >> >> On 21/06/2022 17:03, Jacopo Mondi via libcamera-devel wrote: >>> Introduce the enumeration of internal controls in >>> internal_control_ids.yaml. >>> >>> The list of controls currently defines 4 draft controls which mirror the >>> definition of the V4L2 control they map to. >>> >>> Plumb in the build system the command to generate the definition of >>> internal controls by re-using the same mechanism used for public >>> controls to make it easy to extend it to also handle internal properties >>> in future. >>> >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> >>> --- >>> include/libcamera/internal/meson.build | 18 +++++++++ >>> src/libcamera/internal_control_ids.yaml | 54 +++++++++++++++++++++++++ >>> src/libcamera/meson.build | 16 ++++++++ >>> 3 files changed, 88 insertions(+) >>> create mode 100644 src/libcamera/internal_control_ids.yaml >>> >>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build >>> index 7a780d48ee57..569940b0d2e8 100644 >>> --- a/include/libcamera/internal/meson.build >>> +++ b/include/libcamera/internal/meson.build >>> @@ -44,3 +44,21 @@ libcamera_internal_headers = files([ >>> 'v4l2_videodevice.h', >>> 'yaml_parser.h', >>> ]) >>> + >>> +# Generate the list of internal controls identifiers >>> +internal_control_source_files = ['control_ids'] >>> + >>> +internal_control_headers = [] >>> + >>> +foreach header : internal_control_source_files >>> + input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\ >>> + '../' + header + '.h.in') >>> + internal_control_headers += custom_target('internal_' + header + '_h', >>> + input : input_files, >>> + output : header + '.h', >>> + command : [gen_controls, '--internal=True',\ >>> + '-o', '@OUTPUT@',\ >>> + '@INPUT@'], >>> + install : false) >>> +endforeach >>> +libcamera_internal_headers += internal_control_headers >>> diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml >>> new file mode 100644 >>> index 000000000000..e69e0d30657c >>> --- /dev/null >>> +++ b/src/libcamera/internal_control_ids.yaml >>> @@ -0,0 +1,54 @@ >>> +# SPDX-License-Identifier: LGPL-2.1-or-later >>> +# >>> +# Copyright (C) 2022, Google Inc. >>> +# >>> +%YAML 1.2 >>> +--- >>> +# Enumeration of internal libcamera controls >>> +# Not exposed to application, for library use only >>> + >>> +controls: >>> + >>> + # ---------------------------------------------------------------------------- >>> + # Draft controls section >>> + >>> + - VBlank: >>> + type: int32_t >>> + draft: true >>> + description: | >>> + Vertical blanking. The idle period after every frame during which no >>> + image data is produced. The unit of vertical blanking is a line. Every >>> + line has length of the image width plus horizontal blanking at the pixel >>> + rate defined by V4L2_CID_PIXEL_RATE control in the same sub-device. >>> + >>> + Currently identical to V4L2_CID_VBLANK. >>> + >>> + - HBlank: >>> + type: int32_t >>> + draft: true >>> + description: | >>> + Horizontal blanking. The idle period after every line of image data >>> + during which no image data is produced. The unit of horizontal blanking >>> + is pixels. >>> + >>> + Currently identical to V4L2_CID_HBLANK. >>> + >>> + - SensorAnalogueGain: >>> + type: int32_t >>> + draft: true >>> + description: | >>> + Analogue gain is gain affecting all colour components in the pixel >>> + matrix. The gain operation is performed in the analogue domain before >>> + A/D conversion >>> + >>> + Currently identical to V4L2_CID_ANALOGUE_GAIN. >>> + >>> + - SensorExposure: >>> + type: int32_t >>> + draft: true >>> + description: | >>> + Exposure time, expressed in frame lines. >>> + >>> + Currently identical to V4L2_CID_EXPOSURE. >> >> I have a general comment here, could probably be on top of the series. I >> think we should convey the SensorAnalogueGain as a double value and let the >> CameraSensor call the helper to convert it in a gain code. Thus, the control >> would be Gain, and the CameraSensor class (or a V4L2 specific version of it >> ;-)) would then convert this gain into Analogue Gain + digital gain. > > Ah! I copied the type from the V4L2 control definition! > > I see your point, but doesn't IPA have access to the CameraSensorHelper class > exactly for this purpose (convert the gain value into gain code) ? > > Now, I would dare sayin that the IPA could be even detached from any > sensor-related knowledge and CameraSensorHelpers moved to the > PH/CameraSensor side, so that IPAs only calculate the "analogue" value > and the CameraSensor class computes the gain code. > > Is this the direction where we want to go ? It is the direction I would like it to go, decoupling the IPA from V4L2 controls. They can't be totally agnostic from the hardware, as they take statistics and compute values based on it to send the parameters back, but on the AGC side at least, it does not need to know what the driver can do at all. It just need to know the min/max it can apply for the gain (and time) that's all, I think. > >> >> Same would be true for the SensorExposure control, as we could convey the >> exposure time we want, and it would be converted accordingly into >> VBLANK + exposure (and then would deal with VBLANK beeing called before >> calculating the exposure value in lines with the updated maximum). > > The same reasoning applies here, see CamHelperImx477::GetVBlanking(). > There's a camera-sensor specific part that would require moving camera > sensor helper on the PH side. To be honest this does make sense to me > (without too much thinking, I admit, there might be drawbacks I'm not > seeing atm), but it's a rather big change indeed! Yes, I don't expect it now :-). Maybe is it worth a more in-depth discussion ? JM > >> >> Thus, VBLANK would not be a control for IPA at all, and we would have Gain >> and ExposureTime (for instance) only ? >> >> I don't think it should be done on this series though, as I said, so with or >> without: >> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> >> >>> + >>> +... >>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build >>> index b57bee7ef6ca..4564d3dbb835 100644 >>> --- a/src/libcamera/meson.build >>> +++ b/src/libcamera/meson.build >>> @@ -99,6 +99,7 @@ if not libyaml.found() >>> libyaml = libyaml_wrap.dependency('yaml') >>> endif >>> +# Generate control_ids.cpp and property_ids.cpp >>> control_sources = [] >>> foreach source : control_source_files >>> @@ -111,6 +112,21 @@ endforeach >>> libcamera_sources += control_sources >>> +# Generate internal_control_ids.cpp >>> +internal_control_source_files = ['control_ids'] >>> +internal_control_sources = [] >>> + >>> +foreach source : internal_control_source_files >>> + input_files = files('internal_' + source +'.yaml', source + '.cpp.in') >>> + internal_control_sources += custom_target('internal_' + source + '_cpp', >>> + input : input_files, >>> + output : 'internal_' + source + '.cpp', >>> + command : [gen_controls, '--internal=True',\ >>> + '-o', '@OUTPUT@',\ >>> + '@INPUT@']) >>> +endforeach >>> +libcamera_sources += internal_control_sources >>> + >>> gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' >>> # Use vcs_tag() and not configure_file() or run_command(), to ensure that the
+Kieran +Umang On Wed, Jun 22, 2022 at 09:41:55AM +0200, Jean-Michel Hautbois wrote: > Hi Jacopo, > > On 22/06/2022 09:06, Jacopo Mondi wrote: > > Hi Jean-Michel, > > > > On Tue, Jun 21, 2022 at 08:20:27PM +0200, Jean-Michel Hautbois wrote: > > > Hi Jacopo, > > > > > > Thanks for the patch ! > > > > > > On 21/06/2022 17:03, Jacopo Mondi via libcamera-devel wrote: > > > > Introduce the enumeration of internal controls in > > > > internal_control_ids.yaml. > > > > > > > > The list of controls currently defines 4 draft controls which mirror the > > > > definition of the V4L2 control they map to. > > > > > > > > Plumb in the build system the command to generate the definition of > > > > internal controls by re-using the same mechanism used for public > > > > controls to make it easy to extend it to also handle internal properties > > > > in future. > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > > --- > > > > include/libcamera/internal/meson.build | 18 +++++++++ > > > > src/libcamera/internal_control_ids.yaml | 54 +++++++++++++++++++++++++ > > > > src/libcamera/meson.build | 16 ++++++++ > > > > 3 files changed, 88 insertions(+) > > > > create mode 100644 src/libcamera/internal_control_ids.yaml > > > > > > > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > > > index 7a780d48ee57..569940b0d2e8 100644 > > > > --- a/include/libcamera/internal/meson.build > > > > +++ b/include/libcamera/internal/meson.build > > > > @@ -44,3 +44,21 @@ libcamera_internal_headers = files([ > > > > 'v4l2_videodevice.h', > > > > 'yaml_parser.h', > > > > ]) > > > > + > > > > +# Generate the list of internal controls identifiers > > > > +internal_control_source_files = ['control_ids'] > > > > + > > > > +internal_control_headers = [] > > > > + > > > > +foreach header : internal_control_source_files > > > > + input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\ > > > > + '../' + header + '.h.in') > > > > + internal_control_headers += custom_target('internal_' + header + '_h', > > > > + input : input_files, > > > > + output : header + '.h', > > > > + command : [gen_controls, '--internal=True',\ > > > > + '-o', '@OUTPUT@',\ > > > > + '@INPUT@'], > > > > + install : false) > > > > +endforeach > > > > +libcamera_internal_headers += internal_control_headers > > > > diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml > > > > new file mode 100644 > > > > index 000000000000..e69e0d30657c > > > > --- /dev/null > > > > +++ b/src/libcamera/internal_control_ids.yaml > > > > @@ -0,0 +1,54 @@ > > > > +# SPDX-License-Identifier: LGPL-2.1-or-later > > > > +# > > > > +# Copyright (C) 2022, Google Inc. > > > > +# > > > > +%YAML 1.2 > > > > +--- > > > > +# Enumeration of internal libcamera controls > > > > +# Not exposed to application, for library use only > > > > + > > > > +controls: > > > > + > > > > + # ---------------------------------------------------------------------------- > > > > + # Draft controls section > > > > + > > > > + - VBlank: > > > > + type: int32_t > > > > + draft: true > > > > + description: | > > > > + Vertical blanking. The idle period after every frame during which no > > > > + image data is produced. The unit of vertical blanking is a line. Every > > > > + line has length of the image width plus horizontal blanking at the pixel > > > > + rate defined by V4L2_CID_PIXEL_RATE control in the same sub-device. > > > > + > > > > + Currently identical to V4L2_CID_VBLANK. > > > > + > > > > + - HBlank: > > > > + type: int32_t > > > > + draft: true > > > > + description: | > > > > + Horizontal blanking. The idle period after every line of image data > > > > + during which no image data is produced. The unit of horizontal blanking > > > > + is pixels. > > > > + > > > > + Currently identical to V4L2_CID_HBLANK. > > > > + > > > > + - SensorAnalogueGain: > > > > + type: int32_t > > > > + draft: true > > > > + description: | > > > > + Analogue gain is gain affecting all colour components in the pixel > > > > + matrix. The gain operation is performed in the analogue domain before > > > > + A/D conversion > > > > + > > > > + Currently identical to V4L2_CID_ANALOGUE_GAIN. > > > > + > > > > + - SensorExposure: > > > > + type: int32_t > > > > + draft: true > > > > + description: | > > > > + Exposure time, expressed in frame lines. > > > > + > > > > + Currently identical to V4L2_CID_EXPOSURE. > > > > > > I have a general comment here, could probably be on top of the series. I > > > think we should convey the SensorAnalogueGain as a double value and let the > > > CameraSensor call the helper to convert it in a gain code. Thus, the control > > > would be Gain, and the CameraSensor class (or a V4L2 specific version of it > > > ;-)) would then convert this gain into Analogue Gain + digital gain. > > > > Ah! I copied the type from the V4L2 control definition! > > > > I see your point, but doesn't IPA have access to the CameraSensorHelper class > > exactly for this purpose (convert the gain value into gain code) ? > > > > Now, I would dare sayin that the IPA could be even detached from any > > sensor-related knowledge and CameraSensorHelpers moved to the > > PH/CameraSensor side, so that IPAs only calculate the "analogue" value > > and the CameraSensor class computes the gain code. > > > > Is this the direction where we want to go ? > > It is the direction I would like it to go, decoupling the IPA from V4L2 > controls. They can't be totally agnostic from the hardware, as they take > statistics and compute values based on it to send the parameters back, but > on the AGC side at least, it does not need to know what the driver can do at > all. It just need to know the min/max it can apply for the gain (and time) > that's all, I think. > Any consideration about the closed-source IPA modules ?
diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 7a780d48ee57..569940b0d2e8 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -44,3 +44,21 @@ libcamera_internal_headers = files([ 'v4l2_videodevice.h', 'yaml_parser.h', ]) + +# Generate the list of internal controls identifiers +internal_control_source_files = ['control_ids'] + +internal_control_headers = [] + +foreach header : internal_control_source_files + input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\ + '../' + header + '.h.in') + internal_control_headers += custom_target('internal_' + header + '_h', + input : input_files, + output : header + '.h', + command : [gen_controls, '--internal=True',\ + '-o', '@OUTPUT@',\ + '@INPUT@'], + install : false) +endforeach +libcamera_internal_headers += internal_control_headers diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml new file mode 100644 index 000000000000..e69e0d30657c --- /dev/null +++ b/src/libcamera/internal_control_ids.yaml @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Copyright (C) 2022, Google Inc. +# +%YAML 1.2 +--- +# Enumeration of internal libcamera controls +# Not exposed to application, for library use only + +controls: + + # ---------------------------------------------------------------------------- + # Draft controls section + + - VBlank: + type: int32_t + draft: true + description: | + Vertical blanking. The idle period after every frame during which no + image data is produced. The unit of vertical blanking is a line. Every + line has length of the image width plus horizontal blanking at the pixel + rate defined by V4L2_CID_PIXEL_RATE control in the same sub-device. + + Currently identical to V4L2_CID_VBLANK. + + - HBlank: + type: int32_t + draft: true + description: | + Horizontal blanking. The idle period after every line of image data + during which no image data is produced. The unit of horizontal blanking + is pixels. + + Currently identical to V4L2_CID_HBLANK. + + - SensorAnalogueGain: + type: int32_t + draft: true + description: | + Analogue gain is gain affecting all colour components in the pixel + matrix. The gain operation is performed in the analogue domain before + A/D conversion + + Currently identical to V4L2_CID_ANALOGUE_GAIN. + + - SensorExposure: + type: int32_t + draft: true + description: | + Exposure time, expressed in frame lines. + + Currently identical to V4L2_CID_EXPOSURE. + +... diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index b57bee7ef6ca..4564d3dbb835 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -99,6 +99,7 @@ if not libyaml.found() libyaml = libyaml_wrap.dependency('yaml') endif +# Generate control_ids.cpp and property_ids.cpp control_sources = [] foreach source : control_source_files @@ -111,6 +112,21 @@ endforeach libcamera_sources += control_sources +# Generate internal_control_ids.cpp +internal_control_source_files = ['control_ids'] +internal_control_sources = [] + +foreach source : internal_control_source_files + input_files = files('internal_' + source +'.yaml', source + '.cpp.in') + internal_control_sources += custom_target('internal_' + source + '_cpp', + input : input_files, + output : 'internal_' + source + '.cpp', + command : [gen_controls, '--internal=True',\ + '-o', '@OUTPUT@',\ + '@INPUT@']) +endforeach +libcamera_sources += internal_control_sources + gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' # Use vcs_tag() and not configure_file() or run_command(), to ensure that the
Introduce the enumeration of internal controls in internal_control_ids.yaml. The list of controls currently defines 4 draft controls which mirror the definition of the V4L2 control they map to. Plumb in the build system the command to generate the definition of internal controls by re-using the same mechanism used for public controls to make it easy to extend it to also handle internal properties in future. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/internal/meson.build | 18 +++++++++ src/libcamera/internal_control_ids.yaml | 54 +++++++++++++++++++++++++ src/libcamera/meson.build | 16 ++++++++ 3 files changed, 88 insertions(+) create mode 100644 src/libcamera/internal_control_ids.yaml