| Message ID | 20200925014207.1455796-18-niklas.soderlund@ragnatech.se |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Fri, Sep 25, 2020 at 03:42:02AM +0200, Niklas Söderlund wrote: > Move the path configuration generation and validation to RkISP1Path. > This is done to increase code reuse and to encapsulate the main and self > path differences inside the RkISP1Path class. There is no functional > change. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 115 ++----------------- > src/libcamera/pipeline/rkisp1/rkisp1path.cpp | 88 +++++++++++++- > src/libcamera/pipeline/rkisp1/rkisp1path.h | 18 ++- > 3 files changed, 112 insertions(+), 109 deletions(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 2403eec4691b5ff6..114aee3e180afb77 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -20,7 +20,6 @@ > #include <libcamera/formats.h> > #include <libcamera/ipa/rkisp1.h> > #include <libcamera/request.h> > -#include <libcamera/span.h> > #include <libcamera/stream.h> > > #include "libcamera/internal/camera_sensor.h" > @@ -40,34 +39,6 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(RkISP1) > > -namespace { > -constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 }; > -constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 }; > -constexpr std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{ > - formats::YUYV, > - formats::YVYU, > - formats::VYUY, > - formats::NV16, > - formats::NV61, > - formats::NV21, > - formats::NV12, > - /* \todo Add support for 8-bit greyscale to DRM formats */ > -}; > - > -constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 }; > -constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 }; > -constexpr std::array<PixelFormat, 7> RKISP1_RSZ_SP_FORMATS{ > - formats::YUYV, > - formats::YVYU, > - formats::VYUY, > - formats::NV16, > - formats::NV61, > - formats::NV21, > - formats::NV12, > - /* \todo Add support for BGR888 and RGB565 */ > -}; > -} /* namespace */ > - > class PipelineHandlerRkISP1; > class RkISP1ActionQueueBuffers; > class RkISP1CameraData; > @@ -194,14 +165,6 @@ public: > const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } > > private: > - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; > - > - CameraConfiguration::Status validatePath(StreamConfiguration *cfg, > - const Span<const PixelFormat> &formats, > - const Size &min, const Size &max, > - V4L2VideoDevice *video); > - CameraConfiguration::Status validateMainPath(StreamConfiguration *cfg); > - CameraConfiguration::Status validateSelfPath(StreamConfiguration *cfg); > bool fitsAllPaths(const StreamConfiguration &cfg); > > /* > @@ -514,64 +477,16 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera, > data_ = data; > } > > -CameraConfiguration::Status RkISP1CameraConfiguration::validatePath( > - StreamConfiguration *cfg, const Span<const PixelFormat> &formats, > - const Size &min, const Size &max, V4L2VideoDevice *video) > -{ > - const StreamConfiguration reqCfg = *cfg; > - Status status = Valid; > - > - if (std::find(formats.begin(), formats.end(), cfg->pixelFormat) == > - formats.end()) > - cfg->pixelFormat = formats::NV12; > - > - cfg->size.boundTo(max); > - cfg->size.expandTo(min); > - cfg->bufferCount = RKISP1_BUFFER_COUNT; > - > - V4L2DeviceFormat format = {}; > - format.fourcc = video->toV4L2PixelFormat(cfg->pixelFormat); > - format.size = cfg->size; > - > - int ret = video->tryFormat(&format); > - if (ret) > - return Invalid; > - > - cfg->stride = format.planes[0].bpl; > - cfg->frameSize = format.planes[0].size; > - > - if (cfg->pixelFormat != reqCfg.pixelFormat || cfg->size != reqCfg.size) { > - LOG(RkISP1, Debug) > - << "Adjusting format from " << reqCfg.toString() > - << " to " << cfg->toString(); > - status = Adjusted; > - } > - > - return status; > -} > - > -CameraConfiguration::Status RkISP1CameraConfiguration::validateMainPath(StreamConfiguration *cfg) > -{ > - return validatePath(cfg, RKISP1_RSZ_MP_FORMATS, RKISP1_RSZ_MP_SRC_MIN, > - RKISP1_RSZ_MP_SRC_MAX, data_->mainPath_->video_); > -} > - > -CameraConfiguration::Status RkISP1CameraConfiguration::validateSelfPath(StreamConfiguration *cfg) > -{ > - return validatePath(cfg, RKISP1_RSZ_SP_FORMATS, RKISP1_RSZ_SP_SRC_MIN, > - RKISP1_RSZ_SP_SRC_MAX, data_->selfPath_->video_); > -} > - > bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) > { > StreamConfiguration config; > > config = cfg; > - if (validateMainPath(&config) != Valid) > + if (data_->mainPath_->validate(&config) != Valid) > return false; > > config = cfg; > - if (validateSelfPath(&config) != Valid) > + if (data_->selfPath_->validate(&config) != Valid) > return false; > > return true; > @@ -611,7 +526,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream without adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (validateMainPath(&tryCfg) == Valid) { > + if (data_->mainPath_->validate(&tryCfg) == Valid) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -622,7 +537,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (validateSelfPath(&tryCfg) == Valid) { > + if (data_->selfPath_->validate(&tryCfg) == Valid) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -634,7 +549,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream allowing adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (validateMainPath(&tryCfg) == Adjusted) { > + if (data_->mainPath_->validate(&tryCfg) == Adjusted) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -646,7 +561,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (validateSelfPath(&tryCfg) == Adjusted) { > + if (data_->selfPath_->validate(&tryCfg) == Adjusted) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -734,25 +649,17 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera > return nullptr; > } > > - std::map<PixelFormat, std::vector<SizeRange>> streamFormats; > + StreamConfiguration cfg; > if (useMainPath) { > + cfg = data->mainPath_->generateConfiguration( > + data->sensor_->resolution()); > mainPathAvailable = false; > - for (const PixelFormat &format : RKISP1_RSZ_MP_FORMATS) > - streamFormats[format] = > - { { RKISP1_RSZ_MP_SRC_MIN, data->sensor_->resolution() } }; > } else { > + cfg = data->selfPath_->generateConfiguration( > + data->sensor_->resolution()); > selfPathAvailable = false; > - for (const PixelFormat &format : RKISP1_RSZ_SP_FORMATS) > - streamFormats[format] = > - { { RKISP1_RSZ_SP_SRC_MIN, data->sensor_->resolution() } }; > } > > - StreamFormats formats(streamFormats); > - StreamConfiguration cfg(formats); > - cfg.pixelFormat = formats::NV12; > - cfg.size = data->sensor_->resolution(); > - cfg.bufferCount = 4; > - > config->addConfiguration(cfg); > } > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp > index 758580934817ed6a..0be4d20bb1cd2094 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp > @@ -7,6 +7,7 @@ > > #include "rkisp1path.h" > > +#include <libcamera/formats.h> > #include <libcamera/stream.h> > > #include "libcamera/internal/media_device.h" > @@ -17,8 +18,11 @@ namespace libcamera { > > LOG_DECLARE_CATEGORY(RkISP1) > > -RkISP1Path::RkISP1Path(const char *name) > - : video_(nullptr), name_(name), resizer_(nullptr) > +RkISP1Path::RkISP1Path(const char *name, const std::vector<PixelFormat> &formats, > + const Size &minResolution, const Size &maxResolution) > + : video_(nullptr), name_(name), formats_(formats), > + minResolution_(minResolution), maxResolution_(maxResolution), > + resizer_(nullptr) > { > } > > @@ -44,6 +48,58 @@ bool RkISP1Path::init(MediaDevice *media) > return true; > } > > +StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution) > +{ > + Size maxResolution = resolution; > + maxResolution.boundTo(maxResolution_); > + > + std::map<PixelFormat, std::vector<SizeRange>> streamFormats; > + for (const PixelFormat &format : formats_) > + streamFormats[format] = { { minResolution_, maxResolution } }; > + > + StreamFormats formats(streamFormats); > + StreamConfiguration cfg(formats); > + cfg.pixelFormat = formats::NV12; > + cfg.size = maxResolution; > + cfg.bufferCount = RKISP1_BUFFER_COUNT; > + > + return cfg; > +} > + > +CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) > +{ > + const StreamConfiguration reqCfg = *cfg; > + CameraConfiguration::Status status = CameraConfiguration::Valid; > + > + if (std::find(formats_.begin(), formats_.end(), cfg->pixelFormat) == > + formats_.end()) > + cfg->pixelFormat = formats::NV12; > + > + cfg->size.boundTo(maxResolution_); > + cfg->size.expandTo(minResolution_); > + cfg->bufferCount = RKISP1_BUFFER_COUNT; > + > + V4L2DeviceFormat format = {}; > + format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); > + format.size = cfg->size; > + > + int ret = video_->tryFormat(&format); > + if (ret) > + return CameraConfiguration::Invalid; > + > + cfg->stride = format.planes[0].bpl; > + cfg->frameSize = format.planes[0].size; > + > + if (cfg->pixelFormat != reqCfg.pixelFormat || cfg->size != reqCfg.size) { > + LOG(RkISP1, Debug) > + << "Adjusting format from " << reqCfg.toString() > + << " to " << cfg->toString(); > + status = CameraConfiguration::Adjusted; > + } > + > + return status; > +} > + > int RkISP1Path::configure(const StreamConfiguration &config, > const V4L2SubdeviceFormat &inputFormat) > { > @@ -94,12 +150,36 @@ int RkISP1Path::configure(const StreamConfiguration &config, > } > > RkISP1MainPath::RkISP1MainPath() > - : RkISP1Path("main") > + : RkISP1Path("main", > + { > + formats::YUYV, > + formats::YVYU, > + formats::VYUY, > + formats::NV16, > + formats::NV61, > + formats::NV21, > + formats::NV12, > + /* \todo Add support for 8-bit greyscale to DRM formats */ > + }, > + { 32, 16 }, > + { 4416, 3312 }) > { > } > > RkISP1SelfPath::RkISP1SelfPath() > - : RkISP1Path("self") > + : RkISP1Path("self", > + { > + formats::YUYV, > + formats::YVYU, > + formats::VYUY, > + formats::NV16, > + formats::NV61, > + formats::NV21, > + formats::NV12, > + /* \todo Add support for BGR888 and RGB565 */ > + }, > + { 32, 16 }, > + { 1920, 1920 }) > { > } > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.h b/src/libcamera/pipeline/rkisp1/rkisp1path.h > index 6eb01529d2fddb1c..5b2917c746ee1d95 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1path.h > +++ b/src/libcamera/pipeline/rkisp1/rkisp1path.h > @@ -7,9 +7,15 @@ > #ifndef __LIBCAMERA_PIPELINE_RKISP1_PATH_H__ > #define __LIBCAMERA_PIPELINE_RKISP1_PATH_H__ > > +#include <vector> > + > +#include <libcamera/camera.h> > + > namespace libcamera { > > class MediaDevice; > +class PixelFormat; > +class Size; You need to include the corresponding headers to avoid depending on indirect includes, as those classes are embedded in RkISP1Path. > class V4L2Subdevice; > class V4L2VideoDevice; > struct StreamConfiguration; > @@ -18,11 +24,15 @@ struct V4L2SubdeviceFormat; > class RkISP1Path > { > public: > - RkISP1Path(const char *name); > + RkISP1Path(const char *name, const std::vector<PixelFormat> &formats, > + const Size &minResolution, const Size &maxResolution); > ~RkISP1Path(); > > bool init(MediaDevice *media); > > + StreamConfiguration generateConfiguration(const Size &resolution); > + CameraConfiguration::Status validate(StreamConfiguration *cfg); > + > int configure(const StreamConfiguration &config, > const V4L2SubdeviceFormat &inputFormat); > > @@ -30,8 +40,14 @@ public: > V4L2VideoDevice *video_; > > private: > + static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; > + > const char *name_; > > + const std::vector<PixelFormat> formats_; Could we keep the formats array as constexpr globals and store a span here (and of course move them to rkisp1path.cpp) ? I'm tempted to do the same for the min/max sizes, as that would look less like magic numbers. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + const Size minResolution_; > + const Size maxResolution_; > + > V4L2Subdevice *resizer_; > }; >
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 2403eec4691b5ff6..114aee3e180afb77 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -20,7 +20,6 @@ #include <libcamera/formats.h> #include <libcamera/ipa/rkisp1.h> #include <libcamera/request.h> -#include <libcamera/span.h> #include <libcamera/stream.h> #include "libcamera/internal/camera_sensor.h" @@ -40,34 +39,6 @@ namespace libcamera { LOG_DEFINE_CATEGORY(RkISP1) -namespace { -constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 }; -constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 }; -constexpr std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{ - formats::YUYV, - formats::YVYU, - formats::VYUY, - formats::NV16, - formats::NV61, - formats::NV21, - formats::NV12, - /* \todo Add support for 8-bit greyscale to DRM formats */ -}; - -constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 }; -constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 }; -constexpr std::array<PixelFormat, 7> RKISP1_RSZ_SP_FORMATS{ - formats::YUYV, - formats::YVYU, - formats::VYUY, - formats::NV16, - formats::NV61, - formats::NV21, - formats::NV12, - /* \todo Add support for BGR888 and RGB565 */ -}; -} /* namespace */ - class PipelineHandlerRkISP1; class RkISP1ActionQueueBuffers; class RkISP1CameraData; @@ -194,14 +165,6 @@ public: const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } private: - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - - CameraConfiguration::Status validatePath(StreamConfiguration *cfg, - const Span<const PixelFormat> &formats, - const Size &min, const Size &max, - V4L2VideoDevice *video); - CameraConfiguration::Status validateMainPath(StreamConfiguration *cfg); - CameraConfiguration::Status validateSelfPath(StreamConfiguration *cfg); bool fitsAllPaths(const StreamConfiguration &cfg); /* @@ -514,64 +477,16 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera, data_ = data; } -CameraConfiguration::Status RkISP1CameraConfiguration::validatePath( - StreamConfiguration *cfg, const Span<const PixelFormat> &formats, - const Size &min, const Size &max, V4L2VideoDevice *video) -{ - const StreamConfiguration reqCfg = *cfg; - Status status = Valid; - - if (std::find(formats.begin(), formats.end(), cfg->pixelFormat) == - formats.end()) - cfg->pixelFormat = formats::NV12; - - cfg->size.boundTo(max); - cfg->size.expandTo(min); - cfg->bufferCount = RKISP1_BUFFER_COUNT; - - V4L2DeviceFormat format = {}; - format.fourcc = video->toV4L2PixelFormat(cfg->pixelFormat); - format.size = cfg->size; - - int ret = video->tryFormat(&format); - if (ret) - return Invalid; - - cfg->stride = format.planes[0].bpl; - cfg->frameSize = format.planes[0].size; - - if (cfg->pixelFormat != reqCfg.pixelFormat || cfg->size != reqCfg.size) { - LOG(RkISP1, Debug) - << "Adjusting format from " << reqCfg.toString() - << " to " << cfg->toString(); - status = Adjusted; - } - - return status; -} - -CameraConfiguration::Status RkISP1CameraConfiguration::validateMainPath(StreamConfiguration *cfg) -{ - return validatePath(cfg, RKISP1_RSZ_MP_FORMATS, RKISP1_RSZ_MP_SRC_MIN, - RKISP1_RSZ_MP_SRC_MAX, data_->mainPath_->video_); -} - -CameraConfiguration::Status RkISP1CameraConfiguration::validateSelfPath(StreamConfiguration *cfg) -{ - return validatePath(cfg, RKISP1_RSZ_SP_FORMATS, RKISP1_RSZ_SP_SRC_MIN, - RKISP1_RSZ_SP_SRC_MAX, data_->selfPath_->video_); -} - bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) { StreamConfiguration config; config = cfg; - if (validateMainPath(&config) != Valid) + if (data_->mainPath_->validate(&config) != Valid) return false; config = cfg; - if (validateSelfPath(&config) != Valid) + if (data_->selfPath_->validate(&config) != Valid) return false; return true; @@ -611,7 +526,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() /* Try to match stream without adjusting configuration. */ if (mainPathAvailable) { StreamConfiguration tryCfg = cfg; - if (validateMainPath(&tryCfg) == Valid) { + if (data_->mainPath_->validate(&tryCfg) == Valid) { mainPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); @@ -622,7 +537,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (selfPathAvailable) { StreamConfiguration tryCfg = cfg; - if (validateSelfPath(&tryCfg) == Valid) { + if (data_->selfPath_->validate(&tryCfg) == Valid) { selfPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); @@ -634,7 +549,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() /* Try to match stream allowing adjusting configuration. */ if (mainPathAvailable) { StreamConfiguration tryCfg = cfg; - if (validateMainPath(&tryCfg) == Adjusted) { + if (data_->mainPath_->validate(&tryCfg) == Adjusted) { mainPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); @@ -646,7 +561,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (selfPathAvailable) { StreamConfiguration tryCfg = cfg; - if (validateSelfPath(&tryCfg) == Adjusted) { + if (data_->selfPath_->validate(&tryCfg) == Adjusted) { selfPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); @@ -734,25 +649,17 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera return nullptr; } - std::map<PixelFormat, std::vector<SizeRange>> streamFormats; + StreamConfiguration cfg; if (useMainPath) { + cfg = data->mainPath_->generateConfiguration( + data->sensor_->resolution()); mainPathAvailable = false; - for (const PixelFormat &format : RKISP1_RSZ_MP_FORMATS) - streamFormats[format] = - { { RKISP1_RSZ_MP_SRC_MIN, data->sensor_->resolution() } }; } else { + cfg = data->selfPath_->generateConfiguration( + data->sensor_->resolution()); selfPathAvailable = false; - for (const PixelFormat &format : RKISP1_RSZ_SP_FORMATS) - streamFormats[format] = - { { RKISP1_RSZ_SP_SRC_MIN, data->sensor_->resolution() } }; } - StreamFormats formats(streamFormats); - StreamConfiguration cfg(formats); - cfg.pixelFormat = formats::NV12; - cfg.size = data->sensor_->resolution(); - cfg.bufferCount = 4; - config->addConfiguration(cfg); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp index 758580934817ed6a..0be4d20bb1cd2094 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp @@ -7,6 +7,7 @@ #include "rkisp1path.h" +#include <libcamera/formats.h> #include <libcamera/stream.h> #include "libcamera/internal/media_device.h" @@ -17,8 +18,11 @@ namespace libcamera { LOG_DECLARE_CATEGORY(RkISP1) -RkISP1Path::RkISP1Path(const char *name) - : video_(nullptr), name_(name), resizer_(nullptr) +RkISP1Path::RkISP1Path(const char *name, const std::vector<PixelFormat> &formats, + const Size &minResolution, const Size &maxResolution) + : video_(nullptr), name_(name), formats_(formats), + minResolution_(minResolution), maxResolution_(maxResolution), + resizer_(nullptr) { } @@ -44,6 +48,58 @@ bool RkISP1Path::init(MediaDevice *media) return true; } +StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution) +{ + Size maxResolution = resolution; + maxResolution.boundTo(maxResolution_); + + std::map<PixelFormat, std::vector<SizeRange>> streamFormats; + for (const PixelFormat &format : formats_) + streamFormats[format] = { { minResolution_, maxResolution } }; + + StreamFormats formats(streamFormats); + StreamConfiguration cfg(formats); + cfg.pixelFormat = formats::NV12; + cfg.size = maxResolution; + cfg.bufferCount = RKISP1_BUFFER_COUNT; + + return cfg; +} + +CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) +{ + const StreamConfiguration reqCfg = *cfg; + CameraConfiguration::Status status = CameraConfiguration::Valid; + + if (std::find(formats_.begin(), formats_.end(), cfg->pixelFormat) == + formats_.end()) + cfg->pixelFormat = formats::NV12; + + cfg->size.boundTo(maxResolution_); + cfg->size.expandTo(minResolution_); + cfg->bufferCount = RKISP1_BUFFER_COUNT; + + V4L2DeviceFormat format = {}; + format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); + format.size = cfg->size; + + int ret = video_->tryFormat(&format); + if (ret) + return CameraConfiguration::Invalid; + + cfg->stride = format.planes[0].bpl; + cfg->frameSize = format.planes[0].size; + + if (cfg->pixelFormat != reqCfg.pixelFormat || cfg->size != reqCfg.size) { + LOG(RkISP1, Debug) + << "Adjusting format from " << reqCfg.toString() + << " to " << cfg->toString(); + status = CameraConfiguration::Adjusted; + } + + return status; +} + int RkISP1Path::configure(const StreamConfiguration &config, const V4L2SubdeviceFormat &inputFormat) { @@ -94,12 +150,36 @@ int RkISP1Path::configure(const StreamConfiguration &config, } RkISP1MainPath::RkISP1MainPath() - : RkISP1Path("main") + : RkISP1Path("main", + { + formats::YUYV, + formats::YVYU, + formats::VYUY, + formats::NV16, + formats::NV61, + formats::NV21, + formats::NV12, + /* \todo Add support for 8-bit greyscale to DRM formats */ + }, + { 32, 16 }, + { 4416, 3312 }) { } RkISP1SelfPath::RkISP1SelfPath() - : RkISP1Path("self") + : RkISP1Path("self", + { + formats::YUYV, + formats::YVYU, + formats::VYUY, + formats::NV16, + formats::NV61, + formats::NV21, + formats::NV12, + /* \todo Add support for BGR888 and RGB565 */ + }, + { 32, 16 }, + { 1920, 1920 }) { } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.h b/src/libcamera/pipeline/rkisp1/rkisp1path.h index 6eb01529d2fddb1c..5b2917c746ee1d95 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1path.h @@ -7,9 +7,15 @@ #ifndef __LIBCAMERA_PIPELINE_RKISP1_PATH_H__ #define __LIBCAMERA_PIPELINE_RKISP1_PATH_H__ +#include <vector> + +#include <libcamera/camera.h> + namespace libcamera { class MediaDevice; +class PixelFormat; +class Size; class V4L2Subdevice; class V4L2VideoDevice; struct StreamConfiguration; @@ -18,11 +24,15 @@ struct V4L2SubdeviceFormat; class RkISP1Path { public: - RkISP1Path(const char *name); + RkISP1Path(const char *name, const std::vector<PixelFormat> &formats, + const Size &minResolution, const Size &maxResolution); ~RkISP1Path(); bool init(MediaDevice *media); + StreamConfiguration generateConfiguration(const Size &resolution); + CameraConfiguration::Status validate(StreamConfiguration *cfg); + int configure(const StreamConfiguration &config, const V4L2SubdeviceFormat &inputFormat); @@ -30,8 +40,14 @@ public: V4L2VideoDevice *video_; private: + static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; + const char *name_; + const std::vector<PixelFormat> formats_; + const Size minResolution_; + const Size maxResolution_; + V4L2Subdevice *resizer_; };
Move the path configuration generation and validation to RkISP1Path. This is done to increase code reuse and to encapsulate the main and self path differences inside the RkISP1Path class. There is no functional change. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 115 ++----------------- src/libcamera/pipeline/rkisp1/rkisp1path.cpp | 88 +++++++++++++- src/libcamera/pipeline/rkisp1/rkisp1path.h | 18 ++- 3 files changed, 112 insertions(+), 109 deletions(-)