| Message ID | 20200503123439.15994-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2020-05-03 15:34:39 +0300, Laurent Pinchart wrote: > The CameraSensor class assumes that camera sensors support the exact > same list of sizes of all media bus codes. While allowing a simpler API, > this assumption is incorrect and is blocking usage of some camera > sensors. > > Relaxing the constraint is possible without changes to the CameraSensor > API syntax, but requires changing its semantics. The sizes() function > now returns the list of all sizes for all media bus codes, and the > getFormat() function now searches in all supported media bus codes. The > former is likely not the most useful option for pipeline handlers, but > the sizes() function is currently unused. Designing a better API will > require inspecting current and expected future use cases in pipeline > handlers to determine proper heuristics. > > While at it, fix a small typo in an unrelated comment. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/camera_sensor.cpp | 135 +++++++++++++------------- > src/libcamera/include/camera_sensor.h | 5 +- > 2 files changed, 69 insertions(+), 71 deletions(-) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 4154700a19b5..abafb30eb977 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -119,8 +119,7 @@ LOG_DEFINE_CATEGORY(CameraSensor); > * information. > * > * The implementation is currently limited to sensors that expose a single V4L2 > - * subdevice with a single pad, and support the same frame sizes for all > - * supported media bus codes. It will be extended to support more complex > + * subdevice with a single pad. It will be extended to support more complex > * devices as the needs arise. > */ > > @@ -245,36 +244,34 @@ int CameraSensor::init() > propertyValue = 0; > properties_.set(properties::Rotation, propertyValue); > > - /* Enumerate and cache media bus codes and sizes. */ > - const ImageFormats formats = subdev_->formats(pad_); > - if (formats.isEmpty()) { > + /* Enumerate, sort and cache media bus codes and sizes. */ > + formats_ = subdev_->formats(pad_); > + if (formats_.isEmpty()) { > LOG(CameraSensor, Error) << "No image format found"; > return -EINVAL; > } > > - mbusCodes_ = formats.formats(); > - > - /* > - * Extract the supported sizes from the first format as we only support > - * sensors that offer the same frame sizes for all media bus codes. > - * Verify this assumption and reject the sensor if it isn't true. > - */ > - const std::vector<SizeRange> &sizes = formats.sizes(mbusCodes_[0]); > - std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_), > - [](const SizeRange &range) { return range.max; }); > - > - for (unsigned int code : mbusCodes_) { > - if (formats.sizes(code) != sizes) { > - LOG(CameraSensor, Error) > - << "Frame sizes differ between media bus codes"; > - return -EINVAL; > - } > - } > - > - /* Sort the media bus codes and sizes. */ > + mbusCodes_ = formats_.formats(); > std::sort(mbusCodes_.begin(), mbusCodes_.end()); > + > + for (const auto format : formats_.data()) { > + const std::vector<SizeRange> &ranges = format.second; > + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_), > + [](const SizeRange &range) { return range.max; }); > + } > + > std::sort(sizes_.begin(), sizes_.end()); > > + /* Remove duplicates. */ > + auto last = std::unique(sizes_.begin(), sizes_.end()); > + sizes_.erase(last, sizes_.end()); > + > + /* > + * The sizes_ vector is sorted in ascending order, the resolution is > + * thus the last element of the vector. > + */ > + resolution_ = sizes_.back(); > + > return 0; > } > > @@ -303,21 +300,18 @@ int CameraSensor::init() > /** > * \fn CameraSensor::sizes() > * \brief Retrieve the frame sizes supported by the camera sensor > + * > + * The reported sizes span all media bus codes supported by the camera sensor. > + * Not all sizes may be supported by all media bus codes. > + * > * \return The supported frame sizes sorted in increasing order > */ > > /** > + * \fn CameraSensor::resolution() > * \brief Retrieve the camera sensor resolution > * \return The camera sensor resolution in pixels > */ > -const Size &CameraSensor::resolution() const > -{ > - /* > - * The sizes_ vector is sorted in ascending order, the resolution is > - * thus the last element of the vector. > - */ > - return sizes_.back(); > -} > > /** > * \brief Retrieve the best sensor format for a desired output > @@ -325,13 +319,13 @@ const Size &CameraSensor::resolution() const > * \param[in] size The desired size > * > * Media bus codes are selected from \a mbusCodes, which lists all acceptable > - * codes in decreasing order of preference. This method selects the first code > - * from the list that is supported by the sensor. If none of the desired codes > - * is supported, it returns an error. > + * codes in decreasing order of preference. Media bus codes supported by the > + * sensor but not listed in \a mbusCodes are ignored. If none of the desired > + * codes is supported, it returns an error. > * > * \a size indicates the desired size at the output of the sensor. This method > - * selects the best size supported by the sensor according to the following > - * criteria. > + * selects the best media bus code and size supported by the sensor according > + * to the following criteria. > * > * - The desired \a size shall fit in the sensor output size to avoid the need > * to up-scale. > @@ -339,6 +333,11 @@ const Size &CameraSensor::resolution() const > * need to crop the field of view. > * - The sensor output size shall be as small as possible to lower the required > * bandwidth. > + * - The desired \a size shall be supported by one of the media bus code listed > + * in \a mbusCodes. > + * > + * When multiple media bus codes can produce the same size, the code at the > + * lowest position in \a mbusCodes is selected. > * > * The use of this method is optional, as the above criteria may not match the > * needs of all pipeline handlers. Pipeline handlers may implement custom > @@ -353,52 +352,48 @@ const Size &CameraSensor::resolution() const > V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbusCodes, > const Size &size) const > { > - V4L2SubdeviceFormat format{}; > - > - for (unsigned int code : mbusCodes) { > - if (std::any_of(mbusCodes_.begin(), mbusCodes_.end(), > - [code](unsigned int c) { return c == code; })) { > - format.mbus_code = code; > - break; > - } > - } > - > - if (!format.mbus_code) { > - LOG(CameraSensor, Debug) << "No supported format found"; > - return format; > - } > - > unsigned int desiredArea = size.width * size.height; > unsigned int bestArea = UINT_MAX; > float desiredRatio = static_cast<float>(size.width) / size.height; > float bestRatio = FLT_MAX; > const Size *bestSize = nullptr; > + uint32_t bestCode = 0; > > - for (const Size &sz : sizes_) { > - if (sz.width < size.width || sz.height < size.height) > - continue; > + for (unsigned int code : mbusCodes) { > + const std::vector<SizeRange> &ranges = formats_.sizes(code); > > - float ratio = static_cast<float>(sz.width) / sz.height; > - float ratioDiff = fabsf(ratio - desiredRatio); > - unsigned int area = sz.width * sz.height; > - unsigned int areaDiff = area - desiredArea; > + for (const SizeRange &range : ranges) { > + const Size &sz = range.max; > > - if (ratioDiff > bestRatio) > - continue; > + if (sz.width < size.width || sz.height < size.height) > + continue; > > - if (ratioDiff < bestRatio || areaDiff < bestArea) { > - bestRatio = ratioDiff; > - bestArea = areaDiff; > - bestSize = &sz; > + float ratio = static_cast<float>(sz.width) / sz.height; > + float ratioDiff = fabsf(ratio - desiredRatio); > + unsigned int area = sz.width * sz.height; > + unsigned int areaDiff = area - desiredArea; > + > + if (ratioDiff > bestRatio) > + continue; > + > + if (ratioDiff < bestRatio || areaDiff < bestArea) { > + bestRatio = ratioDiff; > + bestArea = areaDiff; > + bestSize = &sz; > + bestCode = code; > + } > } > } > > if (!bestSize) { > - LOG(CameraSensor, Debug) << "No supported size found"; > - return format; > + LOG(CameraSensor, Debug) << "No supported format or size found"; > + return {}; > } > > - format.size = *bestSize; > + V4L2SubdeviceFormat format{ > + .mbus_code = bestCode, > + .size = *bestSize, > + }; > > return format; > } > @@ -424,7 +419,7 @@ const ControlInfoMap &CameraSensor::controls() const > > /** > * \brief Read controls from the sensor > - * \param[in] ids The list of control to read, specified by their ID > + * \param[in] ids The list of controls to read, specified by their ID > * > * This method reads the value of all controls contained in \a ids, and returns > * their values as a ControlList. > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h > index 24993b74148f..30cf5f34f485 100644 > --- a/src/libcamera/include/camera_sensor.h > +++ b/src/libcamera/include/camera_sensor.h > @@ -14,6 +14,7 @@ > #include <libcamera/controls.h> > #include <libcamera/geometry.h> > > +#include "formats.h" > #include "log.h" > > namespace libcamera { > @@ -51,7 +52,7 @@ public: > const MediaEntity *entity() const { return entity_; } > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > const std::vector<Size> &sizes() const { return sizes_; } > - const Size &resolution() const; > + const Size &resolution() const { return resolution_; } > > V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes, > const Size &size) const; > @@ -74,6 +75,8 @@ private: > > std::string model_; > > + ImageFormats formats_; > + Size resolution_; > std::vector<unsigned int> mbusCodes_; > std::vector<Size> sizes_; > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hello Laurent, Thank you for the patch. :-) On Sun, May 03, 2020 at 03:34:39PM +0300, Laurent Pinchart wrote: > The CameraSensor class assumes that camera sensors support the exact > same list of sizes of all media bus codes. While allowing a simpler API, > this assumption is incorrect and is blocking usage of some camera > sensors. > > Relaxing the constraint is possible without changes to the CameraSensor > API syntax, but requires changing its semantics. The sizes() function > now returns the list of all sizes for all media bus codes, and the > getFormat() function now searches in all supported media bus codes. The > former is likely not the most useful option for pipeline handlers, but > the sizes() function is currently unused. Designing a better API will > require inspecting current and expected future use cases in pipeline > handlers to determine proper heuristics. > > While at it, fix a small typo in an unrelated comment. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera_sensor.cpp | 135 +++++++++++++------------- > src/libcamera/include/camera_sensor.h | 5 +- > 2 files changed, 69 insertions(+), 71 deletions(-) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 4154700a19b5..abafb30eb977 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -119,8 +119,7 @@ LOG_DEFINE_CATEGORY(CameraSensor); > * information. > * > * The implementation is currently limited to sensors that expose a single V4L2 > - * subdevice with a single pad, and support the same frame sizes for all > - * supported media bus codes. It will be extended to support more complex > + * subdevice with a single pad. It will be extended to support more complex > * devices as the needs arise. > */ > > @@ -245,36 +244,34 @@ int CameraSensor::init() > propertyValue = 0; > properties_.set(properties::Rotation, propertyValue); > > - /* Enumerate and cache media bus codes and sizes. */ > - const ImageFormats formats = subdev_->formats(pad_); > - if (formats.isEmpty()) { > + /* Enumerate, sort and cache media bus codes and sizes. */ > + formats_ = subdev_->formats(pad_); > + if (formats_.isEmpty()) { > LOG(CameraSensor, Error) << "No image format found"; > return -EINVAL; > } > > - mbusCodes_ = formats.formats(); > - > - /* > - * Extract the supported sizes from the first format as we only support > - * sensors that offer the same frame sizes for all media bus codes. > - * Verify this assumption and reject the sensor if it isn't true. > - */ > - const std::vector<SizeRange> &sizes = formats.sizes(mbusCodes_[0]); > - std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_), > - [](const SizeRange &range) { return range.max; }); > - > - for (unsigned int code : mbusCodes_) { > - if (formats.sizes(code) != sizes) { > - LOG(CameraSensor, Error) > - << "Frame sizes differ between media bus codes"; > - return -EINVAL; > - } > - } > - > - /* Sort the media bus codes and sizes. */ > + mbusCodes_ = formats_.formats(); > std::sort(mbusCodes_.begin(), mbusCodes_.end()); > + > + for (const auto format : formats_.data()) { This should be for (const auto &format : formats_.data()) { > + const std::vector<SizeRange> &ranges = format.second; > + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_), > + [](const SizeRange &range) { return range.max; }); > + } > + > std::sort(sizes_.begin(), sizes_.end()); > > + /* Remove duplicates. */ > + auto last = std::unique(sizes_.begin(), sizes_.end()); > + sizes_.erase(last, sizes_.end()); > + > + /* > + * The sizes_ vector is sorted in ascending order, the resolution is > + * thus the last element of the vector. > + */ > + resolution_ = sizes_.back(); > + > return 0; > } > > @@ -303,21 +300,18 @@ int CameraSensor::init() > /** > * \fn CameraSensor::sizes() > * \brief Retrieve the frame sizes supported by the camera sensor > + * > + * The reported sizes span all media bus codes supported by the camera sensor. > + * Not all sizes may be supported by all media bus codes. > + * > * \return The supported frame sizes sorted in increasing order > */ > > /** > + * \fn CameraSensor::resolution() > * \brief Retrieve the camera sensor resolution > * \return The camera sensor resolution in pixels > */ > -const Size &CameraSensor::resolution() const > -{ > - /* > - * The sizes_ vector is sorted in ascending order, the resolution is > - * thus the last element of the vector. > - */ > - return sizes_.back(); > -} > > /** > * \brief Retrieve the best sensor format for a desired output > @@ -325,13 +319,13 @@ const Size &CameraSensor::resolution() const > * \param[in] size The desired size > * > * Media bus codes are selected from \a mbusCodes, which lists all acceptable > - * codes in decreasing order of preference. This method selects the first code > - * from the list that is supported by the sensor. If none of the desired codes > - * is supported, it returns an error. > + * codes in decreasing order of preference. Media bus codes supported by the > + * sensor but not listed in \a mbusCodes are ignored. If none of the desired > + * codes is supported, it returns an error. > * > * \a size indicates the desired size at the output of the sensor. This method > - * selects the best size supported by the sensor according to the following > - * criteria. > + * selects the best media bus code and size supported by the sensor according > + * to the following criteria. > * > * - The desired \a size shall fit in the sensor output size to avoid the need > * to up-scale. > @@ -339,6 +333,11 @@ const Size &CameraSensor::resolution() const > * need to crop the field of view. > * - The sensor output size shall be as small as possible to lower the required > * bandwidth. > + * - The desired \a size shall be supported by one of the media bus code listed > + * in \a mbusCodes. > + * > + * When multiple media bus codes can produce the same size, the code at the > + * lowest position in \a mbusCodes is selected. > * > * The use of this method is optional, as the above criteria may not match the > * needs of all pipeline handlers. Pipeline handlers may implement custom > @@ -353,52 +352,48 @@ const Size &CameraSensor::resolution() const > V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbusCodes, > const Size &size) const > { > - V4L2SubdeviceFormat format{}; > - > - for (unsigned int code : mbusCodes) { > - if (std::any_of(mbusCodes_.begin(), mbusCodes_.end(), > - [code](unsigned int c) { return c == code; })) { > - format.mbus_code = code; > - break; > - } > - } > - > - if (!format.mbus_code) { > - LOG(CameraSensor, Debug) << "No supported format found"; > - return format; > - } > - > unsigned int desiredArea = size.width * size.height; > unsigned int bestArea = UINT_MAX; > float desiredRatio = static_cast<float>(size.width) / size.height; > float bestRatio = FLT_MAX; > const Size *bestSize = nullptr; > + uint32_t bestCode = 0; > > - for (const Size &sz : sizes_) { > - if (sz.width < size.width || sz.height < size.height) > - continue; > + for (unsigned int code : mbusCodes) { > + const std::vector<SizeRange> &ranges = formats_.sizes(code); > > - float ratio = static_cast<float>(sz.width) / sz.height; > - float ratioDiff = fabsf(ratio - desiredRatio); > - unsigned int area = sz.width * sz.height; > - unsigned int areaDiff = area - desiredArea; > + for (const SizeRange &range : ranges) { > + const Size &sz = range.max; > > - if (ratioDiff > bestRatio) > - continue; > + if (sz.width < size.width || sz.height < size.height) > + continue; > > - if (ratioDiff < bestRatio || areaDiff < bestArea) { > - bestRatio = ratioDiff; > - bestArea = areaDiff; > - bestSize = &sz; > + float ratio = static_cast<float>(sz.width) / sz.height; > + float ratioDiff = fabsf(ratio - desiredRatio); > + unsigned int area = sz.width * sz.height; > + unsigned int areaDiff = area - desiredArea; > + > + if (ratioDiff > bestRatio) > + continue; > + > + if (ratioDiff < bestRatio || areaDiff < bestArea) { > + bestRatio = ratioDiff; > + bestArea = areaDiff; > + bestSize = &sz; > + bestCode = code; > + } > } > } > > if (!bestSize) { > - LOG(CameraSensor, Debug) << "No supported size found"; > - return format; > + LOG(CameraSensor, Debug) << "No supported format or size found"; > + return {}; > } > > - format.size = *bestSize; > + V4L2SubdeviceFormat format{ > + .mbus_code = bestCode, > + .size = *bestSize, > + }; > > return format; > } > @@ -424,7 +419,7 @@ const ControlInfoMap &CameraSensor::controls() const > > /** > * \brief Read controls from the sensor > - * \param[in] ids The list of control to read, specified by their ID > + * \param[in] ids The list of controls to read, specified by their ID > * > * This method reads the value of all controls contained in \a ids, and returns > * their values as a ControlList. > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h > index 24993b74148f..30cf5f34f485 100644 > --- a/src/libcamera/include/camera_sensor.h > +++ b/src/libcamera/include/camera_sensor.h > @@ -14,6 +14,7 @@ > #include <libcamera/controls.h> > #include <libcamera/geometry.h> > > +#include "formats.h" > #include "log.h" > > namespace libcamera { > @@ -51,7 +52,7 @@ public: > const MediaEntity *entity() const { return entity_; } > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > const std::vector<Size> &sizes() const { return sizes_; } > - const Size &resolution() const; > + const Size &resolution() const { return resolution_; } > > V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes, > const Size &size) const; > @@ -74,6 +75,8 @@ private: > > std::string model_; > > + ImageFormats formats_; > + Size resolution_; > std::vector<unsigned int> mbusCodes_; > std::vector<Size> sizes_; >
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 4154700a19b5..abafb30eb977 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -119,8 +119,7 @@ LOG_DEFINE_CATEGORY(CameraSensor); * information. * * The implementation is currently limited to sensors that expose a single V4L2 - * subdevice with a single pad, and support the same frame sizes for all - * supported media bus codes. It will be extended to support more complex + * subdevice with a single pad. It will be extended to support more complex * devices as the needs arise. */ @@ -245,36 +244,34 @@ int CameraSensor::init() propertyValue = 0; properties_.set(properties::Rotation, propertyValue); - /* Enumerate and cache media bus codes and sizes. */ - const ImageFormats formats = subdev_->formats(pad_); - if (formats.isEmpty()) { + /* Enumerate, sort and cache media bus codes and sizes. */ + formats_ = subdev_->formats(pad_); + if (formats_.isEmpty()) { LOG(CameraSensor, Error) << "No image format found"; return -EINVAL; } - mbusCodes_ = formats.formats(); - - /* - * Extract the supported sizes from the first format as we only support - * sensors that offer the same frame sizes for all media bus codes. - * Verify this assumption and reject the sensor if it isn't true. - */ - const std::vector<SizeRange> &sizes = formats.sizes(mbusCodes_[0]); - std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_), - [](const SizeRange &range) { return range.max; }); - - for (unsigned int code : mbusCodes_) { - if (formats.sizes(code) != sizes) { - LOG(CameraSensor, Error) - << "Frame sizes differ between media bus codes"; - return -EINVAL; - } - } - - /* Sort the media bus codes and sizes. */ + mbusCodes_ = formats_.formats(); std::sort(mbusCodes_.begin(), mbusCodes_.end()); + + for (const auto format : formats_.data()) { + const std::vector<SizeRange> &ranges = format.second; + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_), + [](const SizeRange &range) { return range.max; }); + } + std::sort(sizes_.begin(), sizes_.end()); + /* Remove duplicates. */ + auto last = std::unique(sizes_.begin(), sizes_.end()); + sizes_.erase(last, sizes_.end()); + + /* + * The sizes_ vector is sorted in ascending order, the resolution is + * thus the last element of the vector. + */ + resolution_ = sizes_.back(); + return 0; } @@ -303,21 +300,18 @@ int CameraSensor::init() /** * \fn CameraSensor::sizes() * \brief Retrieve the frame sizes supported by the camera sensor + * + * The reported sizes span all media bus codes supported by the camera sensor. + * Not all sizes may be supported by all media bus codes. + * * \return The supported frame sizes sorted in increasing order */ /** + * \fn CameraSensor::resolution() * \brief Retrieve the camera sensor resolution * \return The camera sensor resolution in pixels */ -const Size &CameraSensor::resolution() const -{ - /* - * The sizes_ vector is sorted in ascending order, the resolution is - * thus the last element of the vector. - */ - return sizes_.back(); -} /** * \brief Retrieve the best sensor format for a desired output @@ -325,13 +319,13 @@ const Size &CameraSensor::resolution() const * \param[in] size The desired size * * Media bus codes are selected from \a mbusCodes, which lists all acceptable - * codes in decreasing order of preference. This method selects the first code - * from the list that is supported by the sensor. If none of the desired codes - * is supported, it returns an error. + * codes in decreasing order of preference. Media bus codes supported by the + * sensor but not listed in \a mbusCodes are ignored. If none of the desired + * codes is supported, it returns an error. * * \a size indicates the desired size at the output of the sensor. This method - * selects the best size supported by the sensor according to the following - * criteria. + * selects the best media bus code and size supported by the sensor according + * to the following criteria. * * - The desired \a size shall fit in the sensor output size to avoid the need * to up-scale. @@ -339,6 +333,11 @@ const Size &CameraSensor::resolution() const * need to crop the field of view. * - The sensor output size shall be as small as possible to lower the required * bandwidth. + * - The desired \a size shall be supported by one of the media bus code listed + * in \a mbusCodes. + * + * When multiple media bus codes can produce the same size, the code at the + * lowest position in \a mbusCodes is selected. * * The use of this method is optional, as the above criteria may not match the * needs of all pipeline handlers. Pipeline handlers may implement custom @@ -353,52 +352,48 @@ const Size &CameraSensor::resolution() const V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbusCodes, const Size &size) const { - V4L2SubdeviceFormat format{}; - - for (unsigned int code : mbusCodes) { - if (std::any_of(mbusCodes_.begin(), mbusCodes_.end(), - [code](unsigned int c) { return c == code; })) { - format.mbus_code = code; - break; - } - } - - if (!format.mbus_code) { - LOG(CameraSensor, Debug) << "No supported format found"; - return format; - } - unsigned int desiredArea = size.width * size.height; unsigned int bestArea = UINT_MAX; float desiredRatio = static_cast<float>(size.width) / size.height; float bestRatio = FLT_MAX; const Size *bestSize = nullptr; + uint32_t bestCode = 0; - for (const Size &sz : sizes_) { - if (sz.width < size.width || sz.height < size.height) - continue; + for (unsigned int code : mbusCodes) { + const std::vector<SizeRange> &ranges = formats_.sizes(code); - float ratio = static_cast<float>(sz.width) / sz.height; - float ratioDiff = fabsf(ratio - desiredRatio); - unsigned int area = sz.width * sz.height; - unsigned int areaDiff = area - desiredArea; + for (const SizeRange &range : ranges) { + const Size &sz = range.max; - if (ratioDiff > bestRatio) - continue; + if (sz.width < size.width || sz.height < size.height) + continue; - if (ratioDiff < bestRatio || areaDiff < bestArea) { - bestRatio = ratioDiff; - bestArea = areaDiff; - bestSize = &sz; + float ratio = static_cast<float>(sz.width) / sz.height; + float ratioDiff = fabsf(ratio - desiredRatio); + unsigned int area = sz.width * sz.height; + unsigned int areaDiff = area - desiredArea; + + if (ratioDiff > bestRatio) + continue; + + if (ratioDiff < bestRatio || areaDiff < bestArea) { + bestRatio = ratioDiff; + bestArea = areaDiff; + bestSize = &sz; + bestCode = code; + } } } if (!bestSize) { - LOG(CameraSensor, Debug) << "No supported size found"; - return format; + LOG(CameraSensor, Debug) << "No supported format or size found"; + return {}; } - format.size = *bestSize; + V4L2SubdeviceFormat format{ + .mbus_code = bestCode, + .size = *bestSize, + }; return format; } @@ -424,7 +419,7 @@ const ControlInfoMap &CameraSensor::controls() const /** * \brief Read controls from the sensor - * \param[in] ids The list of control to read, specified by their ID + * \param[in] ids The list of controls to read, specified by their ID * * This method reads the value of all controls contained in \a ids, and returns * their values as a ControlList. diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h index 24993b74148f..30cf5f34f485 100644 --- a/src/libcamera/include/camera_sensor.h +++ b/src/libcamera/include/camera_sensor.h @@ -14,6 +14,7 @@ #include <libcamera/controls.h> #include <libcamera/geometry.h> +#include "formats.h" #include "log.h" namespace libcamera { @@ -51,7 +52,7 @@ public: const MediaEntity *entity() const { return entity_; } const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } const std::vector<Size> &sizes() const { return sizes_; } - const Size &resolution() const; + const Size &resolution() const { return resolution_; } V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes, const Size &size) const; @@ -74,6 +75,8 @@ private: std::string model_; + ImageFormats formats_; + Size resolution_; std::vector<unsigned int> mbusCodes_; std::vector<Size> sizes_;
The CameraSensor class assumes that camera sensors support the exact same list of sizes of all media bus codes. While allowing a simpler API, this assumption is incorrect and is blocking usage of some camera sensors. Relaxing the constraint is possible without changes to the CameraSensor API syntax, but requires changing its semantics. The sizes() function now returns the list of all sizes for all media bus codes, and the getFormat() function now searches in all supported media bus codes. The former is likely not the most useful option for pipeline handlers, but the sizes() function is currently unused. Designing a better API will require inspecting current and expected future use cases in pipeline handlers to determine proper heuristics. While at it, fix a small typo in an unrelated comment. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/camera_sensor.cpp | 135 +++++++++++++------------- src/libcamera/include/camera_sensor.h | 5 +- 2 files changed, 69 insertions(+), 71 deletions(-)