| Message ID | 20220112092758.4726-1-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for your work. On Wed, 12 Jan 2022 at 09:28, David Plowman <david.plowman@raspberrypi.com> wrote: > This bug crept in when the pipeline handler was converted to use media > controller. > > Previously the sensor's hflip and vflip were being cleared before > querying the sensor for its "native" Bayer order. Now, though, the > sensor's available formats are cached before we can clear these bits. > > Instead, we deduce the transform equivalent to the current hflip and > vflip settings, and apply its inverse to the Bayer formats that we now > have, thereby giving us the untransformed Bayer order that we want. > > The practical consequence of this was that the Bayer order stored in > DNG files was frequently wrong. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline handler > to use media controller") > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 49d7ff23..c1fb9666 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1279,20 +1279,24 @@ int PipelineHandlerRPi::registerCamera(MediaDevice > *unicam, MediaDevice *isp, Me > * Thirdly, what is the "native" Bayer order, when no transforms > are > * applied? > * > - * As part of answering the final question, we reset the camera to > - * no transform at all. > + * We note that the format list has already been populated with > + * whatever flips are currently set, so to answer the final > question > + * we get the current Bayer order and undo the transform implied by > + * the current flip settings. > */ > const V4L2Subdevice *sensor = data->sensor_->device(); > const struct v4l2_query_ext_ctrl *hflipCtrl = > sensor->controlInfo(V4L2_CID_HFLIP); > + Transform currentTransform = Transform::Identity; > if (hflipCtrl) { > /* We assume it will support vflips too... */ > data->supportsFlips_ = true; > data->flipsAlterBayerOrder_ = hflipCtrl->flags & > V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > - ControlList ctrls(data->sensor_->controls()); > - ctrls.set(V4L2_CID_HFLIP, 0); > - ctrls.set(V4L2_CID_VFLIP, 0); > - data->setSensorControls(ctrls); > + ControlList ctrls = data->sensor_->getControls({ > V4L2_CID_HFLIP, V4L2_CID_VFLIP }); > + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) > + currentTransform ^= Transform::HFlip; > + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) > + currentTransform ^= Transform::VFlip; > } > > /* Look for a valid Bayer format. */ > @@ -1307,7 +1311,10 @@ int PipelineHandlerRPi::registerCamera(MediaDevice > *unicam, MediaDevice *isp, Me > LOG(RPI, Error) << "No Bayer format found"; > return -EINVAL; > } > - data->nativeBayerOrder_ = bayerFormat.order; > + /* Applying the inverse transform will give us the native order. */ > + BayerFormat nativeBayerFormat = > bayerFormat.transform(-currentTransform); > + data->nativeBayerOrder_ = nativeBayerFormat.order; > + LOG(RPI, Debug) << "Native Bayer format is " << > nativeBayerFormat.toString(); > > /* > * List the available streams an application may request. At > present, we > I think I understand the logic of the changes and it all looks good. However, I wonder if the following (entirely untested) change would also give us the correct Bayer order accounting for flips after configure()? diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 5ee713fe66a6..02b31d787b55 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) if (isRaw(cfg.pixelFormat)) { cfg.setStream(&data->unicam_[Unicam::Image]); data->unicam_[Unicam::Image].setExternal(true); + cfg.pixelFormat = unicamFormat.fourcc.toPixelFormat(); continue; } > -- > 2.30.2 > >
Hi Naush Thanks for the reply. On Wed, 12 Jan 2022 at 10:07, Naushir Patuck <naush@raspberrypi.com> wrote: > > Hi David, > > Thank you for your work. > > On Wed, 12 Jan 2022 at 09:28, David Plowman <david.plowman@raspberrypi.com> wrote: >> >> This bug crept in when the pipeline handler was converted to use media >> controller. >> >> Previously the sensor's hflip and vflip were being cleared before >> querying the sensor for its "native" Bayer order. Now, though, the >> sensor's available formats are cached before we can clear these bits. >> >> Instead, we deduce the transform equivalent to the current hflip and >> vflip settings, and apply its inverse to the Bayer formats that we now >> have, thereby giving us the untransformed Bayer order that we want. >> >> The practical consequence of this was that the Bayer order stored in >> DNG files was frequently wrong. >> >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com> >> Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline handler to use media controller") >> --- >> .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++------- >> 1 file changed, 14 insertions(+), 7 deletions(-) >> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> index 49d7ff23..c1fb9666 100644 >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> @@ -1279,20 +1279,24 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me >> * Thirdly, what is the "native" Bayer order, when no transforms are >> * applied? >> * >> - * As part of answering the final question, we reset the camera to >> - * no transform at all. >> + * We note that the format list has already been populated with >> + * whatever flips are currently set, so to answer the final question >> + * we get the current Bayer order and undo the transform implied by >> + * the current flip settings. >> */ >> const V4L2Subdevice *sensor = data->sensor_->device(); >> const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP); >> + Transform currentTransform = Transform::Identity; >> if (hflipCtrl) { >> /* We assume it will support vflips too... */ >> data->supportsFlips_ = true; >> data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT; >> >> - ControlList ctrls(data->sensor_->controls()); >> - ctrls.set(V4L2_CID_HFLIP, 0); >> - ctrls.set(V4L2_CID_VFLIP, 0); >> - data->setSensorControls(ctrls); >> + ControlList ctrls = data->sensor_->getControls({ V4L2_CID_HFLIP, V4L2_CID_VFLIP }); >> + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) >> + currentTransform ^= Transform::HFlip; >> + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) >> + currentTransform ^= Transform::VFlip; >> } >> >> /* Look for a valid Bayer format. */ >> @@ -1307,7 +1311,10 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me >> LOG(RPI, Error) << "No Bayer format found"; >> return -EINVAL; >> } >> - data->nativeBayerOrder_ = bayerFormat.order; >> + /* Applying the inverse transform will give us the native order. */ >> + BayerFormat nativeBayerFormat = bayerFormat.transform(-currentTransform); >> + data->nativeBayerOrder_ = nativeBayerFormat.order; >> + LOG(RPI, Debug) << "Native Bayer format is " << nativeBayerFormat.toString(); >> >> /* >> * List the available streams an application may request. At present, we > > > I think I understand the logic of the changes and it all looks good. However, I wonder if the > following (entirely untested) change would also give us the correct Bayer order accounting for > flips after configure()? > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 5ee713fe66a6..02b31d787b55 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > if (isRaw(cfg.pixelFormat)) { > cfg.setStream(&data->unicam_[Unicam::Image]); > data->unicam_[Unicam::Image].setExternal(true); > + cfg.pixelFormat = unicamFormat.fourcc.toPixelFormat(); > continue; > } That's pretty interesting. I wonder, is there any way to do this earlier, my impression is that we need to set the pixel format correctly in validate(), rather than wait for configure()? David > > >> >> -- >> 2.30.2 >>
Hi David, On Wed, 12 Jan 2022 at 10:20, David Plowman <david.plowman@raspberrypi.com> wrote: > Hi Naush > > Thanks for the reply. > > On Wed, 12 Jan 2022 at 10:07, Naushir Patuck <naush@raspberrypi.com> > wrote: > > > > Hi David, > > > > Thank you for your work. > > > > On Wed, 12 Jan 2022 at 09:28, David Plowman < > david.plowman@raspberrypi.com> wrote: > >> > >> This bug crept in when the pipeline handler was converted to use media > >> controller. > >> > >> Previously the sensor's hflip and vflip were being cleared before > >> querying the sensor for its "native" Bayer order. Now, though, the > >> sensor's available formats are cached before we can clear these bits. > >> > >> Instead, we deduce the transform equivalent to the current hflip and > >> vflip settings, and apply its inverse to the Bayer formats that we now > >> have, thereby giving us the untransformed Bayer order that we want. > >> > >> The practical consequence of this was that the Bayer order stored in > >> DNG files was frequently wrong. > >> > >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > >> Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline > handler to use media controller") > >> --- > >> .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++------- > >> 1 file changed, 14 insertions(+), 7 deletions(-) > >> > >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > >> index 49d7ff23..c1fb9666 100644 > >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > >> @@ -1279,20 +1279,24 @@ int > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > >> * Thirdly, what is the "native" Bayer order, when no > transforms are > >> * applied? > >> * > >> - * As part of answering the final question, we reset the camera > to > >> - * no transform at all. > >> + * We note that the format list has already been populated with > >> + * whatever flips are currently set, so to answer the final > question > >> + * we get the current Bayer order and undo the transform > implied by > >> + * the current flip settings. > >> */ > >> const V4L2Subdevice *sensor = data->sensor_->device(); > >> const struct v4l2_query_ext_ctrl *hflipCtrl = > sensor->controlInfo(V4L2_CID_HFLIP); > >> + Transform currentTransform = Transform::Identity; > >> if (hflipCtrl) { > >> /* We assume it will support vflips too... */ > >> data->supportsFlips_ = true; > >> data->flipsAlterBayerOrder_ = hflipCtrl->flags & > V4L2_CTRL_FLAG_MODIFY_LAYOUT; > >> > >> - ControlList ctrls(data->sensor_->controls()); > >> - ctrls.set(V4L2_CID_HFLIP, 0); > >> - ctrls.set(V4L2_CID_VFLIP, 0); > >> - data->setSensorControls(ctrls); > >> + ControlList ctrls = data->sensor_->getControls({ > V4L2_CID_HFLIP, V4L2_CID_VFLIP }); > >> + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) > >> + currentTransform ^= Transform::HFlip; > >> + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) > >> + currentTransform ^= Transform::VFlip; > >> } > >> > >> /* Look for a valid Bayer format. */ > >> @@ -1307,7 +1311,10 @@ int > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > >> LOG(RPI, Error) << "No Bayer format found"; > >> return -EINVAL; > >> } > >> - data->nativeBayerOrder_ = bayerFormat.order; > >> + /* Applying the inverse transform will give us the native > order. */ > >> + BayerFormat nativeBayerFormat = > bayerFormat.transform(-currentTransform); > >> + data->nativeBayerOrder_ = nativeBayerFormat.order; > >> + LOG(RPI, Debug) << "Native Bayer format is " << > nativeBayerFormat.toString(); > >> > >> /* > >> * List the available streams an application may request. At > present, we > > > > > > I think I understand the logic of the changes and it all looks good. > However, I wonder if the > > following (entirely untested) change would also give us the correct > Bayer order accounting for > > flips after configure()? > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index 5ee713fe66a6..02b31d787b55 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera *camera, > CameraConfiguration *config) > > if (isRaw(cfg.pixelFormat)) { > > cfg.setStream(&data->unicam_[Unicam::Image]); > > data->unicam_[Unicam::Image].setExternal(true); > > + cfg.pixelFormat = > unicamFormat.fourcc.toPixelFormat(); > > continue; > > } > > That's pretty interesting. I wonder, is there any way to do this > earlier, my impression is that we need to set the pixel format > correctly in validate(), rather than wait for configure()? > I don't think that would be possible. We only get to know the user requested flips in the configure(), so validate() would only ever return the un-flipped (user point of view) ordering. Naush > > David > > > > > > >> > >> -- > >> 2.30.2 > >> >
Quoting Naushir Patuck (2022-01-12 10:24:02) > Hi David, > > On Wed, 12 Jan 2022 at 10:20, David Plowman <david.plowman@raspberrypi.com> > wrote: > > > Hi Naush > > > > Thanks for the reply. > > > > On Wed, 12 Jan 2022 at 10:07, Naushir Patuck <naush@raspberrypi.com> > > wrote: > > > > > > Hi David, > > > > > > Thank you for your work. > > > > > > On Wed, 12 Jan 2022 at 09:28, David Plowman < > > david.plowman@raspberrypi.com> wrote: > > >> > > >> This bug crept in when the pipeline handler was converted to use media > > >> controller. > > >> > > >> Previously the sensor's hflip and vflip were being cleared before > > >> querying the sensor for its "native" Bayer order. Now, though, the > > >> sensor's available formats are cached before we can clear these bits. > > >> > > >> Instead, we deduce the transform equivalent to the current hflip and > > >> vflip settings, and apply its inverse to the Bayer formats that we now > > >> have, thereby giving us the untransformed Bayer order that we want. > > >> > > >> The practical consequence of this was that the Bayer order stored in > > >> DNG files was frequently wrong. > > >> > > >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > >> Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline > > handler to use media controller") > > >> --- > > >> .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++------- > > >> 1 file changed, 14 insertions(+), 7 deletions(-) > > >> > > >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > >> index 49d7ff23..c1fb9666 100644 > > >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > >> @@ -1279,20 +1279,24 @@ int > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > > >> * Thirdly, what is the "native" Bayer order, when no > > transforms are > > >> * applied? > > >> * > > >> - * As part of answering the final question, we reset the camera > > to > > >> - * no transform at all. > > >> + * We note that the format list has already been populated with > > >> + * whatever flips are currently set, so to answer the final > > question > > >> + * we get the current Bayer order and undo the transform > > implied by > > >> + * the current flip settings. > > >> */ > > >> const V4L2Subdevice *sensor = data->sensor_->device(); > > >> const struct v4l2_query_ext_ctrl *hflipCtrl = > > sensor->controlInfo(V4L2_CID_HFLIP); > > >> + Transform currentTransform = Transform::Identity; > > >> if (hflipCtrl) { > > >> /* We assume it will support vflips too... */ > > >> data->supportsFlips_ = true; > > >> data->flipsAlterBayerOrder_ = hflipCtrl->flags & > > V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > >> > > >> - ControlList ctrls(data->sensor_->controls()); > > >> - ctrls.set(V4L2_CID_HFLIP, 0); > > >> - ctrls.set(V4L2_CID_VFLIP, 0); > > >> - data->setSensorControls(ctrls); > > >> + ControlList ctrls = data->sensor_->getControls({ > > V4L2_CID_HFLIP, V4L2_CID_VFLIP }); > > >> + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) > > >> + currentTransform ^= Transform::HFlip; > > >> + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) > > >> + currentTransform ^= Transform::VFlip; I'm worried that this might leave us in an inconsistent state at startup. I can see we definitely need to account for the flips when mapping the current bayerFormat.order to a native order, and I think that sounds reasonable, so my only concern is if we need to 'reset' the HFLIP/VFLIP controls to known defaults at startup, either after we've handled this native bayer ordering, or ... perhaps we should do it when we construct the CameraSensor class ... before we read the formats? > > >> } > > >> > > >> /* Look for a valid Bayer format. */ > > >> @@ -1307,7 +1311,10 @@ int > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > > >> LOG(RPI, Error) << "No Bayer format found"; > > >> return -EINVAL; > > >> } > > >> - data->nativeBayerOrder_ = bayerFormat.order; > > >> + /* Applying the inverse transform will give us the native > > order. */ > > >> + BayerFormat nativeBayerFormat = > > bayerFormat.transform(-currentTransform); > > >> + data->nativeBayerOrder_ = nativeBayerFormat.order; > > >> + LOG(RPI, Debug) << "Native Bayer format is " << > > nativeBayerFormat.toString(); > > >> > > >> /* > > >> * List the available streams an application may request. At > > present, we > > > > > > > > > I think I understand the logic of the changes and it all looks good. > > However, I wonder if the > > > following (entirely untested) change would also give us the correct > > Bayer order accounting for > > > flips after configure()? > > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > index 5ee713fe66a6..02b31d787b55 100644 > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > @@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera *camera, > > CameraConfiguration *config) > > > if (isRaw(cfg.pixelFormat)) { > > > cfg.setStream(&data->unicam_[Unicam::Image]); > > > data->unicam_[Unicam::Image].setExternal(true); > > > + cfg.pixelFormat = > > unicamFormat.fourcc.toPixelFormat(); > > > continue; > > > } > > > > That's pretty interesting. I wonder, is there any way to do this > > earlier, my impression is that we need to set the pixel format > > correctly in validate(), rather than wait for configure()? > > > > I don't think that would be possible. We only get to know the user > requested flips in the configure(), so validate() would only ever return > the un-flipped (user point of view) ordering. > Also, if I understand this correctly it's trying to determine the 'constant' native bayer order of the sensor. If it is consistent, and doesn't change with any configuration, then I would expect this to be done when the camera is created/registered as this patch does. Validate should be able to identify everything that would be affected by the configuration when configure is called with that configuration. -- Kieran > Naush > > > > > > David > > > > > > > > > > >> > > >> -- > > >> 2.30.2 > > >> > >
Hi Kieran, Naush On Thu, 13 Jan 2022 at 09:48, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Naushir Patuck (2022-01-12 10:24:02) > > Hi David, > > > > On Wed, 12 Jan 2022 at 10:20, David Plowman <david.plowman@raspberrypi.com> > > wrote: > > > > > Hi Naush > > > > > > Thanks for the reply. > > > > > > On Wed, 12 Jan 2022 at 10:07, Naushir Patuck <naush@raspberrypi.com> > > > wrote: > > > > > > > > Hi David, > > > > > > > > Thank you for your work. > > > > > > > > On Wed, 12 Jan 2022 at 09:28, David Plowman < > > > david.plowman@raspberrypi.com> wrote: > > > >> > > > >> This bug crept in when the pipeline handler was converted to use media > > > >> controller. > > > >> > > > >> Previously the sensor's hflip and vflip were being cleared before > > > >> querying the sensor for its "native" Bayer order. Now, though, the > > > >> sensor's available formats are cached before we can clear these bits. > > > >> > > > >> Instead, we deduce the transform equivalent to the current hflip and > > > >> vflip settings, and apply its inverse to the Bayer formats that we now > > > >> have, thereby giving us the untransformed Bayer order that we want. > > > >> > > > >> The practical consequence of this was that the Bayer order stored in > > > >> DNG files was frequently wrong. > > > >> > > > >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > >> Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline > > > handler to use media controller") > > > >> --- > > > >> .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++------- > > > >> 1 file changed, 14 insertions(+), 7 deletions(-) > > > >> > > > >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > >> index 49d7ff23..c1fb9666 100644 > > > >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > >> @@ -1279,20 +1279,24 @@ int > > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > > > >> * Thirdly, what is the "native" Bayer order, when no > > > transforms are > > > >> * applied? > > > >> * > > > >> - * As part of answering the final question, we reset the camera > > > to > > > >> - * no transform at all. > > > >> + * We note that the format list has already been populated with > > > >> + * whatever flips are currently set, so to answer the final > > > question > > > >> + * we get the current Bayer order and undo the transform > > > implied by > > > >> + * the current flip settings. > > > >> */ > > > >> const V4L2Subdevice *sensor = data->sensor_->device(); > > > >> const struct v4l2_query_ext_ctrl *hflipCtrl = > > > sensor->controlInfo(V4L2_CID_HFLIP); > > > >> + Transform currentTransform = Transform::Identity; > > > >> if (hflipCtrl) { > > > >> /* We assume it will support vflips too... */ > > > >> data->supportsFlips_ = true; > > > >> data->flipsAlterBayerOrder_ = hflipCtrl->flags & > > > V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > > >> > > > >> - ControlList ctrls(data->sensor_->controls()); > > > >> - ctrls.set(V4L2_CID_HFLIP, 0); > > > >> - ctrls.set(V4L2_CID_VFLIP, 0); > > > >> - data->setSensorControls(ctrls); > > > >> + ControlList ctrls = data->sensor_->getControls({ > > > V4L2_CID_HFLIP, V4L2_CID_VFLIP }); > > > >> + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) > > > >> + currentTransform ^= Transform::HFlip; > > > >> + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) > > > >> + currentTransform ^= Transform::VFlip; > > I'm worried that this might leave us in an inconsistent state at > startup. > > I can see we definitely need to account for the flips when mapping the > current bayerFormat.order to a native order, and I think that sounds > reasonable, so my only concern is if we need to 'reset' the HFLIP/VFLIP > controls to known defaults at startup, either after we've handled this > native bayer ordering, or ... perhaps we should do it when we construct > the CameraSensor class ... before we read the formats? I don't think anything should end up inconsistent - whatever the current h/vflip settings it should (and in my testing does) come out with the correct native Bayer order every time. But I agree, maybe there's a place where we can clear the flips before we cache those formats? That would definitely make things easier here. I assume the Bayer order of the cached formats can't matter - as the camera may get flipped later anyway and everything carries on working. > > > > > > >> } > > > >> > > > >> /* Look for a valid Bayer format. */ > > > >> @@ -1307,7 +1311,10 @@ int > > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > > > >> LOG(RPI, Error) << "No Bayer format found"; > > > >> return -EINVAL; > > > >> } > > > >> - data->nativeBayerOrder_ = bayerFormat.order; > > > >> + /* Applying the inverse transform will give us the native > > > order. */ > > > >> + BayerFormat nativeBayerFormat = > > > bayerFormat.transform(-currentTransform); > > > >> + data->nativeBayerOrder_ = nativeBayerFormat.order; > > > >> + LOG(RPI, Debug) << "Native Bayer format is " << > > > nativeBayerFormat.toString(); > > > >> > > > >> /* > > > >> * List the available streams an application may request. At > > > present, we > > > > > > > > > > > > I think I understand the logic of the changes and it all looks good. > > > However, I wonder if the > > > > following (entirely untested) change would also give us the correct > > > Bayer order accounting for > > > > flips after configure()? > > > > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > index 5ee713fe66a6..02b31d787b55 100644 > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > @@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera *camera, > > > CameraConfiguration *config) > > > > if (isRaw(cfg.pixelFormat)) { > > > > cfg.setStream(&data->unicam_[Unicam::Image]); > > > > data->unicam_[Unicam::Image].setExternal(true); > > > > + cfg.pixelFormat = > > > unicamFormat.fourcc.toPixelFormat(); > > > > continue; > > > > } > > > > > > That's pretty interesting. I wonder, is there any way to do this > > > earlier, my impression is that we need to set the pixel format > > > correctly in validate(), rather than wait for configure()? > > > > > > > I don't think that would be possible. We only get to know the user > > requested flips in the configure(), so validate() would only ever return > > the un-flipped (user point of view) ordering. > > > > Also, if I understand this correctly it's trying to determine the 'constant' > native bayer order of the sensor. If it is consistent, and doesn't > change with any configuration, then I would expect this to be done when > the camera is created/registered as this patch does. > > Validate should be able to identify everything that would be affected by > the configuration when configure is called with that configuration. Yes. If we can clear the flips and do everything in registerCamera that would seem ideal... David > > -- > Kieran > > > > Naush > > > > > > > > > > David > > > > > > > > > > > > > > >> > > > >> -- > > > >> 2.30.2 > > > >> > > >
Hi again On Thu, 13 Jan 2022 at 10:12, David Plowman <david.plowman@raspberrypi.com> wrote: > > Hi Kieran, Naush > > On Thu, 13 Jan 2022 at 09:48, Kieran Bingham > <kieran.bingham@ideasonboard.com> wrote: > > > > Quoting Naushir Patuck (2022-01-12 10:24:02) > > > Hi David, > > > > > > On Wed, 12 Jan 2022 at 10:20, David Plowman <david.plowman@raspberrypi.com> > > > wrote: > > > > > > > Hi Naush > > > > > > > > Thanks for the reply. > > > > > > > > On Wed, 12 Jan 2022 at 10:07, Naushir Patuck <naush@raspberrypi.com> > > > > wrote: > > > > > > > > > > Hi David, > > > > > > > > > > Thank you for your work. > > > > > > > > > > On Wed, 12 Jan 2022 at 09:28, David Plowman < > > > > david.plowman@raspberrypi.com> wrote: > > > > >> > > > > >> This bug crept in when the pipeline handler was converted to use media > > > > >> controller. > > > > >> > > > > >> Previously the sensor's hflip and vflip were being cleared before > > > > >> querying the sensor for its "native" Bayer order. Now, though, the > > > > >> sensor's available formats are cached before we can clear these bits. > > > > >> > > > > >> Instead, we deduce the transform equivalent to the current hflip and > > > > >> vflip settings, and apply its inverse to the Bayer formats that we now > > > > >> have, thereby giving us the untransformed Bayer order that we want. > > > > >> > > > > >> The practical consequence of this was that the Bayer order stored in > > > > >> DNG files was frequently wrong. > > > > >> > > > > >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > >> Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline > > > > handler to use media controller") > > > > >> --- > > > > >> .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++------- > > > > >> 1 file changed, 14 insertions(+), 7 deletions(-) > > > > >> > > > > >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > >> index 49d7ff23..c1fb9666 100644 > > > > >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > >> @@ -1279,20 +1279,24 @@ int > > > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > > > > >> * Thirdly, what is the "native" Bayer order, when no > > > > transforms are > > > > >> * applied? > > > > >> * > > > > >> - * As part of answering the final question, we reset the camera > > > > to > > > > >> - * no transform at all. > > > > >> + * We note that the format list has already been populated with > > > > >> + * whatever flips are currently set, so to answer the final > > > > question > > > > >> + * we get the current Bayer order and undo the transform > > > > implied by > > > > >> + * the current flip settings. > > > > >> */ > > > > >> const V4L2Subdevice *sensor = data->sensor_->device(); > > > > >> const struct v4l2_query_ext_ctrl *hflipCtrl = > > > > sensor->controlInfo(V4L2_CID_HFLIP); > > > > >> + Transform currentTransform = Transform::Identity; > > > > >> if (hflipCtrl) { > > > > >> /* We assume it will support vflips too... */ > > > > >> data->supportsFlips_ = true; > > > > >> data->flipsAlterBayerOrder_ = hflipCtrl->flags & > > > > V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > > > >> > > > > >> - ControlList ctrls(data->sensor_->controls()); > > > > >> - ctrls.set(V4L2_CID_HFLIP, 0); > > > > >> - ctrls.set(V4L2_CID_VFLIP, 0); > > > > >> - data->setSensorControls(ctrls); > > > > >> + ControlList ctrls = data->sensor_->getControls({ > > > > V4L2_CID_HFLIP, V4L2_CID_VFLIP }); > > > > >> + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) > > > > >> + currentTransform ^= Transform::HFlip; > > > > >> + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) > > > > >> + currentTransform ^= Transform::VFlip; > > > > I'm worried that this might leave us in an inconsistent state at > > startup. > > > > I can see we definitely need to account for the flips when mapping the > > current bayerFormat.order to a native order, and I think that sounds > > reasonable, so my only concern is if we need to 'reset' the HFLIP/VFLIP > > controls to known defaults at startup, either after we've handled this > > native bayer ordering, or ... perhaps we should do it when we construct > > the CameraSensor class ... before we read the formats? > > I don't think anything should end up inconsistent - whatever the > current h/vflip settings it should (and in my testing does) come out > with the correct native Bayer order every time. But I agree, maybe > there's a place where we can clear the flips before we cache those > formats? That would definitely make things easier here. > > I assume the Bayer order of the cached formats can't matter - as the > camera may get flipped later anyway and everything carries on working. It looks like it fills that list of formats in CameraSensor::init(). I wonder if that should clear the flip bits first (where they exist)? Would that be acceptable? David > > > > > > > > > > > >> } > > > > >> > > > > >> /* Look for a valid Bayer format. */ > > > > >> @@ -1307,7 +1311,10 @@ int > > > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me > > > > >> LOG(RPI, Error) << "No Bayer format found"; > > > > >> return -EINVAL; > > > > >> } > > > > >> - data->nativeBayerOrder_ = bayerFormat.order; > > > > >> + /* Applying the inverse transform will give us the native > > > > order. */ > > > > >> + BayerFormat nativeBayerFormat = > > > > bayerFormat.transform(-currentTransform); > > > > >> + data->nativeBayerOrder_ = nativeBayerFormat.order; > > > > >> + LOG(RPI, Debug) << "Native Bayer format is " << > > > > nativeBayerFormat.toString(); > > > > >> > > > > >> /* > > > > >> * List the available streams an application may request. At > > > > present, we > > > > > > > > > > > > > > > I think I understand the logic of the changes and it all looks good. > > > > However, I wonder if the > > > > > following (entirely untested) change would also give us the correct > > > > Bayer order accounting for > > > > > flips after configure()? > > > > > > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > index 5ee713fe66a6..02b31d787b55 100644 > > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > @@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera *camera, > > > > CameraConfiguration *config) > > > > > if (isRaw(cfg.pixelFormat)) { > > > > > cfg.setStream(&data->unicam_[Unicam::Image]); > > > > > data->unicam_[Unicam::Image].setExternal(true); > > > > > + cfg.pixelFormat = > > > > unicamFormat.fourcc.toPixelFormat(); > > > > > continue; > > > > > } > > > > > > > > That's pretty interesting. I wonder, is there any way to do this > > > > earlier, my impression is that we need to set the pixel format > > > > correctly in validate(), rather than wait for configure()? > > > > > > > > > > I don't think that would be possible. We only get to know the user > > > requested flips in the configure(), so validate() would only ever return > > > the un-flipped (user point of view) ordering. > > > > > > > Also, if I understand this correctly it's trying to determine the 'constant' > > native bayer order of the sensor. If it is consistent, and doesn't > > change with any configuration, then I would expect this to be done when > > the camera is created/registered as this patch does. > > > > Validate should be able to identify everything that would be affected by > > the configuration when configure is called with that configuration. > > Yes. If we can clear the flips and do everything in registerCamera > that would seem ideal... > > David > > > > > -- > > Kieran > > > > > > > Naush > > > > > > > > > > > > > > David > > > > > > > > > > > > > > > > > > >> > > > > >> -- > > > > >> 2.30.2 > > > > >> > > > >
On Thu, 13 Jan 2022 at 10:30, David Plowman <david.plowman@raspberrypi.com> wrote: > Hi again > > On Thu, 13 Jan 2022 at 10:12, David Plowman > <david.plowman@raspberrypi.com> wrote: > > > > Hi Kieran, Naush > > > > On Thu, 13 Jan 2022 at 09:48, Kieran Bingham > > <kieran.bingham@ideasonboard.com> wrote: > > > > > > Quoting Naushir Patuck (2022-01-12 10:24:02) > > > > Hi David, > > > > > > > > On Wed, 12 Jan 2022 at 10:20, David Plowman < > david.plowman@raspberrypi.com> > > > > wrote: > > > > > > > > > Hi Naush > > > > > > > > > > Thanks for the reply. > > > > > > > > > > On Wed, 12 Jan 2022 at 10:07, Naushir Patuck < > naush@raspberrypi.com> > > > > > wrote: > > > > > > > > > > > > Hi David, > > > > > > > > > > > > Thank you for your work. > > > > > > > > > > > > On Wed, 12 Jan 2022 at 09:28, David Plowman < > > > > > david.plowman@raspberrypi.com> wrote: > > > > > >> > > > > > >> This bug crept in when the pipeline handler was converted to > use media > > > > > >> controller. > > > > > >> > > > > > >> Previously the sensor's hflip and vflip were being cleared > before > > > > > >> querying the sensor for its "native" Bayer order. Now, though, > the > > > > > >> sensor's available formats are cached before we can clear these > bits. > > > > > >> > > > > > >> Instead, we deduce the transform equivalent to the current > hflip and > > > > > >> vflip settings, and apply its inverse to the Bayer formats that > we now > > > > > >> have, thereby giving us the untransformed Bayer order that we > want. > > > > > >> > > > > > >> The practical consequence of this was that the Bayer order > stored in > > > > > >> DNG files was frequently wrong. > > > > > >> > > > > > >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > > >> Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the > pipeline > > > > > handler to use media controller") > > > > > >> --- > > > > > >> .../pipeline/raspberrypi/raspberrypi.cpp | 21 > ++++++++++++------- > > > > > >> 1 file changed, 14 insertions(+), 7 deletions(-) > > > > > >> > > > > > >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > >> index 49d7ff23..c1fb9666 100644 > > > > > >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > >> @@ -1279,20 +1279,24 @@ int > > > > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, > MediaDevice *isp, Me > > > > > >> * Thirdly, what is the "native" Bayer order, when no > > > > > transforms are > > > > > >> * applied? > > > > > >> * > > > > > >> - * As part of answering the final question, we reset > the camera > > > > > to > > > > > >> - * no transform at all. > > > > > >> + * We note that the format list has already been > populated with > > > > > >> + * whatever flips are currently set, so to answer the > final > > > > > question > > > > > >> + * we get the current Bayer order and undo the transform > > > > > implied by > > > > > >> + * the current flip settings. > > > > > >> */ > > > > > >> const V4L2Subdevice *sensor = data->sensor_->device(); > > > > > >> const struct v4l2_query_ext_ctrl *hflipCtrl = > > > > > sensor->controlInfo(V4L2_CID_HFLIP); > > > > > >> + Transform currentTransform = Transform::Identity; > > > > > >> if (hflipCtrl) { > > > > > >> /* We assume it will support vflips too... */ > > > > > >> data->supportsFlips_ = true; > > > > > >> data->flipsAlterBayerOrder_ = hflipCtrl->flags & > > > > > V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > > > > >> > > > > > >> - ControlList ctrls(data->sensor_->controls()); > > > > > >> - ctrls.set(V4L2_CID_HFLIP, 0); > > > > > >> - ctrls.set(V4L2_CID_VFLIP, 0); > > > > > >> - data->setSensorControls(ctrls); > > > > > >> + ControlList ctrls = data->sensor_->getControls({ > > > > > V4L2_CID_HFLIP, V4L2_CID_VFLIP }); > > > > > >> + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) > > > > > >> + currentTransform ^= Transform::HFlip; > > > > > >> + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) > > > > > >> + currentTransform ^= Transform::VFlip; > > > > > > I'm worried that this might leave us in an inconsistent state at > > > startup. > > > > > > I can see we definitely need to account for the flips when mapping the > > > current bayerFormat.order to a native order, and I think that sounds > > > reasonable, so my only concern is if we need to 'reset' the HFLIP/VFLIP > > > controls to known defaults at startup, either after we've handled this > > > native bayer ordering, or ... perhaps we should do it when we construct > > > the CameraSensor class ... before we read the formats? > > > > I don't think anything should end up inconsistent - whatever the > > current h/vflip settings it should (and in my testing does) come out > > with the correct native Bayer order every time. But I agree, maybe > > there's a place where we can clear the flips before we cache those > > formats? That would definitely make things easier here. > > > > I assume the Bayer order of the cached formats can't matter - as the > > camera may get flipped later anyway and everything carries on working. > > It looks like it fills that list of formats in CameraSensor::init(). I > wonder if that should clear the flip bits first (where they exist)? > Would that be acceptable? > I would be happy with that behaviour. Naush > > David > > > > > > > > > > > > > > > > > >> } > > > > > >> > > > > > >> /* Look for a valid Bayer format. */ > > > > > >> @@ -1307,7 +1311,10 @@ int > > > > > PipelineHandlerRPi::registerCamera(MediaDevice *unicam, > MediaDevice *isp, Me > > > > > >> LOG(RPI, Error) << "No Bayer format found"; > > > > > >> return -EINVAL; > > > > > >> } > > > > > >> - data->nativeBayerOrder_ = bayerFormat.order; > > > > > >> + /* Applying the inverse transform will give us the > native > > > > > order. */ > > > > > >> + BayerFormat nativeBayerFormat = > > > > > bayerFormat.transform(-currentTransform); > > > > > >> + data->nativeBayerOrder_ = nativeBayerFormat.order; > > > > > >> + LOG(RPI, Debug) << "Native Bayer format is " << > > > > > nativeBayerFormat.toString(); > > > > > >> > > > > > >> /* > > > > > >> * List the available streams an application may > request. At > > > > > present, we > > > > > > > > > > > > > > > > > > I think I understand the logic of the changes and it all looks > good. > > > > > However, I wonder if the > > > > > > following (entirely untested) change would also give us the > correct > > > > > Bayer order accounting for > > > > > > flips after configure()? > > > > > > > > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > > index 5ee713fe66a6..02b31d787b55 100644 > > > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > > > @@ -761,6 +761,7 @@ int PipelineHandlerRPi::configure(Camera > *camera, > > > > > CameraConfiguration *config) > > > > > > if (isRaw(cfg.pixelFormat)) { > > > > > > > cfg.setStream(&data->unicam_[Unicam::Image]); > > > > > > > data->unicam_[Unicam::Image].setExternal(true); > > > > > > + cfg.pixelFormat = > > > > > unicamFormat.fourcc.toPixelFormat(); > > > > > > continue; > > > > > > } > > > > > > > > > > That's pretty interesting. I wonder, is there any way to do this > > > > > earlier, my impression is that we need to set the pixel format > > > > > correctly in validate(), rather than wait for configure()? > > > > > > > > > > > > > I don't think that would be possible. We only get to know the user > > > > requested flips in the configure(), so validate() would only ever > return > > > > the un-flipped (user point of view) ordering. > > > > > > > > > > Also, if I understand this correctly it's trying to determine the > 'constant' > > > native bayer order of the sensor. If it is consistent, and doesn't > > > change with any configuration, then I would expect this to be done when > > > the camera is created/registered as this patch does. > > > > > > Validate should be able to identify everything that would be affected > by > > > the configuration when configure is called with that configuration. > > > > Yes. If we can clear the flips and do everything in registerCamera > > that would seem ideal... > > > > David > > > > > > > > -- > > > Kieran > > > > > > > > > > Naush > > > > > > > > > > > > > > > > > > David > > > > > > > > > > > > > > > > > > > > > > >> > > > > > >> -- > > > > > >> 2.30.2 > > > > > >> > > > > > >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 49d7ff23..c1fb9666 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1279,20 +1279,24 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me * Thirdly, what is the "native" Bayer order, when no transforms are * applied? * - * As part of answering the final question, we reset the camera to - * no transform at all. + * We note that the format list has already been populated with + * whatever flips are currently set, so to answer the final question + * we get the current Bayer order and undo the transform implied by + * the current flip settings. */ const V4L2Subdevice *sensor = data->sensor_->device(); const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP); + Transform currentTransform = Transform::Identity; if (hflipCtrl) { /* We assume it will support vflips too... */ data->supportsFlips_ = true; data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT; - ControlList ctrls(data->sensor_->controls()); - ctrls.set(V4L2_CID_HFLIP, 0); - ctrls.set(V4L2_CID_VFLIP, 0); - data->setSensorControls(ctrls); + ControlList ctrls = data->sensor_->getControls({ V4L2_CID_HFLIP, V4L2_CID_VFLIP }); + if (ctrls.get(V4L2_CID_HFLIP).get<int32_t>()) + currentTransform ^= Transform::HFlip; + if (ctrls.get(V4L2_CID_VFLIP).get<int32_t>()) + currentTransform ^= Transform::VFlip; } /* Look for a valid Bayer format. */ @@ -1307,7 +1311,10 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me LOG(RPI, Error) << "No Bayer format found"; return -EINVAL; } - data->nativeBayerOrder_ = bayerFormat.order; + /* Applying the inverse transform will give us the native order. */ + BayerFormat nativeBayerFormat = bayerFormat.transform(-currentTransform); + data->nativeBayerOrder_ = nativeBayerFormat.order; + LOG(RPI, Debug) << "Native Bayer format is " << nativeBayerFormat.toString(); /* * List the available streams an application may request. At present, we
This bug crept in when the pipeline handler was converted to use media controller. Previously the sensor's hflip and vflip were being cleared before querying the sensor for its "native" Bayer order. Now, though, the sensor's available formats are cached before we can clear these bits. Instead, we deduce the transform equivalent to the current hflip and vflip settings, and apply its inverse to the Bayer formats that we now have, thereby giving us the untransformed Bayer order that we want. The practical consequence of this was that the Bayer order stored in DNG files was frequently wrong. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline handler to use media controller") --- .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)