[libcamera-devel] [PATCH 4/5] libcamera: v4l2_device: Add formatsMap typedef
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jun 8 15:08:39 CEST 2020
Hi Jacopo,
On Mon, Jun 08, 2020 at 10:02:58AM +0200, Jacopo Mondi wrote:
> On Sat, Jun 06, 2020 at 02:31:39AM +0300, Laurent Pinchart wrote:
> > On Fri, May 29, 2020 at 01:03:34PM +0200, Jacopo Mondi wrote:
> > > Provide a type definition in the V4L2VideoDevice and V4L2Subdevice
> > > classes to shorten the ImageFormatsMap<V4L2PixelFormat> and
> > > ImageFormatsMap<uint32_t> spcialization names.
> > >
> > > The new types reads as V4L2VideoDevice::formatsMap and
> > > V4L2Subdevice::formatsMap respectively.
> >
> > That should be FormatsMap (or actually even Formats).
>
> I'm still not satisfied with any name we tried for this construct. But
> shorting it down to ::Format is better.
Do you mean Formats ?
> > I would split this in two, with the V4L2VideoDevice part squashed with
> > 2/5, as otherwise you essentially update the same code twice, oncee to
> > move from an explicit std::map to ImageFormats<V4L2PixelFormat>, and a
> > second time to move from ImageFormats<V4L2PixelFormat> to
> > V4L2VideoDevice::FormatsMap. The V4L2Subdevice part could then be moved
> > after patch 1/5 and before patch 2/5.
>
> This was intentional to ease review, but I can squash them and provide
> typenames when moving users of V4L2VideoDevice from std::map to
> ImageFormat
I think that will actually ease review for me :-) The
V4L2Subdevice::Formats should of course still be introduced in a
separate patch.
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > > include/libcamera/internal/camera_sensor.h | 6 ++----
> > > include/libcamera/internal/v4l2_subdevice.h | 4 +++-
> > > include/libcamera/internal/v4l2_videodevice.h | 4 +++-
> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 ++++++-----
> > > src/libcamera/pipeline/simple/simple.cpp | 2 +-
> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-
> > > src/libcamera/v4l2_subdevice.cpp | 9 +++++++--
> > > src/libcamera/v4l2_videodevice.cpp | 9 +++++++--
> > > test/v4l2_subdevice/list_formats.cpp | 2 +-
> > > 9 files changed, 31 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > > index 52eedb9ef880..1353edb1da28 100644
> > > --- a/include/libcamera/internal/camera_sensor.h
> > > +++ b/include/libcamera/internal/camera_sensor.h
> > > @@ -16,13 +16,11 @@
> > >
> > > #include "libcamera/internal/formats.h"
> > > #include "libcamera/internal/log.h"
> > > +#include "libcamera/internal/v4l2_subdevice.h"
> > >
> > > namespace libcamera {
> > >
> > > class MediaEntity;
> > > -class V4L2Subdevice;
> > > -
> > > -struct V4L2SubdeviceFormat;
> > >
> > > struct CameraSensorInfo {
> > > std::string model;
> > > @@ -75,7 +73,7 @@ private:
> > >
> > > std::string model_;
> > >
> > > - ImageFormatsMap<uint32_t> formats_;
> > > + V4L2Subdevice::formatsMap formats_;
> > > Size resolution_;
> > > std::vector<unsigned int> mbusCodes_;
> > > std::vector<Size> sizes_;
> > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > > index c9aa3428f93c..06f10d7b4c5d 100644
> > > --- a/include/libcamera/internal/v4l2_subdevice.h
> > > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > > @@ -32,6 +32,8 @@ struct V4L2SubdeviceFormat {
> > > class V4L2Subdevice : public V4L2Device
> > > {
> > > public:
> > > + using formatsMap = ImageFormatsMap<uint32_t>;
> > > +
> > > enum Whence {
> > > ActiveFormat,
> > > TryFormat,
> > > @@ -51,7 +53,7 @@ public:
> > > int setSelection(unsigned int pad, unsigned int target,
> > > Rectangle *rect);
> > >
> > > - ImageFormatsMap<uint32_t> formats(unsigned int pad);
> > > + formatsMap formats(unsigned int pad);
> > >
> > > int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > > Whence whence = ActiveFormat);
> > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > > index 02d101db0b2b..9632ad71b988 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 formatsMap = ImageFormatsMap<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);
> > > - ImageFormatsMap<V4L2PixelFormat> formats(uint32_t code = 0);
> > > + formatsMap 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 e0132113b072..7dd579090ec6 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -65,7 +65,7 @@ double scoreFormat(double desired, double actual)
> > > return score;
> > > }
> > >
> > > -V4L2DeviceFormat findBestMode(ImageFormatsMap<V4L2PixelFormat> &formatsMap, const Size &req)
> > > +V4L2DeviceFormat findBestMode(V4L2VideoDevice::formatsMap &formatsMap, const Size &req)
> > > {
> > > double bestScore = 9e9, score;
> > > V4L2DeviceFormat bestMode = {};
> > > @@ -424,7 +424,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > > * Calculate the best sensor mode we can use based on
> > > * the user request.
> > > */
> > > - ImageFormatsMap<V4L2PixelFormat> fmts = data_->unicam_[Unicam::Image].dev()->formats();
> > > + V4L2VideoDevice::formatsMap fmts =
> > > + data_->unicam_[Unicam::Image].dev()->formats();
> > > V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
> > > PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
> > > if (cfg.size != sensorFormat.size ||
> > > @@ -478,7 +479,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > > *
> > > */
> > > PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> > > - ImageFormatsMap<V4L2PixelFormat> fmts;
> > > + V4L2VideoDevice::formatsMap fmts;
> > >
> > > if (i == maxIndex)
> > > fmts = data_->isp_[Isp::Output0].dev()->formats();
> > > @@ -515,7 +516,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > > RPiCameraData *data = cameraData(camera);
> > > CameraConfiguration *config = new RPiCameraConfiguration(data);
> > > V4L2DeviceFormat sensorFormat;
> > > - ImageFormatsMap<V4L2PixelFormat> fmts;
> > > + V4L2VideoDevice::formatsMap fmts;
> > >
> > > if (roles.empty())
> > > return config;
> > > @@ -602,7 +603,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > > }
> > >
> > > /* First calculate the best sensor mode we can use based on the user request. */
> > > - ImageFormatsMap<V4L2PixelFormat> fmts = data->unicam_[Unicam::Image].dev()->formats();
> > > + V4L2VideoDevice::formatsMap 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 62906b560d6a..feaa2854f037 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -275,7 +275,7 @@ int SimpleCameraData::init()
> > > return ret;
> > > }
> > >
> > > - ImageFormatsMap<V4L2PixelFormat> videoFormats = video_->formats(format.mbus_code);
> > > + V4L2VideoDevice::formatsMap 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 e1b1ae32b821..6250186e9390 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -159,7 +159,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
> > > if (roles.empty())
> > > return config;
> > >
> > > - ImageFormatsMap<V4L2PixelFormat> v4l2Formats = data->video_->formats();
> > > + V4L2VideoDevice::formatsMap 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_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > > index 80db4cda82bb..0dd8e8686967 100644
> > > --- a/src/libcamera/v4l2_subdevice.cpp
> > > +++ b/src/libcamera/v4l2_subdevice.cpp
> > > @@ -197,6 +197,11 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
> > > * any device left open will be closed, and any resources released.
> > > */
> > >
> > > +/**
> > > + * \typedef V4L2Subdevice::formatsMap
> > > + * \brief Map of media bus codes to associated image sizes
> > > + */
> > > +
> > > /**
> > > * \enum V4L2Subdevice::Whence
> > > * \brief Specify the type of format for getFormat() and setFormat() operations
> > > @@ -320,9 +325,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > > *
> > > * \return A list of the supported device formats
> > > */
> > > -ImageFormatsMap<uint32_t> V4L2Subdevice::formats(unsigned int pad)
> > > +V4L2Subdevice::formatsMap V4L2Subdevice::formats(unsigned int pad)
> > > {
> > > - ImageFormatsMap<uint32_t> formats;
> > > + formatsMap formats;
> > >
> > > if (pad >= entity_->pads().size()) {
> > > LOG(V4L2, Error) << "Invalid pad: " << pad;
> > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > > index b264915cb73a..6d48018396c3 100644
> > > --- a/src/libcamera/v4l2_videodevice.cpp
> > > +++ b/src/libcamera/v4l2_videodevice.cpp
> > > @@ -461,6 +461,11 @@ const std::string V4L2DeviceFormat::toString() const
> > > * \context This class is \threadbound.
> > > */
> > >
> > > +/**
> > > + * \typedef V4L2VideoDevice::formatsMap
> > > + * \brief Map of V4l2PixelFormat to associated image sizes
> > > + */
> > > +
> > > /**
> > > * \brief Construct a V4L2VideoDevice
> > > * \param[in] deviceNode The file-system path to the video device node
> > > @@ -925,9 +930,9 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
> > > *
> > > * \return A list of the supported video device formats
> > > */
> > > -ImageFormatsMap<V4L2PixelFormat> V4L2VideoDevice::formats(uint32_t code)
> > > +V4L2VideoDevice::formatsMap V4L2VideoDevice::formats(uint32_t code)
> > > {
> > > - ImageFormatsMap<V4L2PixelFormat> formats;
> > > + formatsMap formats;
> > >
> > > for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
> > > std::vector<SizeRange> sizes = enumSizes(pixelFormat);
> > > diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> > > index adcf5606c961..a32f5c833c28 100644
> > > --- a/test/v4l2_subdevice/list_formats.cpp
> > > +++ b/test/v4l2_subdevice/list_formats.cpp
> > > @@ -48,7 +48,7 @@ void ListFormatsTest::printFormats(unsigned int pad,
> > > int ListFormatsTest::run()
> > > {
> > > /* List all formats available on existing "Scaler" pads. */
> > > - ImageFormatsMap<uint32_t> formats;
> > > + V4L2Subdevice::formatsMap formats;
> > >
> > > formats = scaler_->formats(0);
> > > if (formats.isEmpty()) {
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list