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

[Jacopo Mondi]

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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190419/3ef46e3e/attachment.sig>