diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index a3a7289a7aa7..fb2f7ba3423c 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -25,14 +25,19 @@ class Request; class CameraConfiguration { public: + enum Status { + Valid, + Adjusted, + Invalid, + }; + using iterator = std::vector::iterator; using const_iterator = std::vector::const_iterator; - CameraConfiguration(); + virtual ~CameraConfiguration(); void addConfiguration(const StreamConfiguration &cfg); - - bool isValid() const; + virtual Status validate() = 0; StreamConfiguration &at(unsigned int index); const StreamConfiguration &at(unsigned int index) const; @@ -53,11 +58,13 @@ public: bool empty() const; std::size_t size() const; -private: +protected: + CameraConfiguration(); + std::vector config_; }; -class Camera final +class Camera final : public std::enable_shared_from_this { public: static std::shared_ptr create(PipelineHandler *pipe, diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 535c2420893e..5ecd7e0e38d7 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -116,7 +116,7 @@ static std::unique_ptr prepareCameraConfig() } std::unique_ptr config = camera->generateConfiguration(roles); - if (!config || !config->isValid()) { + if (!config || config->size() != roles.size()) { std::cerr << "Failed to get default stream configuration" << std::endl; return nullptr; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 0e5fd7f57271..b25a80bce1a2 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -52,6 +52,28 @@ LOG_DECLARE_CATEGORY(Camera) * operator[](int) returns a reference to the StreamConfiguration based on its * insertion index. Accessing a stream configuration with an invalid index * results in undefined behaviour. + * + * CameraConfiguration instances are retrieved from the camera with + * Camera::generateConfiguration(). Applications may then inspect the + * configuration, modify it, and possibly add new stream configuration entries + * with addConfiguration(). Once the camera configuration satisfies the + * application, it shall be validated by a call to validate(). The validation + * implements "try" semantics: it adjusts invalid configurations to the closest + * achievable parameters instead of rejecting them completely. Applications + * then decide whether to accept the modified configuration, or try again with + * a different set of parameters. Once the configuration is valid, it is passed + * to Camera::configure(). + */ + +/** + * \enum CameraConfiguration::Status + * \brief Validity of a camera configuration + * \var CameraConfiguration::Valid + * The configuration is fully valid + * \var CameraConfiguration::Adjusted + * The configuration has been adjusted to a valid configuration + * \var CameraConfiguration::Invalid + * The configuration is invalid and can't be adjusted automatically */ /** @@ -73,6 +95,10 @@ CameraConfiguration::CameraConfiguration() { } +CameraConfiguration::~CameraConfiguration() +{ +} + /** * \brief Add a stream configuration to the camera configuration * \param[in] cfg The stream configuration @@ -83,27 +109,31 @@ void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg) } /** - * \brief Check if the camera configuration is valid + * \fn CameraConfiguration::validate() + * \brief Validate and possibly adjust the camera configuration * - * A camera configuration is deemed to be valid if it contains at least one - * stream configuration and all stream configurations contain valid information. - * Stream configurations are deemed to be valid if all fields are none zero. + * This method adjusts the camera configuration to the closest valid + * configuration and returns the validation status. * - * \return True if the configuration is valid + * \todo: Define exactly when to return each status code. Should stream + * parameters set to 0 by the caller be adjusted without returning Adjusted ? + * This would potentially be useful for applications but would get in the way + * in Camera::configure(). Do we need an extra status code to signal this ? + * + * \todo: Handle validation of buffers count when refactoring the buffers API. + * + * \return A CameraConfiguration::Status value that describes the validation + * status. + * \retval CameraConfiguration::Invalid The configuration is invalid and can't + * be adjusted. This may only occur in extreme cases such as when the + * configuration is empty. + * \retval CameraConfigutation::Adjusted The configuration has been adjusted + * and is now valid. Parameters may have changed for any stream, and stream + * configurations may have been removed. The caller shall check the + * configuration carefully. + * \retval CameraConfiguration::Valid The configuration was already valid and + * hasn't been adjusted. */ -bool CameraConfiguration::isValid() const -{ - if (empty()) - return false; - - for (const StreamConfiguration &cfg : config_) { - if (cfg.size.width == 0 || cfg.size.height == 0 || - cfg.pixelFormat == 0 || cfg.bufferCount == 0) - return false; - } - - return true; -} /** * \brief Retrieve a reference to a stream configuration @@ -218,6 +248,11 @@ std::size_t CameraConfiguration::size() const return config_.size(); } +/** + * \var CameraConfiguration::config_ + * \brief The vector of stream configurations + */ + /** * \class Camera * \brief Camera device @@ -577,10 +612,9 @@ std::unique_ptr Camera::generateConfiguration(const StreamR * The caller specifies which streams are to be involved and their configuration * by populating \a config. * - * The easiest way to populate the array of config is to fetch an initial - * configuration from the camera with generateConfiguration() and then change - * the parameters to fit the caller's need and once all the streams parameters - * are configured hand that over to configure() to actually setup the camera. + * The configuration is created by generateConfiguration(), and adjusted by the + * caller with CameraConfiguration::validate(). This method only accepts fully + * valid configurations and returns an error if \a config is not valid. * * Exclusive access to the camera shall be ensured by a call to acquire() prior * to calling this function, otherwise an -EACCES error will be returned. @@ -605,7 +639,7 @@ int Camera::configure(CameraConfiguration *config) if (!stateBetween(CameraAcquired, CameraConfigured)) return -EACCES; - if (!config->isValid()) { + if (config->validate() != CameraConfiguration::Valid) { LOG(Camera, Error) << "Can't configure camera with invalid configuration"; return -EINVAL; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 5b46fb68ced2..05005c42106b 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -164,6 +164,33 @@ public: IPU3Stream vfStream_; }; +class IPU3CameraConfiguration : public CameraConfiguration +{ +public: + IPU3CameraConfiguration(Camera *camera, IPU3CameraData *data); + + Status validate() override; + + const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } + const std::vector &streams() { return streams_; } + +private: + static constexpr unsigned int IPU3_BUFFER_COUNT = 4; + + void adjustStream(StreamConfiguration &cfg, bool scale); + + /* + * The IPU3CameraData instance is guaranteed to be valid as long as the + * corresponding Camera instance is valid. In order to borrow a + * reference to the camera data, store a new reference to the camera. + */ + std::shared_ptr camera_; + const IPU3CameraData *data_; + + V4L2SubdeviceFormat sensorFormat_; + std::vector streams_; +}; + class PipelineHandlerIPU3 : public PipelineHandler { public: @@ -186,8 +213,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - static constexpr unsigned int IPU3_BUFFER_COUNT = 4; - IPU3CameraData *cameraData(const Camera *camera) { return static_cast( @@ -202,6 +227,151 @@ private: MediaDevice *imguMediaDev_; }; +IPU3CameraConfiguration::IPU3CameraConfiguration(Camera *camera, + IPU3CameraData *data) + : CameraConfiguration() +{ + camera_ = camera->shared_from_this(); + data_ = data; +} + +void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) +{ + /* The only pixel format the driver supports is NV12. */ + cfg.pixelFormat = V4L2_PIX_FMT_NV12; + + if (scale) { + /* + * Provide a suitable default that matches the sensor aspect + * ratio. + */ + if (!cfg.size.width || !cfg.size.height) { + cfg.size.width = 1280; + cfg.size.height = 1280 * sensorFormat_.size.height + / sensorFormat_.size.width; + } + + /* + * \todo: Clamp the size to the hardware bounds when we will + * figure them out. + * + * \todo: Handle the scaler (BDS) restrictions. The BDS can + * only scale with the same factor in both directions, and the + * scaling factor is limited to a multiple of 1/32. At the + * moment the ImgU driver hides these constraints by applying + * additional cropping, this should be fixed on the driver + * side, and cropping should be exposed to us. + */ + } else { + /* + * \todo: Properly support cropping when the ImgU driver + * interface will be cleaned up. + */ + cfg.size = sensorFormat_.size; + } + + /* + * Clamp the size to match the ImgU alignment constraints. The width + * shall be a multiple of 8 pixels and the height a multiple of 4 + * pixels. + */ + if (cfg.size.width % 8 || cfg.size.height % 4) { + cfg.size.width &= ~7; + cfg.size.height &= ~3; + } + + cfg.bufferCount = IPU3_BUFFER_COUNT; +} + +CameraConfiguration::Status IPU3CameraConfiguration::validate() +{ + const CameraSensor *sensor = data_->cio2_.sensor_; + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 2) { + config_.resize(2); + status = Adjusted; + } + + /* + * Select the sensor format by collecting the maximum width and height + * and picking the closest larger match, as the IPU3 can downscale + * only. If no resolution is requested for any stream, or if no sensor + * resolution is large enough, pick the largest one. + */ + Size size = {}; + + for (const StreamConfiguration &cfg : config_) { + if (cfg.size.width > size.width) + size.width = cfg.size.width; + if (cfg.size.height > size.height) + size.height = cfg.size.height; + } + + if (!size.width || !size.height) + size = sensor->resolution(); + + sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10 }, + size); + if (!sensorFormat_.size.width || !sensorFormat_.size.height) + sensorFormat_.size = sensor->resolution(); + + /* + * Verify and update all configuration entries, and assign a stream to + * each of them. The viewfinder stream can scale, while the output + * stream can crop only, so select the output stream when the requested + * resolution is equal to the sensor resolution, and the viewfinder + * stream otherwise. + */ + std::set availableStreams = { + &data_->outStream_, + &data_->vfStream_, + }; + + streams_.clear(); + streams_.reserve(config_.size()); + + for (unsigned int i = 0; i < config_.size(); ++i) { + StreamConfiguration &cfg = config_[i]; + const unsigned int pixelFormat = cfg.pixelFormat; + const Size size = cfg.size; + const IPU3Stream *stream; + + if (cfg.size == sensorFormat_.size) + stream = &data_->outStream_; + else + stream = &data_->vfStream_; + + if (availableStreams.find(stream) == availableStreams.end()) + stream = *availableStreams.begin(); + + LOG(IPU3, Debug) + << "Assigned '" << stream->name_ << "' to stream " << i; + + bool scale = stream == &data_->vfStream_; + adjustStream(config_[i], scale); + + if (cfg.pixelFormat != pixelFormat || cfg.size != size) { + LOG(IPU3, Debug) + << "Stream " << i << " configuration adjusted to " + << cfg.toString(); + status = Adjusted; + } + + streams_.push_back(stream); + availableStreams.erase(stream); + } + + return status; +} + PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) : PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr) { @@ -211,12 +381,14 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); - CameraConfiguration *config = new CameraConfiguration(); + IPU3CameraConfiguration *config; std::set streams = { &data->outStream_, &data->vfStream_, }; + config = new IPU3CameraConfiguration(camera, data); + for (const StreamRole role : roles) { StreamConfiguration cfg = {}; IPU3Stream *stream = nullptr; @@ -296,71 +468,25 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, streams.erase(stream); - cfg.pixelFormat = V4L2_PIX_FMT_NV12; - cfg.bufferCount = IPU3_BUFFER_COUNT; - config->addConfiguration(cfg); } + config->validate(); + return config; } -int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) { + IPU3CameraConfiguration *config = + static_cast(c); IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; IPU3Stream *vfStream = &data->vfStream_; CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; - Size sensorSize = {}; int ret; - outStream->active_ = false; - vfStream->active_ = false; - for (StreamConfiguration &cfg : *config) { - /* - * Pick a stream for the configuration entry. - * \todo: This is a naive temporary implementation that will be - * reworked when implementing camera configuration validation. - */ - IPU3Stream *stream = vfStream->active_ ? outStream : vfStream; - - /* - * Verify that the requested size respects the IPU3 alignment - * requirements (the image width shall be a multiple of 8 - * pixels and its height a multiple of 4 pixels) and the camera - * maximum sizes. - * - * \todo: Consider the BDS scaling factor requirements: "the - * downscaling factor must be an integer value multiple of 1/32" - */ - if (cfg.size.width % 8 || cfg.size.height % 4) { - LOG(IPU3, Error) - << "Invalid stream size: bad alignment"; - return -EINVAL; - } - - const Size &resolution = cio2->sensor_->resolution(); - if (cfg.size.width > resolution.width || - cfg.size.height > resolution.height) { - LOG(IPU3, Error) - << "Invalid stream size: larger than sensor resolution"; - return -EINVAL; - } - - /* - * Collect the maximum width and height: IPU3 can downscale - * only. - */ - if (cfg.size.width > sensorSize.width) - sensorSize.width = cfg.size.width; - if (cfg.size.height > sensorSize.height) - sensorSize.height = cfg.size.height; - - stream->active_ = true; - cfg.setStream(stream); - } - /* * \todo: Enable links selectively based on the requested streams. * As of now, enable all links unconditionally. @@ -373,6 +499,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) * Pass the requested stream size to the CIO2 unit and get back the * adjusted format to be propagated to the ImgU output devices. */ + const Size &sensorSize = config->sensorFormat().size; V4L2DeviceFormat cio2Format = {}; ret = cio2->configure(sensorSize, &cio2Format); if (ret) @@ -383,8 +510,22 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) return ret; /* Apply the format to the configured streams output devices. */ - for (StreamConfiguration &cfg : *config) { - IPU3Stream *stream = static_cast(cfg.stream()); + outStream->active_ = false; + vfStream->active_ = false; + + for (unsigned int i = 0; i < config->size(); ++i) { + /* + * Use a const_cast<> here instead of storing a mutable stream + * pointer in the configuration to let the compiler catch + * unwanted modifications of camera data in the configuration + * validate() implementation. + */ + IPU3Stream *stream = const_cast(config->streams()[i]); + StreamConfiguration &cfg = (*config)[i]; + + stream->active_ = true; + cfg.setStream(stream); + ret = imgu->configureOutput(stream->device_, cfg); if (ret) return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 8b279e76c0b0..9b3eea2f6dd3 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -5,6 +5,8 @@ * rkisp1.cpp - Pipeline handler for Rockchip ISP1 */ +#include +#include #include #include #include @@ -45,6 +47,29 @@ public: CameraSensor *sensor_; }; +class RkISP1CameraConfiguration : public CameraConfiguration +{ +public: + RkISP1CameraConfiguration(Camera *camera, RkISP1CameraData *data); + + Status validate() override; + + const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } + +private: + static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; + + /* + * The RkISP1CameraData instance is guaranteed to be valid as long as the + * corresponding Camera instance is valid. In order to borrow a + * reference to the camera data, store a new reference to the camera. + */ + std::shared_ptr camera_; + const RkISP1CameraData *data_; + + V4L2SubdeviceFormat sensorFormat_; +}; + class PipelineHandlerRkISP1 : public PipelineHandler { public: @@ -68,8 +93,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - RkISP1CameraData *cameraData(const Camera *camera) { return static_cast( @@ -88,6 +111,95 @@ private: Camera *activeCamera_; }; +RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera, + RkISP1CameraData *data) + : CameraConfiguration() +{ + camera_ = camera->shared_from_this(); + data_ = data; +} + +CameraConfiguration::Status RkISP1CameraConfiguration::validate() +{ + static const std::array formats{ + V4L2_PIX_FMT_YUYV, + V4L2_PIX_FMT_YVYU, + V4L2_PIX_FMT_VYUY, + V4L2_PIX_FMT_NV16, + V4L2_PIX_FMT_NV61, + V4L2_PIX_FMT_NV21, + V4L2_PIX_FMT_NV12, + V4L2_PIX_FMT_GREY, + }; + + const CameraSensor *sensor = data_->sensor_; + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 1) { + config_.resize(1); + status = Adjusted; + } + + StreamConfiguration &cfg = config_[0]; + + /* Adjust the pixel format. */ + if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) == + formats.end()) { + LOG(RkISP1, Debug) << "Adjusting format to NV12"; + cfg.pixelFormat = V4L2_PIX_FMT_NV12; + status = Adjusted; + } + + /* Select the sensor format. */ + sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12, + MEDIA_BUS_FMT_SGBRG12_1X12, + MEDIA_BUS_FMT_SGRBG12_1X12, + MEDIA_BUS_FMT_SRGGB12_1X12, + MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10, + MEDIA_BUS_FMT_SBGGR8_1X8, + MEDIA_BUS_FMT_SGBRG8_1X8, + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SRGGB8_1X8 }, + cfg.size); + if (!sensorFormat_.size.width || !sensorFormat_.size.height) + sensorFormat_.size = sensor->resolution(); + + /* + * Provide a suitable default that matches the sensor aspect + * ratio and clamp the size to the hardware bounds. + * + * \todo: Check the hardware alignment constraints. + */ + const Size size = cfg.size; + + if (!cfg.size.width || !cfg.size.height) { + cfg.size.width = 1280; + cfg.size.height = 1280 * sensorFormat_.size.height + / sensorFormat_.size.width; + } + + cfg.size.width = std::max(32U, std::min(4416U, cfg.size.width)); + cfg.size.height = std::max(16U, std::min(3312U, cfg.size.height)); + + if (cfg.size != size) { + LOG(RkISP1, Debug) + << "Adjusting size from " << size.toString() + << " to " << cfg.size.toString(); + status = Adjusted; + } + + cfg.bufferCount = RKISP1_BUFFER_COUNT; + + return status; +} + PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) : PipelineHandler(manager), dphy_(nullptr), isp_(nullptr), video_(nullptr) @@ -109,7 +221,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera const StreamRoles &roles) { RkISP1CameraData *data = cameraData(camera); - CameraConfiguration *config = new CameraConfiguration(); + CameraConfiguration *config = new RkISP1CameraConfiguration(camera, data); if (roles.empty()) return config; @@ -117,29 +229,23 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.size = data->sensor_->resolution(); - cfg.bufferCount = RKISP1_BUFFER_COUNT; config->addConfiguration(cfg); + config->validate(); + return config; } -int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config) +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) { + RkISP1CameraConfiguration *config = + static_cast(c); RkISP1CameraData *data = cameraData(camera); StreamConfiguration &cfg = config->at(0); CameraSensor *sensor = data->sensor_; int ret; - /* Verify the configuration. */ - const Size &resolution = sensor->resolution(); - if (cfg.size.width > resolution.width || - cfg.size.height > resolution.height) { - LOG(RkISP1, Error) - << "Invalid stream size: larger than sensor resolution"; - return -EINVAL; - } - /* * Configure the sensor links: enable the link corresponding to this * camera and disable all the other sensor links. @@ -167,21 +273,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config * Configure the format on the sensor output and propagate it through * the pipeline. */ - V4L2SubdeviceFormat format; - format = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12, - MEDIA_BUS_FMT_SGBRG12_1X12, - MEDIA_BUS_FMT_SGRBG12_1X12, - MEDIA_BUS_FMT_SRGGB12_1X12, - MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10, - MEDIA_BUS_FMT_SBGGR8_1X8, - MEDIA_BUS_FMT_SGBRG8_1X8, - MEDIA_BUS_FMT_SGRBG8_1X8, - MEDIA_BUS_FMT_SRGGB8_1X8 }, - cfg.size); - + V4L2SubdeviceFormat format = config->sensorFormat(); LOG(RkISP1, Debug) << "Configuring sensor with " << format.toString(); ret = sensor->setFormat(&format); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 4ffe52aa70d7..45260f34c8f5 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -39,6 +39,14 @@ public: Stream stream_; }; +class UVCCameraConfiguration : public CameraConfiguration +{ +public: + UVCCameraConfiguration(); + + Status validate() override; +}; + class PipelineHandlerUVC : public PipelineHandler { public: @@ -68,6 +76,45 @@ private: } }; +UVCCameraConfiguration::UVCCameraConfiguration() + : CameraConfiguration() +{ +} + +CameraConfiguration::Status UVCCameraConfiguration::validate() +{ + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 1) { + config_.resize(1); + status = Adjusted; + } + + StreamConfiguration &cfg = config_[0]; + + /* \todo: Validate the configuration against the device capabilities. */ + const unsigned int pixelFormat = cfg.pixelFormat; + const Size size = cfg.size; + + cfg.pixelFormat = V4L2_PIX_FMT_YUYV; + cfg.size = { 640, 480 }; + + if (cfg.pixelFormat != pixelFormat || cfg.size != size) { + LOG(UVC, Debug) + << "Adjusting configuration from " << cfg.toString() + << " to " << cfg.size.toString() << "-YUYV"; + status = Adjusted; + } + + cfg.bufferCount = 4; + + return status; +} + PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) : PipelineHandler(manager) { @@ -76,7 +123,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) { - CameraConfiguration *config = new CameraConfiguration(); + CameraConfiguration *config = new UVCCameraConfiguration(); if (roles.empty()) return config; @@ -88,6 +135,8 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, config->addConfiguration(cfg); + config->validate(); + return config; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index ed5b1ad4502a..0e4eede351d8 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -5,6 +5,9 @@ * vimc.cpp - Pipeline handler for the vimc device */ +#include +#include + #include #include #include @@ -39,6 +42,14 @@ public: Stream stream_; }; +class VimcCameraConfiguration : public CameraConfiguration +{ +public: + VimcCameraConfiguration(); + + Status validate() override; +}; + class PipelineHandlerVimc : public PipelineHandler { public: @@ -68,6 +79,57 @@ private: } }; +VimcCameraConfiguration::VimcCameraConfiguration() + : CameraConfiguration() +{ +} + +CameraConfiguration::Status VimcCameraConfiguration::validate() +{ + static const std::array formats{ + V4L2_PIX_FMT_BGR24, + V4L2_PIX_FMT_RGB24, + V4L2_PIX_FMT_ARGB32, + }; + + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 1) { + config_.resize(1); + status = Adjusted; + } + + StreamConfiguration &cfg = config_[0]; + + /* Adjust the pixel format. */ + if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) == + formats.end()) { + LOG(VIMC, Debug) << "Adjusting format to RGB24"; + cfg.pixelFormat = V4L2_PIX_FMT_RGB24; + status = Adjusted; + } + + /* Clamp the size based on the device limits. */ + const Size size = cfg.size; + + cfg.size.width = std::max(16U, std::min(4096U, cfg.size.width)); + cfg.size.height = std::max(16U, std::min(2160U, cfg.size.height)); + + if (cfg.size != size) { + LOG(VIMC, Debug) + << "Adjusting size to " << cfg.size.toString(); + status = Adjusted; + } + + cfg.bufferCount = 4; + + return status; +} + PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) : PipelineHandler(manager) { @@ -76,7 +138,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, const StreamRoles &roles) { - CameraConfiguration *config = new CameraConfiguration(); + CameraConfiguration *config = new VimcCameraConfiguration(); if (roles.empty()) return config; @@ -88,6 +150,8 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, config->addConfiguration(cfg); + config->validate(); + return config; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index de46e98880a2..dd56907d817e 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -248,17 +248,14 @@ void PipelineHandler::unlock() * is the Camera class which will receive configuration to apply from the * application. * - * Each pipeline handler implementation is responsible for validating - * that the configuration requested in \a config can be achieved - * exactly. Any difference in pixel format, frame size or any other - * parameter shall result in the -EINVAL error being returned, and no - * change in configuration being applied to the pipeline. If - * configuration of a subset of the streams can't be satisfied, the - * whole configuration is considered invalid. + * The configuration is guaranteed to have been validated with + * CameraConfiguration::valid(). The pipeline handler implementation shall not + * perform further validation and may rely on any custom field stored in its + * custom CameraConfiguration derived class. * - * Once the configuration is validated and the camera configured, the pipeline - * handler shall associate a Stream instance to each StreamConfiguration entry - * in the CameraConfiguration with the StreamConfiguration::setStream() method. + * When configuring the camera the pipeline handler shall associate a Stream + * instance to each StreamConfiguration entry in the CameraConfiguration using + * the StreamConfiguration::setStream() method. * * \return 0 on success or a negative error code otherwise */ diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index bb7d380cdc1a..c0835c250c65 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -45,7 +45,7 @@ protected: CameraTest::init(); config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config_) { + if (!config_ || config_->size() != 1) { cout << "Failed to generate default configuration" << endl; CameraTest::cleanup(); return TestFail; @@ -58,11 +58,6 @@ protected: { StreamConfiguration &cfg = config_->at(0); - if (!config_->isValid()) { - cout << "Failed to read default configuration" << endl; - return TestFail; - } - if (camera_->acquire()) { cout << "Failed to acquire the camera" << endl; return TestFail; diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index 8a767d2321e0..ce2ec5d02e7b 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -22,7 +22,7 @@ protected: /* Test asking for configuration for a video stream. */ config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config || !config->isValid()) { + if (!config || config->size() != 1) { cout << "Default configuration invalid" << endl; return TestFail; } @@ -32,7 +32,7 @@ protected: * stream roles returns an empty camera configuration. */ config = camera_->generateConfiguration({}); - if (!config || config->isValid()) { + if (!config || config->size() != 0) { cout << "Failed to retrieve configuration for empty roles list" << endl; return TestFail; diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index ece987c7752a..9f10f795a5d8 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -21,7 +21,7 @@ protected: CameraTest::init(); config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config_) { + if (!config_ || config_->size() != 1) { cout << "Failed to generate default configuration" << endl; CameraTest::cleanup(); return TestFail; @@ -34,11 +34,6 @@ protected: { StreamConfiguration &cfg = config_->at(0); - if (!config_->isValid()) { - cout << "Failed to read default configuration" << endl; - return TestFail; - } - if (camera_->acquire()) { cout << "Failed to acquire the camera" << endl; return TestFail;