<div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div><div>Thank you for your feedback,</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 27 Oct 2021 at 13:33, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Naushir Patuck (2021-10-27 10:28:03)<br>
> Sensor flips might change the Bayer order of the requested format. The existing<br>
> code would set a sensor format along with the appropriate Unicam and ISP input<br>
> formats, but reset the latter two on start() once the flips had been requested.<br>
> <br>
> We can now set the sensor flips just before we set the sensor mode in<br>
> configure(), thereby not needing the second pair of format sets in start().<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
<br>
Will the user be able to flip at runtime? I suspect they can't currently<br>
anyway so it doesn't matter, but anyway, I can't see anything wrong with<br>
this:<br></blockquote><div><br></div><div>No runtime changes of flips are not possible. That would be too complicated to deal with :-)</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
<br>
> ---<br>
> .../pipeline/raspberrypi/raspberrypi.cpp | 56 ++++++-------------<br>
> 1 file changed, 16 insertions(+), 40 deletions(-)<br>
> <br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index 52521857b61c..eaa2676b891e 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -657,16 +657,28 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
> }<br>
> }<br>
> <br>
> + /*<br>
> + * Configure the H/V flip controls based on the combination of<br>
> + * the sensor and user transform.<br>
> + */<br>
> + if (data->supportsFlips_) {<br>
> + const RPiCameraConfiguration *rpiConfig =<br>
> + static_cast<const RPiCameraConfiguration *>(config);<br>
> + ControlList controls;<br>
> +<br>
> + controls.set(V4L2_CID_HFLIP,<br>
> + static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));<br>
> + controls.set(V4L2_CID_VFLIP,<br>
> + static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));<br>
> + data->setSensorControls(controls);<br>
> + }<br>
> +<br>
> /* First calculate the best sensor mode we can use based on the user request. */<br>
> V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);<br>
> ret = data->sensor_->setFormat(&sensorFormat);<br>
> if (ret)<br>
> return ret;<br>
> <br>
> - /*<br>
> - * Unicam image output format. The ISP input format gets set at start,<br>
> - * just in case we have swapped bayer orders due to flips.<br>
> - */<br>
> V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing);<br>
> ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);<br>
> if (ret)<br>
> @@ -676,10 +688,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
> << " - Selected sensor format: " << sensorFormat.toString()<br>
> << " - Selected unicam format: " << unicamFormat.toString();<br>
> <br>
> - /*<br>
> - * This format may be reset on start() if the bayer order has changed<br>
> - * because of flips in the sensor.<br>
> - */<br>
> ret = data->isp_[Isp::Input].dev()->setFormat(&unicamFormat);<br>
> if (ret)<br>
> return ret;<br>
> @@ -902,23 +910,6 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)<br>
> return ret;<br>
> }<br>
> <br>
> - /*<br>
> - * IPA configure may have changed the sensor flips - hence the bayer<br>
> - * order. Get the sensor format and set the ISP input now.<br>
> - */<br>
> - V4L2SubdeviceFormat sensorFormat;<br>
> - data->sensor_->device()->getFormat(0, &sensorFormat);<br>
> -<br>
> - V4L2DeviceFormat ispFormat;<br>
> - ispFormat.fourcc = mbusCodeToBayerFormat(sensorFormat.mbus_code).toV4L2PixelFormat();<br>
> - ispFormat.size = sensorFormat.size;<br>
> -<br>
> - ret = data->isp_[Isp::Input].dev()->setFormat(&ispFormat);<br>
> - if (ret) {<br>
> - stop(camera);<br>
> - return ret;<br>
> - }<br>
> -<br>
> /* Enable SOF event generation. */<br>
> data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true);<br>
> <br>
> @@ -1332,10 +1323,6 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)<br>
> <br>
> int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
> {<br>
> - /* We know config must be an RPiCameraConfiguration. */<br>
> - const RPiCameraConfiguration *rpiConfig =<br>
> - static_cast<const RPiCameraConfiguration *>(config);<br>
> -<br>
> std::map<unsigned int, IPAStream> streamConfig;<br>
> std::map<unsigned int, ControlInfoMap> entityControls;<br>
> ipa::RPi::IPAConfig ipaConfig;<br>
> @@ -1386,17 +1373,6 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
> return -EPIPE;<br>
> }<br>
> <br>
> - /*<br>
> - * Configure the H/V flip controls based on the combination of<br>
> - * the sensor and user transform.<br>
> - */<br>
> - if (supportsFlips_) {<br>
> - controls.set(V4L2_CID_HFLIP,<br>
> - static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));<br>
> - controls.set(V4L2_CID_VFLIP,<br>
> - static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));<br>
> - }<br>
> -<br>
> if (!controls.empty())<br>
> setSensorControls(controls);<br>
> <br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>