Message ID | 20250124215806.158024-4-mzamazal@redhat.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Hi Milan, Thank you for the patch. On Fri, Jan 24, 2025 at 10:57:54PM +0100, Milan Zamazal wrote: > In order to support raw streams, we need to add raw formats to software > ISP configurations. In this preparatory patch, the raw formats are > excluded from software ISP output configurations. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 6e9bc630..06df909b 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -26,6 +26,7 @@ > > #include <libcamera/camera.h> > #include <libcamera/control_ids.h> > +#include <libcamera/pixel_format.h> > #include <libcamera/request.h> > #include <libcamera/stream.h> > > @@ -209,6 +210,12 @@ static const SimplePipelineInfo supportedDevices[] = { > { "sun6i-csi", {}, false }, > }; > > +bool isRawFormat(const PixelFormat &format) > +{ > + return libcamera::PixelFormatInfo::info(format).colourEncoding == > + libcamera::PixelFormatInfo::ColourEncodingRAW; > +} > + > } /* namespace */ > > class SimpleCameraData : public Camera::Private > @@ -1284,7 +1291,8 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > > cfg.setStream(&data->streams_[i]); > > - if (data->useConversion_) > + if (data->useConversion_ && > + (!data->swIsp_ || !isRawFormat(cfg.pixelFormat))) That's getting confusing. As the code is hard enough to follow, I'm not keen on making it even more obfuscated. At the moment, useConversion_ and swIsp_ are mutually exclusive. I understand this won't be the case anymore with this series, and useConversion_ will be true when using the software ISP and capturing both the raw and processed streams. If you want to reuse some of the converter infrastructure (including variables), I would like to see clear naming rules. We could for instance use convert* for data related to the hardware converters, swisp* for data related to the software ISP, and another prefix (name TBD) for data common to both. Ideally, of course, we should abstract both hardware converters and the software ISP with the same interface. > outputCfgs.push_back(cfg); > } >
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 6e9bc630..06df909b 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -26,6 +26,7 @@ #include <libcamera/camera.h> #include <libcamera/control_ids.h> +#include <libcamera/pixel_format.h> #include <libcamera/request.h> #include <libcamera/stream.h> @@ -209,6 +210,12 @@ static const SimplePipelineInfo supportedDevices[] = { { "sun6i-csi", {}, false }, }; +bool isRawFormat(const PixelFormat &format) +{ + return libcamera::PixelFormatInfo::info(format).colourEncoding == + libcamera::PixelFormatInfo::ColourEncodingRAW; +} + } /* namespace */ class SimpleCameraData : public Camera::Private @@ -1284,7 +1291,8 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) cfg.setStream(&data->streams_[i]); - if (data->useConversion_) + if (data->useConversion_ && + (!data->swIsp_ || !isRawFormat(cfg.pixelFormat))) outputCfgs.push_back(cfg); }
In order to support raw streams, we need to add raw formats to software ISP configurations. In this preparatory patch, the raw formats are excluded from software ISP output configurations. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/pipeline/simple/simple.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)