[libcamera-devel] [PATCH v2 03/11] libcamera: media_device: Only read device information in populate()

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, May 08, 2019 at 05:18:23PM +0200, Niklas Söderlund wrote:
> There is no reason to reread the MEDIA_IOC_DEVICE_INFO information every
> time the media device is opened. Move it populate() where it will be
> read once together the other information about the media device.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

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

> ---
>  src/libcamera/media_device.cpp | 51 +++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 4b3b8f1fa3e6aaad..416a0d0c207ea72d 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -126,18 +126,11 @@ bool MediaDevice::acquire()
>   */
>  
>  /**
> - * \brief Open a media device and retrieve device information
> - *
> - * Before populating the media graph or performing any operation that interact
> - * with the device node associated with the media device, the device node must
> - * be opened.
> - *
> - * This function also retrieves media device information from the device node,
> - * which can be queried through driver().
> - *
> - * If the device is already open the function returns -EBUSY.
> + * \brief Open the media device
>   *
>   * \return 0 on success or a negative error code otherwise
> + * \retval -EBUSY Media device already open
> + * \sa close()
>   */
>  int MediaDevice::open()
>  {
> @@ -156,20 +149,6 @@ int MediaDevice::open()
>  	}
>  	fd_ = ret;
>  
> -	struct media_device_info info = { };
> -	ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info);
> -	if (ret) {
> -		ret = -errno;
> -		LOG(MediaDevice, Error)
> -			<< "Failed to get media device info "
> -			<< ": " << strerror(-ret);
> -		return ret;
> -	}
> -
> -	driver_ = info.driver;
> -	model_ = info.model;
> -	version_ = info.media_version;
> -
>  	return 0;
>  }
>  
> @@ -196,12 +175,13 @@ void MediaDevice::close()
>  }
>  
>  /**
> - * \brief Populate the media graph with media objects
> + * \brief Populate the MediaDevice with device information and media objects
>   *
> - * This function enumerates all media objects in the media device graph and
> - * creates their MediaObject representations. All entities, pads and links are
> - * stored as MediaEntity, MediaPad and MediaLink respectively, with cross-
> - * references between objects. Interfaces are not processed.
> + * This function retrieves the media device information and enumerates all
> + * media objects in the media device graph and creates their MediaObject
> + * representations. All entities, pads and links are stored as MediaEntity,
> + * MediaPad and MediaLink respectively, with cross-references between objects.
> + * Interfaces are not processed.
>   *
>   * Entities are stored in a separate list in the MediaDevice to ease lookup,
>   * while pads are accessible from the entity they belong to and links from the
> @@ -225,6 +205,19 @@ int MediaDevice::populate()
>  	if (ret)
>  		return ret;
>  
> +	struct media_device_info info = {};
> +	ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info);
> +	if (ret) {
> +		ret = -errno;
> +		LOG(MediaDevice, Error)
> +			<< "Failed to get media device info " << strerror(-ret);
> +		goto done;
> +	}
> +
> +	driver_ = info.driver;
> +	model_ = info.model;
> +	version_ = info.media_version;
> +
>  	/*
>  	 * Keep calling G_TOPOLOGY until the version number stays stable.
>  	 */
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,

Laurent Pinchart