| Message ID | 20250709134229.135949-2-uajain@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, thank you for the patch. Umang Jain <uajain@igalia.com> writes: > Bayer pattern on the sensor can change while configuring it with the > intended capture format. This is due to the transform being applied on > the sensor which supports [v/h]flips. > > During configure(), the simple pipeline handler does not detect any > bayer pattern changes that can arise due to the transformations being > applied via SimpleCameraData:setupFormats(). In such cases, the video > node will be configured in-correctly, without realising the bayer > pattern has changed on the sensor, for the given capture format. > > This patch detects the bayer pattern change after the sensor has > been configured and retrieves the corresponding V4L2 pixel format > to correctly configure the video node. The patches make software ISP working on my RPi 4B but red and blue are swapped (tested with `cam -D'). > Signed-off-by: Umang Jain <uajain@igalia.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index efb07051..e2a48724 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1353,8 +1353,20 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > if (ret < 0) > return ret; > > - /* Configure the video node. */ > - V4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); > + /* Configure the video node, taking into account any Bayer pattern change. */ > + V4L2PixelFormat videoFormat; > + if (format.code == pipeConfig->code) { > + videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); > + } else { > + /* > + * Bayer pattern has changed because of the transform that was applied on > + * the sensor. Get the V4L2PixelFormat corresponding to the configured Bayer > + * pattern. > + */ > + BayerFormat cfgBayer = BayerFormat::fromPixelFormat(pipeConfig->captureFormat); > + cfgBayer.order = data->sensor_->bayerOrder(config->combinedTransform()); > + videoFormat = cfgBayer.toV4L2PixelFormat(); > + } > > V4L2DeviceFormat captureFormat; > captureFormat.fourcc = videoFormat;
Hi Milan On Wed, Jul 09, 2025 at 04:51:05PM +0200, Milan Zamazal wrote: > Hi Umang, > > thank you for the patch. > > Umang Jain <uajain@igalia.com> writes: > > > Bayer pattern on the sensor can change while configuring it with the > > intended capture format. This is due to the transform being applied on > > the sensor which supports [v/h]flips. > > > > During configure(), the simple pipeline handler does not detect any > > bayer pattern changes that can arise due to the transformations being > > applied via SimpleCameraData:setupFormats(). In such cases, the video > > node will be configured in-correctly, without realising the bayer > > pattern has changed on the sensor, for the given capture format. > > > > This patch detects the bayer pattern change after the sensor has > > been configured and retrieves the corresponding V4L2 pixel format > > to correctly configure the video node. > > The patches make software ISP working on my RPi 4B but red and blue > are swapped (tested with `cam -D'). Using IMX219 right? I think that is happening due to the fact this patch did not update the cached pipeConfig_, after the flips are applied. So swISP_ is getting fed a (outdated)pipeConfig where no flips are applied. .... ah, but for that to happen, we need to make the cached pipeConfig_ mutable eek! > > > Signed-off-by: Umang Jain <uajain@igalia.com> > > --- > > src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++++++++-- > > 1 file changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index efb07051..e2a48724 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -1353,8 +1353,20 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > > if (ret < 0) > > return ret; > > > > - /* Configure the video node. */ > > - V4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); > > + /* Configure the video node, taking into account any Bayer pattern change. */ > > + V4L2PixelFormat videoFormat; > > + if (format.code == pipeConfig->code) { > > + videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); > > + } else { > > + /* > > + * Bayer pattern has changed because of the transform that was applied on > > + * the sensor. Get the V4L2PixelFormat corresponding to the configured Bayer > > + * pattern. > > + */ > > + BayerFormat cfgBayer = BayerFormat::fromPixelFormat(pipeConfig->captureFormat); > > + cfgBayer.order = data->sensor_->bayerOrder(config->combinedTransform()); > > + videoFormat = cfgBayer.toV4L2PixelFormat(); > > + } > > > > V4L2DeviceFormat captureFormat; > > captureFormat.fourcc = videoFormat; >
Umang Jain <uajain@igalia.com> writes: > Hi Milan > > On Wed, Jul 09, 2025 at 04:51:05PM +0200, Milan Zamazal wrote: >> Hi Umang, >> >> thank you for the patch. >> >> Umang Jain <uajain@igalia.com> writes: >> >> > Bayer pattern on the sensor can change while configuring it with the >> > intended capture format. This is due to the transform being applied on >> > the sensor which supports [v/h]flips. >> > >> > During configure(), the simple pipeline handler does not detect any >> > bayer pattern changes that can arise due to the transformations being >> > applied via SimpleCameraData:setupFormats(). In such cases, the video >> > node will be configured in-correctly, without realising the bayer >> > pattern has changed on the sensor, for the given capture format. >> > >> > This patch detects the bayer pattern change after the sensor has >> > been configured and retrieves the corresponding V4L2 pixel format >> > to correctly configure the video node. >> >> The patches make software ISP working on my RPi 4B but red and blue >> are swapped (tested with `cam -D'). > > Using IMX219 right? Right. > I think that is happening due to the fact this patch did not update the > cached pipeConfig_, after the flips are applied. So swISP_ is getting > fed a (outdated)pipeConfig where no flips are applied. > > .... > ah, but for that to happen, we need to make the cached pipeConfig_ mutable eek! > >> >> > Signed-off-by: Umang Jain <uajain@igalia.com> >> > --- >> > src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++++++++-- >> > 1 file changed, 14 insertions(+), 2 deletions(-) >> > >> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> > index efb07051..e2a48724 100644 >> > --- a/src/libcamera/pipeline/simple/simple.cpp >> > +++ b/src/libcamera/pipeline/simple/simple.cpp >> > @@ -1353,8 +1353,20 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) >> > if (ret < 0) >> > return ret; >> > >> > - /* Configure the video node. */ >> > - V4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); >> > + /* Configure the video node, taking into account any Bayer pattern change. */ >> > + V4L2PixelFormat videoFormat; >> > + if (format.code == pipeConfig->code) { >> > + videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); >> > + } else { >> > + /* >> > + * Bayer pattern has changed because of the transform that was applied on >> > + * the sensor. Get the V4L2PixelFormat corresponding to the configured Bayer >> > + * pattern. >> > + */ >> > + BayerFormat cfgBayer = BayerFormat::fromPixelFormat(pipeConfig->captureFormat); >> > + cfgBayer.order = data->sensor_->bayerOrder(config->combinedTransform()); >> > + videoFormat = cfgBayer.toV4L2PixelFormat(); >> > + } >> > >> > V4L2DeviceFormat captureFormat; >> > captureFormat.fourcc = videoFormat; >>
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index efb07051..e2a48724 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1353,8 +1353,20 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) if (ret < 0) return ret; - /* Configure the video node. */ - V4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); + /* Configure the video node, taking into account any Bayer pattern change. */ + V4L2PixelFormat videoFormat; + if (format.code == pipeConfig->code) { + videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); + } else { + /* + * Bayer pattern has changed because of the transform that was applied on + * the sensor. Get the V4L2PixelFormat corresponding to the configured Bayer + * pattern. + */ + BayerFormat cfgBayer = BayerFormat::fromPixelFormat(pipeConfig->captureFormat); + cfgBayer.order = data->sensor_->bayerOrder(config->combinedTransform()); + videoFormat = cfgBayer.toV4L2PixelFormat(); + } V4L2DeviceFormat captureFormat; captureFormat.fourcc = videoFormat;
Bayer pattern on the sensor can change while configuring it with the intended capture format. This is due to the transform being applied on the sensor which supports [v/h]flips. During configure(), the simple pipeline handler does not detect any bayer pattern changes that can arise due to the transformations being applied via SimpleCameraData:setupFormats(). In such cases, the video node will be configured in-correctly, without realising the bayer pattern has changed on the sensor, for the given capture format. This patch detects the bayer pattern change after the sensor has been configured and retrieves the corresponding V4L2 pixel format to correctly configure the video node. Signed-off-by: Umang Jain <uajain@igalia.com> --- src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)