[libcamera-devel] [PATCH v3 1/5] libcamera: converter: a few fixes to ConverterFactoryBase documentation

[Laurent Pinchart]

Hi Andrey,

Thank you for the patch.

On Thu, Sep 28, 2023 at 09:55:33PM +0300, Andrey Konovalov via libcamera-devel wrote:
> The description of ConverterFactoryBase::registerType() referred to
> a converter factory as "converter class" and "converter". Fix that.
> 
> Also make the descriptions of ConverterFactoryBase::compatibles() and
> ConverterFactoryBase::create() a bit more specific.
> 
> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
> ---
>  src/libcamera/converter.cpp | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp
> index fa0f1ec8..15701363 100644
> --- a/src/libcamera/converter.cpp
> +++ b/src/libcamera/converter.cpp
> @@ -199,16 +199,19 @@ ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initiali
>  
>  /**
>   * \fn ConverterFactoryBase::compatibles()
> - * \return The names compatibles
> + * \return The list of compatible name aliases of the converter
>   */
>  
>  /**
> - * \brief Create an instance of the converter corresponding to a named factory
> - * \param[in] media Name of the factory
> + * \brief Create an instance of the converter corresponding to the media device
> + * \param[in] media The media device to create the converter for
>   *
>   * \return A unique pointer to a new instance of the converter subclass

While at it, I'd drop the "A unique pointer to", that's implicit by
usage of unique_ptr.

> - * corresponding to the named factory or one of its alias. Otherwise a null
> - * pointer if no such factory exists
> + * corresponding to the media device. The converter is created by matching
> + * the factory name or any of its compatible aliases with the media device
> + * driver name.

I'd move this sentence to the description of the function.

> + * If the media device driver name doesn't match anything a null pointer is
> + * returned.

 * \brief Create an instance of the converter corresponding to the media device
 * \param[in] media The media device to create the converter for
 *
 * The converter is created by matching the factory name or any of its
 * compatible aliases with the media device driver name.
 *
 * \return A new instance of the converter subclass corresponding to the media
 * device, or null if the media device driver name doesn't match anything

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

>   */
>  std::unique_ptr<Converter> ConverterFactoryBase::create(MediaDevice *media)
>  {
> @@ -236,10 +239,11 @@ std::unique_ptr<Converter> ConverterFactoryBase::create(MediaDevice *media)
>  }
>  
>  /**
> - * \brief Add a converter class to the registry
> + * \brief Add a converter factory to the registry
>   * \param[in] factory Factory to use to construct the converter class
>   *
> - * The caller is responsible to guarantee the uniqueness of the converter name.
> + * The caller is responsible to guarantee the uniqueness of the converter
> + * factory name.
>   */
>  void ConverterFactoryBase::registerType(ConverterFactoryBase *factory)
>  {

-- 
Regards,

Laurent Pinchart