| Message ID | 20250402114928.937042-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Wed, Apr 02, 2025 at 01:49:28PM +0200, Barnabás Pőcze wrote: > The `AeEnable` control is handled by the `Camera` class directly, but it > still has to be added because `ControlInfoMap`s are not easily modifiable. > > See 338ba00e7abfe8 ("ipa: rkisp1: agc: Report new AeEnable control as available") > for more details and a similar change in rkisp1. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 5adc89fdb..ab180e820 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -583,6 +583,9 @@ int UVCCameraData::init(MediaDevice *media) > /* Initialise the supported controls. */ > ControlInfoMap::Map ctrls; > > + /* \todo Move this to the Camera class */ > + ctrls[&controls::AeEnable] = ControlInfo(false, true, true); > + The patch is fine, I wonder however if we shouldn't just be adding this to all pipelines. My understanding is that we can't do it in the Camera class a controls is a const, so once populated by the pipeline it can't be modified. However at the moment 4 pipeline handlers (mali, rkisp1, rpi and now uvc) registers this, other pipelines (the ipu3 one in particular) only expose ExposureTimeMode and AnalogueGainMode. For this patch Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > for (const auto &ctrl : video_->controls()) { > uint32_t cid = ctrl.first->id(); > const ControlInfo &info = ctrl.second; > -- > 2.49.0 >
On Wed, Apr 02, 2025 at 01:49:28PM +0200, Barnabás Pőcze wrote: > The `AeEnable` control is handled by the `Camera` class directly, but it > still has to be added because `ControlInfoMap`s are not easily modifiable. > > See 338ba00e7abfe8 ("ipa: rkisp1: agc: Report new AeEnable control as available") > for more details and a similar change in rkisp1. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 5adc89fdb..ab180e820 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -583,6 +583,9 @@ int UVCCameraData::init(MediaDevice *media) > /* Initialise the supported controls. */ > ControlInfoMap::Map ctrls; > > + /* \todo Move this to the Camera class */ > + ctrls[&controls::AeEnable] = ControlInfo(false, true, true); Shouldn't this be done only for camera that support the V4L2_CID_EXPOSURE_AUTO control ? > + > for (const auto &ctrl : video_->controls()) { > uint32_t cid = ctrl.first->id(); > const ControlInfo &info = ctrl.second;
2025. 04. 02. 16:40 keltezéssel, Laurent Pinchart írta: > On Wed, Apr 02, 2025 at 01:49:28PM +0200, Barnabás Pőcze wrote: >> The `AeEnable` control is handled by the `Camera` class directly, but it >> still has to be added because `ControlInfoMap`s are not easily modifiable. >> >> See 338ba00e7abfe8 ("ipa: rkisp1: agc: Report new AeEnable control as available") >> for more details and a similar change in rkisp1. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> index 5adc89fdb..ab180e820 100644 >> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> @@ -583,6 +583,9 @@ int UVCCameraData::init(MediaDevice *media) >> /* Initialise the supported controls. */ >> ControlInfoMap::Map ctrls; >> >> + /* \todo Move this to the Camera class */ >> + ctrls[&controls::AeEnable] = ControlInfo(false, true, true); > > Shouldn't this be done only for camera that support the > V4L2_CID_EXPOSURE_AUTO control ? Ahh, yes, indeed, you're right. > >> + >> for (const auto &ctrl : video_->controls()) { >> uint32_t cid = ctrl.first->id(); >> const ControlInfo &info = ctrl.second; >
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 5adc89fdb..ab180e820 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -583,6 +583,9 @@ int UVCCameraData::init(MediaDevice *media) /* Initialise the supported controls. */ ControlInfoMap::Map ctrls; + /* \todo Move this to the Camera class */ + ctrls[&controls::AeEnable] = ControlInfo(false, true, true); + for (const auto &ctrl : video_->controls()) { uint32_t cid = ctrl.first->id(); const ControlInfo &info = ctrl.second;
The `AeEnable` control is handled by the `Camera` class directly, but it still has to be added because `ControlInfoMap`s are not easily modifiable. See 338ba00e7abfe8 ("ipa: rkisp1: agc: Report new AeEnable control as available") for more details and a similar change in rkisp1. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +++ 1 file changed, 3 insertions(+)