| Message ID | 20230928185537.20178-2-andrey.konovalov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On jeu., sept. 28, 2023 at 21:55, Andrey Konovalov <andrey.konovalov@linaro.org> 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@linaro.org> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > --- > 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 > - * 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. > + * If the media device driver name doesn't match anything a null pointer is > + * returned. > */ > 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) > { > -- > 2.34.1
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@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@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) > {
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 - * 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. + * If the media device driver name doesn't match anything a null pointer is + * returned. */ 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) {
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@linaro.org> --- src/libcamera/converter.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)