[libcamera-devel] [PATCH 5/5] libcamera: v4l2_device: Rename formats() method
Jacopo Mondi
jacopo at jmondi.org
Mon Jun 8 10:04:57 CEST 2020
Hi Laurent,
On Sat, Jun 06, 2020 at 02:33:57AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, May 29, 2020 at 01:03:35PM +0200, Jacopo Mondi wrote:
> > Both the V4L2VideoDevice and V4L2Subdevice classes provide a formats()
> > methods which returns an ImageFormatMap specialization.
> >
> > The method name formats() collides with ImageFormatsMap::formats() and
> > makes accessing the map or the list of supported format identifiers
> > confusing.
> >
> > Rename the V4L2VideoDevice and V4L2Subdevice methods to imageFormats(),
> > to distinguish the two.
>
> This makes lines a bit longer, and I don't find the formats() name
> confusing, but I'm also not opposed to the new name. If you decide to
> keep this patch (maybe wait a bit to see if you get more review
> comments ?),
A bit longer, yes, but having to call .formats() on something you just
obtained by calling a different .formats() method it's really not nice
to me.
I'll keep this one at the end of the series and see how it is received
in v2.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > include/libcamera/internal/v4l2_subdevice.h | 2 +-
> > include/libcamera/internal/v4l2_videodevice.h | 2 +-
> > src/libcamera/camera_sensor.cpp | 2 +-
> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++-----
> > src/libcamera/pipeline/simple/simple.cpp | 2 +-
> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-
> > src/libcamera/v4l2_subdevice.cpp | 2 +-
> > src/libcamera/v4l2_videodevice.cpp | 2 +-
> > test/v4l2_subdevice/list_formats.cpp | 6 +++---
> > 9 files changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > index 06f10d7b4c5d..2218e90c1da6 100644
> > --- a/include/libcamera/internal/v4l2_subdevice.h
> > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > @@ -53,7 +53,7 @@ public:
> > int setSelection(unsigned int pad, unsigned int target,
> > Rectangle *rect);
> >
> > - formatsMap formats(unsigned int pad);
> > + formatsMap imageFormats(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 9632ad71b988..2d968797ee86 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -189,7 +189,7 @@ public:
> >
> > int getFormat(V4L2DeviceFormat *format);
> > int setFormat(V4L2DeviceFormat *format);
> > - formatsMap formats(uint32_t code = 0);
> > + formatsMap imageFormats(uint32_t code = 0);
> >
> > int setSelection(unsigned int target, Rectangle *rect);
> >
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index b14b4051dca6..8aafdfe76299 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -244,7 +244,7 @@ int CameraSensor::init()
> > properties_.set(properties::Rotation, propertyValue);
> >
> > /* Enumerate, sort and cache media bus codes and sizes. */
> > - formats_ = subdev_->formats(pad_);
> > + formats_ = subdev_->imageFormats(pad_);
> > if (formats_.isEmpty()) {
> > LOG(CameraSensor, Error) << "No image format found";
> > return -EINVAL;
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 7dd579090ec6..a407945819be 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -425,7 +425,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > * the user request.
> > */
> > V4L2VideoDevice::formatsMap fmts =
> > - data_->unicam_[Unicam::Image].dev()->formats();
> > + data_->unicam_[Unicam::Image].dev()->imageFormats();
> > V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
> > PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
> > if (cfg.size != sensorFormat.size ||
> > @@ -482,9 +482,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > V4L2VideoDevice::formatsMap fmts;
> >
> > if (i == maxIndex)
> > - fmts = data_->isp_[Isp::Output0].dev()->formats();
> > + fmts = data_->isp_[Isp::Output0].dev()->imageFormats();
> > else
> > - fmts = data_->isp_[Isp::Output1].dev()->formats();
> > + fmts = data_->isp_[Isp::Output1].dev()->imageFormats();
> >
> > if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt, false)) == fmts.end()) {
> > /* If we cannot find a native format, use a default one. */
> > @@ -527,7 +527,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > switch (role) {
> > case StreamRole::StillCaptureRaw:
> > cfg.size = data->sensor_->resolution();
> > - fmts = data->unicam_[Unicam::Image].dev()->formats();
> > + fmts = data->unicam_[Unicam::Image].dev()->imageFormats();
> > sensorFormat = findBestMode(fmts, cfg.size);
> > cfg.pixelFormat = sensorFormat.fourcc.toPixelFormat();
> > ASSERT(cfg.pixelFormat.isValid());
> > @@ -603,7 +603,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > }
> >
> > /* First calculate the best sensor mode we can use based on the user request. */
> > - V4L2VideoDevice::formatsMap fmts = data->unicam_[Unicam::Image].dev()->formats();
> > + V4L2VideoDevice::formatsMap fmts = data->unicam_[Unicam::Image].dev()->imageFormats();
> > V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);
> >
> > /*
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index feaa2854f037..6857ec75cb1f 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -275,7 +275,7 @@ int SimpleCameraData::init()
> > return ret;
> > }
> >
> > - V4L2VideoDevice::formatsMap videoFormats = video_->formats(format.mbus_code);
> > + V4L2VideoDevice::formatsMap videoFormats = video_->imageFormats(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 6250186e9390..39c8353a2adf 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;
> >
> > - V4L2VideoDevice::formatsMap v4l2Formats = data->video_->formats();
> > + V4L2VideoDevice::formatsMap v4l2Formats = data->video_->imageFormats();
> > 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 0dd8e8686967..24ca6d80b27e 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -325,7 +325,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > *
> > * \return A list of the supported device formats
> > */
> > -V4L2Subdevice::formatsMap V4L2Subdevice::formats(unsigned int pad)
> > +V4L2Subdevice::formatsMap V4L2Subdevice::imageFormats(unsigned int pad)
> > {
> > formatsMap formats;
> >
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 6d48018396c3..08cae92978d4 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -930,7 +930,7 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
> > *
> > * \return A list of the supported video device formats
> > */
> > -V4L2VideoDevice::formatsMap V4L2VideoDevice::formats(uint32_t code)
> > +V4L2VideoDevice::formatsMap V4L2VideoDevice::imageFormats(uint32_t code)
> > {
> > formatsMap formats;
> >
> > diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> > index a32f5c833c28..262362dee3c9 100644
> > --- a/test/v4l2_subdevice/list_formats.cpp
> > +++ b/test/v4l2_subdevice/list_formats.cpp
> > @@ -50,7 +50,7 @@ int ListFormatsTest::run()
> > /* List all formats available on existing "Scaler" pads. */
> > V4L2Subdevice::formatsMap formats;
> >
> > - formats = scaler_->formats(0);
> > + formats = scaler_->imageFormats(0);
> > if (formats.isEmpty()) {
> > cerr << "Failed to list formats on pad 0 of subdevice "
> > << scaler_->entity()->name() << endl;
> > @@ -59,7 +59,7 @@ int ListFormatsTest::run()
> > for (unsigned int code : formats.formats())
> > printFormats(0, code, formats.sizes(code));
> >
> > - formats = scaler_->formats(1);
> > + formats = scaler_->imageFormats(1);
> > if (formats.isEmpty()) {
> > cerr << "Failed to list formats on pad 1 of subdevice "
> > << scaler_->entity()->name() << endl;
> > @@ -69,7 +69,7 @@ int ListFormatsTest::run()
> > printFormats(1, code, formats.sizes(code));
> >
> > /* List format on a non-existing pad, format vector shall be empty. */
> > - formats = scaler_->formats(2);
> > + formats = scaler_->imageFormats(2);
> > if (!formats.isEmpty()) {
> > cerr << "Listing formats on non-existing pad 2 of subdevice "
> > << scaler_->entity()->name()
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list