[libcamera-devel] [PATCH v2 6/6] libcamera: v4l2_videodevice: Use ImageFormats
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Jun 26 03:49:08 CEST 2020
Hi Jacopo,
Thank you for the patch.
On Tue, Jun 09, 2020 at 01:28:44AM +0200, Jacopo Mondi wrote:
> ImageFormats was meant to be used not only for V4L2Subdevice but
> for video devices as well. Since the introduction of of V4L2PixelFormat
s/of of/of/
> the V4L2VideoDevice class and its users have been using a raw map to
> enumerate and inspect the V4L2VideoDevice formats.
>
> Provide a V4L2VideoDevice::Formats type definition as for V4L2Subdevice
> and use ImageFormats wherever possible in pipeline handlers.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/v4l2_videodevice.h | 4 +++-
> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++++++--------
> src/libcamera/pipeline/simple/simple.cpp | 3 +--
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +--
> src/libcamera/v4l2_videodevice.cpp | 14 ++++++++++----
> 5 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 4d21f5a01ec8..06c65e863a99 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -168,6 +168,8 @@ public:
> class V4L2VideoDevice : public V4L2Device
> {
> public:
> + using Formats = ImageFormats<V4L2PixelFormat>;
> +
> explicit V4L2VideoDevice(const std::string &deviceNode);
> explicit V4L2VideoDevice(const MediaEntity *entity);
> V4L2VideoDevice(const V4L2VideoDevice &) = delete;
> @@ -187,7 +189,7 @@ public:
>
> int getFormat(V4L2DeviceFormat *format);
> int setFormat(V4L2DeviceFormat *format);
> - std::map<V4L2PixelFormat, std::vector<SizeRange>> formats(uint32_t code = 0);
> + Formats formats(uint32_t code = 0);
>
> int setSelection(unsigned int target, Rectangle *rect);
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index b9b88506c646..861d0864f9f7 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -37,8 +37,6 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(RPI)
>
> -using V4L2PixFmtMap = std::map<V4L2PixelFormat, std::vector<SizeRange>>;
> -
> namespace {
>
> bool isRaw(PixelFormat &pixFmt)
> @@ -67,7 +65,7 @@ double scoreFormat(double desired, double actual)
> return score;
> }
>
> -V4L2DeviceFormat findBestMode(V4L2PixFmtMap &formatsMap, const Size &req)
> +V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap, const Size &req)
> {
> double bestScore = 9e9, score;
> V4L2DeviceFormat bestMode = {};
> @@ -427,7 +425,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> * Calculate the best sensor mode we can use based on
> * the user request.
> */
> - V4L2PixFmtMap fmts = data_->unicam_[Unicam::Image].dev()->formats();
> + V4L2VideoDevice::Formats fmts = data_->unicam_[Unicam::Image].dev()->formats();
> V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
> PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
> if (cfg.size != sensorFormat.size ||
> @@ -481,14 +479,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> *
> */
> PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> - V4L2PixFmtMap fmts;
> + V4L2VideoDevice::Formats fmts;
>
> if (i == maxIndex)
> fmts = data_->isp_[Isp::Output0].dev()->formats();
> else
> fmts = data_->isp_[Isp::Output1].dev()->formats();
>
> - if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt, false)) == fmts.end()) {
> + if (!fmts.contains(V4L2PixelFormat::fromPixelFormat(cfgPixFmt, false))) {
> /* If we cannot find a native format, use a default one. */
> cfgPixFmt = PixelFormat(DRM_FORMAT_NV12);
> status = Adjusted;
> @@ -518,7 +516,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> RPiCameraData *data = cameraData(camera);
> CameraConfiguration *config = new RPiCameraConfiguration(data);
> V4L2DeviceFormat sensorFormat;
> - V4L2PixFmtMap fmts;
> + V4L2VideoDevice::Formats fmts;
>
> if (roles.empty())
> return config;
> @@ -605,7 +603,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> }
>
> /* First calculate the best sensor mode we can use based on the user request. */
> - V4L2PixFmtMap fmts = data->unicam_[Unicam::Image].dev()->formats();
> + V4L2VideoDevice::Formats fmts = data->unicam_[Unicam::Image].dev()->formats();
> V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);
>
> /*
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 1ec8d0f7de03..59c4a6da1ee9 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -275,8 +275,7 @@ int SimpleCameraData::init()
> return ret;
> }
>
> - std::map<V4L2PixelFormat, std::vector<SizeRange>> videoFormats =
> - video_->formats(format.mbus_code);
> + V4L2VideoDevice::Formats videoFormats = video_->formats(format.mbus_code);
>
> LOG(SimplePipeline, Debug)
> << "Adding configuration for " << format.size.toString()
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index a074909499f1..9e41cbd50c1a 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -159,8 +159,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
> if (roles.empty())
> return config;
>
> - std::map<V4L2PixelFormat, std::vector<SizeRange>> v4l2Formats =
> - data->video_->formats();
> + V4L2VideoDevice::Formats v4l2Formats = data->video_->formats();
> std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
> std::transform(v4l2Formats.begin(), v4l2Formats.end(),
> std::inserter(deviceFormats, deviceFormats.begin()),
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 3614b2ed1cbc..077e150ceda6 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -461,6 +461,12 @@ const std::string V4L2DeviceFormat::toString() const
> * \context This class is \threadbound.
> */
>
> +/**
> + * \typedef V4L2VideoDevice::Formats
> + * \brief Enumeration of V4L2PixelFormat instances associated to image
> + * resolutions
Same comment as for the V4L2Subdevice variant.
> + */
> +
> /**
> * \brief Construct a V4L2VideoDevice
> * \param[in] deviceNode The file-system path to the video device node
> @@ -925,23 +931,23 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
> *
> * \return A list of the supported video device formats
> */
> -std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats(uint32_t code)
> +V4L2VideoDevice::Formats V4L2VideoDevice::formats(uint32_t code)
> {
> - std::map<V4L2PixelFormat, std::vector<SizeRange>> formats;
> + Formats formats;
>
> for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
> std::vector<SizeRange> sizes = enumSizes(pixelFormat);
> if (sizes.empty())
> return {};
>
> - if (formats.find(pixelFormat) != formats.end()) {
> + if (!formats.contains(pixelFormat)) {
This should for
if (formats.contains(pixelFormat)) {
The patch otherwise looks good to me, but I'll wait until we finish
discussing patch 3/6 before acking it. In the meantime, could you test
the series on RPi, as I believe the bug in the implementation of
contains() should have affected it ?
> LOG(V4L2, Error)
> << "Could not add sizes for pixel format "
> << pixelFormat;
> return {};
> }
>
> - formats.emplace(pixelFormat, sizes);
> + formats.addFormat(pixelFormat, sizes);
> }
>
> return formats;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list