<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your work.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 12 Jan 2022 at 09:28, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.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">This bug crept in when the pipeline handler was converted to use media<br>
controller.<br>
<br>
Previously the sensor's hflip and vflip were being cleared before<br>
querying the sensor for its "native" Bayer order. Now, though, the<br>
sensor's available formats are cached before we can clear these bits.<br>
<br>
Instead, we deduce the transform equivalent to the current hflip and<br>
vflip settings, and apply its inverse to the Bayer formats that we now<br>
have, thereby giving us the untransformed Bayer order that we want.<br>
<br>
The practical consequence of this was that the Bayer order stored in<br>
DNG files was frequently wrong.<br>
<br>
Signed-off-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline handler to use media controller")<br>
---<br>
.../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++-------<br>
1 file changed, 14 insertions(+), 7 deletions(-)<br>
<br>
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
index 49d7ff23..c1fb9666 100644<br>
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
@@ -1279,20 +1279,24 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me<br>
* Thirdly, what is the "native" Bayer order, when no transforms are<br>
* applied?<br>
*<br>
- * As part of answering the final question, we reset the camera to<br>
- * no transform at all.<br>
+ * We note that the format list has already been populated with<br>
+ * whatever flips are currently set, so to answer the final question<br>
+ * we get the current Bayer order and undo the transform implied by<br>
+ * the current flip settings.<br>
*/<br>
const V4L2Subdevice *sensor = data->sensor_->device();<br>
const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP);<br>
+ Transform currentTransform = Transform::Identity;<br>
if (hflipCtrl) {<br>
/* We assume it will support vflips too... */<br>
data->supportsFlips_ = true;<br>
data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;<br>
<br>
- ControlList ctrls(data->sensor_->controls());<br>
- ctrls.set(V4L2_CID_HFLIP, 0);<br>
- ctrls.set(V4L2_CID_VFLIP, 0);<br>
- data->setSensorControls(ctrls);<br>
+ ControlList ctrls = data->sensor_->getControls({ V4L2_CID_HFLIP, V4L2_CID_VFLIP });<br>
+ if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>())<br>
+ currentTransform ^= Transform::HFlip;<br>
+ if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>())<br>
+ currentTransform ^= Transform::VFlip;<br>
}<br>
<br>
/* Look for a valid Bayer format. */<br>
@@ -1307,7 +1311,10 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me<br>
LOG(RPI, Error) << "No Bayer format found";<br>
return -EINVAL;<br>
}<br>
- data->nativeBayerOrder_ = bayerFormat.order;<br>
+ /* Applying the inverse transform will give us the native order. */<br>
+ BayerFormat nativeBayerFormat = bayerFormat.transform(-currentTransform);<br>
+ data->nativeBayerOrder_ = nativeBayerFormat.order;<br>
+ LOG(RPI, Debug) << "Native Bayer format is " << nativeBayerFormat.toString();<br>
<br>
/*<br>
* List the available streams an application may request. At present, we<br></blockquote><div><br></div><div>I think I understand the logic of the changes and it all looks good. However, I wonder if the</div><div>following (entirely untested) change would also give us the correct Bayer order accounting for</div><div>flips after configure()?</div><div><br></div><div>diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>index 5ee713fe66a6..02b31d787b55 100644<br>--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>@@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br> if (isRaw(cfg.pixelFormat)) {<br> cfg.setStream(&data->unicam_[Unicam::Image]);<br> data->unicam_[Unicam::Image].setExternal(true);<br>+ cfg.pixelFormat = unicamFormat.fourcc.toPixelFormat();<br> continue;<br> }<br></div><div><br></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>
2.30.2<br>
<br>
</blockquote></div></div>