[libcamera-devel] libcamera: stream: Document protected members

1061 diff mbox series

Since commit:
4e1dc9004fca ("libcamera: stream: Make Stream inheritable")
the private members of the Stream class have been turned into protected,
to allows sub-class to access them.

As Doxygen generates documentation for protected members (but not
private ones), add documentation to the stream class for the
'bufferMap_' and 'configuration_' members.

Fixes: 4e1dc9004fca ("libcamera: stream: Make Stream inheritable")
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/stream.cpp | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Hi Jacopo,

Thank you for the patch.

On Thu, Apr 18, 2019 at 05:18:24PM +0200, Jacopo Mondi wrote:
> Since commit:
> 4e1dc9004fca ("libcamera: stream: Make Stream inheritable")

No need for a blank line,

Since commit 4e1dc9004fca ("libcamera: stream: Make Stream inheritable")

> the private members of the Stream class have been turned into protected,
> to allows sub-class to access them.

s/allows sub-class/allow subclasses/

> As Doxygen generates documentation for protected members (but not
> private ones), add documentation to the stream class for the

s/ones/members/

> 'bufferMap_' and 'configuration_' members.
> 
> Fixes: 4e1dc9004fca ("libcamera: stream: Make Stream inheritable")
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/stream.cpp | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 85cd5256ee2f..5839b983f45f 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -194,4 +194,21 @@ Stream::Stream()
>   * \return The active configuration of the stream
>   */
>  
> +/**
> + * \var Stream::bufferPool_
> + * \brief The pool of buffers associated with the stream
> + *
> + * The stream buffer pool is populated by the Camera class after a succesfull
> + * stream configuration.
> + */
> +
> +/**
> + * \var Stream::configuration_
> + * \brief The configuration associated with the stream
> + *
> + * The stream configuration is accessible only to Stream sub-classes and to

This is implied by its protected status, isn't it ?

> + * the Camera class, that associates the Stream with the configuration returned
> + * by a succesfull PipelineHandler::configureStreams() call.

How about documenting when the configuration is set ?

 * \brief The stream configuration
 *
 * The configuration for the stream is set by any successful call to
 * Camera::configureStreams() to includes the stream, and remains valid until
 * the next call to Camera::configureStreams() regardless of if it includes the
 * stream.

> + */
> +
>  } /* namespace libcamera */

Hi Laurent,

On Thu, Apr 18, 2019 at 06:32:06PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Thu, Apr 18, 2019 at 05:18:24PM +0200, Jacopo Mondi wrote:
> > Since commit:
> > 4e1dc9004fca ("libcamera: stream: Make Stream inheritable")
>
> No need for a blank line,
>
> Since commit 4e1dc9004fca ("libcamera: stream: Make Stream inheritable")
>
> > the private members of the Stream class have been turned into protected,
> > to allows sub-class to access them.
>
> s/allows sub-class/allow subclasses/
>
> > As Doxygen generates documentation for protected members (but not
> > private ones), add documentation to the stream class for the
>
> s/ones/members/
>
> > 'bufferMap_' and 'configuration_' members.
> >
> > Fixes: 4e1dc9004fca ("libcamera: stream: Make Stream inheritable")
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/stream.cpp | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > index 85cd5256ee2f..5839b983f45f 100644
> > --- a/src/libcamera/stream.cpp
> > +++ b/src/libcamera/stream.cpp
> > @@ -194,4 +194,21 @@ Stream::Stream()
> >   * \return The active configuration of the stream
> >   */
> >
> > +/**
> > + * \var Stream::bufferPool_
> > + * \brief The pool of buffers associated with the stream
> > + *
> > + * The stream buffer pool is populated by the Camera class after a succesfull
> > + * stream configuration.
> > + */
> > +
> > +/**
> > + * \var Stream::configuration_
> > + * \brief The configuration associated with the stream
> > + *
> > + * The stream configuration is accessible only to Stream sub-classes and to
>
> This is implied by its protected status, isn't it ?
>
> > + * the Camera class, that associates the Stream with the configuration returned
> > + * by a succesfull PipelineHandler::configureStreams() call.
>
> How about documenting when the configuration is set ?
>
>  * \brief The stream configuration
>  *
>  * The configuration for the stream is set by any successful call to
>  * Camera::configureStreams() to includes the stream, and remains valid until
>  * the next call to Camera::configureStreams() regardless of if it includes the
>  * stream.
>

Taken in with s/to includes/that includes/

Thanks
  j

> > + */
> > +
> >  } /* namespace libcamera */
>
> --
> Regards,
>
> Laurent Pinchart