| Message ID | 20220622102047.22492-4-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | 9dacde0d651df322058c3611b3515820bf06715e |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush Thanks for the patch. LGTM! Reviewed-by: David Plowman <david.plowman@raspberrypi.com> David On Wed, 22 Jun 2022 at 11:21, Naushir Patuck via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > The ScalerCrop control is handled directly by the pipeline handler. Remove the > control from the IPA's static ControlInfoMap, and let the pipeline handler add > it to the ControlInfoMap advertised to the application, ensuring the limits > are set appropriately based on the current sensor mode. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 4596f2babcea..66a84b1dfb97 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -941,7 +941,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > data->properties_.set(properties::SensorSensitivity, result.modeSensitivity); > > /* Update the controls that the Raspberry Pi IPA can handle. */ > - data->controlInfo_ = result.controlInfo; > + ControlInfoMap::Map ctrlMap; > + for (auto const &c : result.controlInfo) > + ctrlMap.emplace(c.first, c.second); > + > + /* Add the ScalerCrop control limits based on the current mode. */ > + ctrlMap.emplace(&controls::ScalerCrop, > + ControlInfo(Rectangle(data->ispMinCropSize_), Rectangle(data->sensorInfo_.outputSize))); > + > + data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap()); > > /* Setup the Video Mux/Bridge entities. */ > for (auto &[device, link] : data->bridgeDevices_) { > -- > 2.25.1 >
Quoting Naushir Patuck via libcamera-devel (2022-06-22 11:20:47) > The ScalerCrop control is handled directly by the pipeline handler. Remove the > control from the IPA's static ControlInfoMap, and let the pipeline handler add > it to the ControlInfoMap advertised to the application, ensuring the limits > are set appropriately based on the current sensor mode. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 4596f2babcea..66a84b1dfb97 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -941,7 +941,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > data->properties_.set(properties::SensorSensitivity, result.modeSensitivity); > > /* Update the controls that the Raspberry Pi IPA can handle. */ > - data->controlInfo_ = result.controlInfo; > + ControlInfoMap::Map ctrlMap; > + for (auto const &c : result.controlInfo) > + ctrlMap.emplace(c.first, c.second); Sorry - I thought I'd handled this patch already. Yes, I see my concerns about emplace here were misfounded, as it's a new list each time - so it's fine for now. A more defined mechanism to update them might be beneficial, but it's internal so can be handled later. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + > + /* Add the ScalerCrop control limits based on the current mode. */ > + ctrlMap.emplace(&controls::ScalerCrop, > + ControlInfo(Rectangle(data->ispMinCropSize_), Rectangle(data->sensorInfo_.outputSize))); > + > + data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap()); > > /* Setup the Video Mux/Bridge entities. */ > for (auto &[device, link] : data->bridgeDevices_) { > -- > 2.25.1 >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 4596f2babcea..66a84b1dfb97 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -941,7 +941,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) data->properties_.set(properties::SensorSensitivity, result.modeSensitivity); /* Update the controls that the Raspberry Pi IPA can handle. */ - data->controlInfo_ = result.controlInfo; + ControlInfoMap::Map ctrlMap; + for (auto const &c : result.controlInfo) + ctrlMap.emplace(c.first, c.second); + + /* Add the ScalerCrop control limits based on the current mode. */ + ctrlMap.emplace(&controls::ScalerCrop, + ControlInfo(Rectangle(data->ispMinCropSize_), Rectangle(data->sensorInfo_.outputSize))); + + data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap()); /* Setup the Video Mux/Bridge entities. */ for (auto &[device, link] : data->bridgeDevices_) {
The ScalerCrop control is handled directly by the pipeline handler. Remove the control from the IPA's static ControlInfoMap, and let the pipeline handler add it to the ControlInfoMap advertised to the application, ensuring the limits are set appropriately based on the current sensor mode. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)