[libcamera-devel] [PATCH v3 01/10] libcamera: camera_sensor: Make mbusCodes() return a set
Jacopo Mondi
jacopo at jmondi.org
Wed Jun 24 11:18:10 CEST 2020
Hi Niklas,
On Tue, Jun 23, 2020 at 04:09:21AM +0200, Niklas Söderlund wrote:
> It makes little sens for a CameraSensor to support the same media bus
sense
s/a CameraSensor/the CameraSensor class/
> format multiple times, change the container from a vector<> to a set<>.
>
> This was discovered while working with the IPU3 pipeline handler that
> Unnecessarily had to carry code to sort vectors in order to find
s/Un/un/
> intersections. As this change requires switching the container to set<>
> in that pipeline handler as well fix this while we are at it.
as well,
Which sensor does report mulitple times the same mbus_code ? Isn't
this a driver bug if it happens ?
I see V4L2Subdevice::formats() populating the image formats with the
mbus codes returned by V4L2Subdevice::enumPadCodes() and when I print
out on Soraka the codes there collected I only get MEDIA_BUS_FMT_SGRBG10_1X10.
Looking at the drivers of the two there installed sensors confirms
that only one format is supported..
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/internal/camera_sensor.h | 4 ++--
> include/libcamera/internal/formats.h | 2 +-
> src/libcamera/camera_sensor.cpp | 1 -
> src/libcamera/formats.cpp | 7 +++----
> src/libcamera/pipeline/ipu3/ipu3.cpp | 13 +++++--------
> test/camera-sensor.cpp | 2 +-
> 6 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 7f07413f95602881..6d92818f16e4b752 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -50,7 +50,7 @@ public:
>
> const std::string &model() const { return model_; }
> const MediaEntity *entity() const { return entity_; }
> - const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
> + const std::set<unsigned int> &mbusCodes() const { return mbusCodes_; }
> const std::vector<Size> &sizes() const { return sizes_; }
> const Size &resolution() const { return resolution_; }
>
> @@ -77,7 +77,7 @@ private:
>
> ImageFormats formats_;
> Size resolution_;
> - std::vector<unsigned int> mbusCodes_;
> + std::set<unsigned int> mbusCodes_;
> std::vector<Size> sizes_;
>
> ControlList properties_;
> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> index 4b172efc6588d374..e8aa555dcbe99350 100644
> --- a/include/libcamera/internal/formats.h
> +++ b/include/libcamera/internal/formats.h
> @@ -24,7 +24,7 @@ public:
> int addFormat(unsigned int format, const std::vector<SizeRange> &sizes);
>
> bool isEmpty() const;
> - std::vector<unsigned int> formats() const;
> + std::set<unsigned int> formats() const;
> const std::vector<SizeRange> &sizes(unsigned int format) const;
> const std::map<unsigned int, std::vector<SizeRange>> &data() const;
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index b14b4051dca606b9..5ab50de660534f3f 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -251,7 +251,6 @@ int CameraSensor::init()
> }
>
> mbusCodes_ = formats_.formats();
> - std::sort(mbusCodes_.begin(), mbusCodes_.end());
>
> for (const auto &format : formats_.data()) {
> const std::vector<SizeRange> &ranges = format.second;
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 1272de29c802c539..6234fd98f3e2002c 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -68,14 +68,13 @@ bool ImageFormats::isEmpty() const
> * \brief Retrieve a list of all supported image formats
> * \return List of pixel formats or media bus codes
> */
> -std::vector<unsigned int> ImageFormats::formats() const
> +std::set<unsigned int> ImageFormats::formats() const
> {
> - std::vector<unsigned int> formats;
> - formats.reserve(data_.size());
> + std::set<unsigned int> formats;
>
> /* \todo: Should this be cached instead of computed each time? */
> for (auto const &it : data_)
> - formats.push_back(it.first);
> + formats.insert(it.first);
>
> return formats;
> }
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 5544288b14f739c0..1a59de0c58975b3c 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1441,15 +1441,12 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> /*
> * Make sure the sensor produces at least one format compatible with
> * the CIO2 requirements.
> - *
> - * utils::set_overlap requires the ranges to be sorted, keep the
> - * cio2Codes vector sorted in ascending order.
> */
> - const std::vector<unsigned int> cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10,
> - MEDIA_BUS_FMT_SGRBG10_1X10,
> - MEDIA_BUS_FMT_SGBRG10_1X10,
> - MEDIA_BUS_FMT_SRGGB10_1X10 };
> - const std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();
> + const std::set<unsigned int> cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10,
> + MEDIA_BUS_FMT_SGRBG10_1X10,
> + MEDIA_BUS_FMT_SGBRG10_1X10,
> + MEDIA_BUS_FMT_SRGGB10_1X10 };
> + const std::set<unsigned int> &sensorCodes = sensor_->mbusCodes();
> if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),
> cio2Codes.begin(), cio2Codes.end())) {
> LOG(IPU3, Error)
> diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
> index 8c7fd1d2d4445db3..146b1b7d6c294fdb 100644
> --- a/test/camera-sensor.cpp
> +++ b/test/camera-sensor.cpp
> @@ -67,7 +67,7 @@ protected:
> return TestFail;
> }
>
> - const std::vector<unsigned int> &codes = sensor_->mbusCodes();
> + const std::set<unsigned int> &codes = sensor_->mbusCodes();
> auto iter = std::find(codes.begin(), codes.end(),
> MEDIA_BUS_FMT_ARGB8888_1X32);
> if (iter == codes.end()) {
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list