[libcamera-devel] [PATCH] libcamera: v4l2_device: Construct from MediaEntity*

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Feb 04, 2019 at 04:56:05PM +0100, Jacopo Mondi wrote:
> The V4L2Device constructor wanted a "const MediaEntity &", while
> entities are usually retrieved by pointer before being used to construct
> a V4L2Device. Change the constructor argument to "MediaEntity *".

A non-const pointer hints that the function could keep a reference,
modify the object instance and/or accept a NULL parameter. None of this
is true in this case. Do we really need to pass by pointer ? Shouldn't
it then be at least a const pointer ?

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/v4l2_device.h  | 2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
>  src/libcamera/pipeline/uvcvideo.cpp  | 2 +-
>  src/libcamera/v4l2_device.cpp        | 4 ++--
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 4ff48fb..1067e45 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -72,7 +72,7 @@ class V4L2Device
>  {
>  public:
>  	explicit V4L2Device(const std::string &deviceNode);
> -	explicit V4L2Device(const MediaEntity &entity);
> +	explicit V4L2Device(MediaEntity *entity);
>  	V4L2Device(const V4L2Device &) = delete;
>  	~V4L2Device();
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 7823bbb..fa3c522 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -182,7 +182,7 @@ V4L2Device *PipelineHandlerIPU3::createVideoDevice(unsigned int id)
>  	if (!cio2)
>  		return nullptr;
> 
> -	V4L2Device *dev = new V4L2Device(*cio2);
> +	V4L2Device *dev = new V4L2Device(cio2);
>  	if (dev->open()) {
>  		delete dev;
>  		return nullptr;
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 821e4c2..e31816b 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -91,7 +91,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> 
>  	for (MediaEntity *entity : media_->entities()) {
>  		if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> -			video_ = new V4L2Device(*entity);
> +			video_ = new V4L2Device(entity);
>  			break;
>  		}
>  	}
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 4d1f76b..61db7f6 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -169,8 +169,8 @@ V4L2Device::V4L2Device(const std::string &deviceNode)
>   *
>   * Construct a V4L2Device from a MediaEntity's device node path.
>   */
> -V4L2Device::V4L2Device(const MediaEntity &entity)
> -	: V4L2Device(entity.deviceNode())
> +V4L2Device::V4L2Device(MediaEntity *entity)
> +	: V4L2Device(entity->deviceNode())
>  {
>  }
> 

-- 
Regards,

Laurent Pinchart