[PATCH 2/2] libcamera: Add V4L2VideoDevice::getCachedFormat

[Kieran Bingham]

Quoting Harvey Yang (2024-10-09 20:58:51)
> In the upcoming mtkisp7 pipeline handler, it needs to access the cached
> format from a derived class. This patch adds a protected getter to allow
> that.

"A derived class" sounds ... well - what is deriving from a
V4L2VideoDevice. We normally use encapsulation for these classes, not
derivation... ?

Are you making some sort of specialisation of a V4L2VideoDevice that
somehow doesn't use the standard V4L2 API? or something else that makes
it 'not a V4L2VideoDevice' ?


> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>  include/libcamera/internal/v4l2_videodevice.h | 2 ++
>  src/libcamera/v4l2_videodevice.cpp            | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 9f53e37cd..389276302 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -241,6 +241,8 @@ public:
>  protected:
>         std::string logPrefix() const override;
>  
> +       V4L2DeviceFormat getCachedFormat() { return format_; }

We don't normally call accessors 'cached'. Is this specifically cached
or would it be wrong to just name this getFormat()? Would that imply
that the function should call down to the device and get the format
perhaps ??

In fact, I don't think we usually prefix 'get' either so it could just
be
	V4L2DeviceFormat format() { return format_; }



> +
>  private:
>         LIBCAMERA_DISABLE_COPY(V4L2VideoDevice)
>  
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 1110fb535..1f6ad96c1 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -2113,6 +2113,11 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,
>         return std::make_unique<V4L2VideoDevice>(mediaEntity);
>  }
>  
> +/**
> + * \fn V4L2VideoDevice::getCachedFormat()
> + * \return Cached \a V4L2VideoDevice::format_

And then I would state that this returns the format or perhaps returns
the current format;

Which could at least be expanded on, or clarified in the \brief

> + */
> +
>  /**
>   * \brief Convert \a PixelFormat to a V4L2PixelFormat supported by the device
>   * \param[in] pixelFormat The PixelFormat to convert
> -- 
> 2.47.0.rc0.187.ge670bccf7e-goog
>