[libcamera-devel] [PATCH v2 2/6] libcamera: v4l2_subdevice: Provide Formats typedef

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

Maybe s/typedef/type definition/ (or type alias) in the subject, as it's
not a C typedef ?

On Tue, Jun 09, 2020 at 01:28:40AM +0200, Jacopo Mondi wrote:
> Provide a type definition for ImageFormats<uint32_t> to allow
> users of V4L2Subdevice to refer to it as V4L2Subdevice::Formats.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/internal/camera_sensor.h  |  6 ++----
>  include/libcamera/internal/v4l2_subdevice.h |  4 +++-
>  src/libcamera/v4l2_subdevice.cpp            | 10 ++++++++--
>  test/v4l2_subdevice/list_formats.cpp        |  2 +-
>  4 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d5814a26a121..06c8292ca301 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_;
>  
> -	ImageFormats<uint32_t> formats_;
> +	V4L2Subdevice::Formats 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 f811d316dada..c678fc87e80f 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 Formats = ImageFormats<uint32_t>;
> +
>  	enum Whence {
>  		ActiveFormat,
>  		TryFormat,
> @@ -51,7 +53,7 @@ public:
>  	int setSelection(unsigned int pad, unsigned int target,
>  			 Rectangle *rect);
>  
> -	ImageFormats<uint32_t> formats(unsigned int pad);
> +	Formats formats(unsigned int pad);
>  
>  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  		      Whence whence = ActiveFormat);
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 9fa20e84a904..de7d0c0db2b6 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -197,6 +197,12 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
>   * any device left open will be closed, and any resources released.
>   */
>  
> +/**
> + * \typedef V4L2Subdevice::Formats
> + * \brief Enumeration of uint32_t media bus codes associated to image
> + * resolutions

Should this mention ImageFormats ? Maybe

 * \brief An ImageFormats specialization mapping uint32_t media bus codes to
 * image resolutions

?

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + */
> +
>  /**
>   * \enum V4L2Subdevice::Whence
>   * \brief Specify the type of format for getFormat() and setFormat() operations
> @@ -320,9 +326,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
>   *
>   * \return A list of the supported device formats
>   */
> -ImageFormats<uint32_t> V4L2Subdevice::formats(unsigned int pad)
> +V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
>  {
> -	ImageFormats<uint32_t> formats;
> +	Formats formats;
>  
>  	if (pad >= entity_->pads().size()) {
>  		LOG(V4L2, Error) << "Invalid pad: " << pad;
> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> index 3e8d4cdba045..7f0b3333f617 100644
> --- a/test/v4l2_subdevice/list_formats.cpp
> +++ b/test/v4l2_subdevice/list_formats.cpp
> @@ -47,7 +47,7 @@ void ListFormatsTest::printFormats(unsigned int pad,
>  int ListFormatsTest::run()
>  {
>  	/* List all formats available on existing "Scaler" pads. */
> -	ImageFormats<uint32_t> formats;
> +	V4L2Subdevice::Formats formats;
>  
>  	formats = scaler_->formats(0);
>  	if (formats.isEmpty()) {

-- 
Regards,

Laurent Pinchart