| Message ID | 20190409192548.20325-6-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. On 2019-04-09 21:25:41 +0200, Jacopo Mondi wrote: > Add to the Request class a method to access the map to Stream to Buffer. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/request.h | 2 ++ > src/libcamera/request.cpp | 10 ++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > index 0dbd425115e8..2004312a2233 100644 > --- a/include/libcamera/request.h > +++ b/include/libcamera/request.h > @@ -37,6 +37,8 @@ public: > > Status status() const { return status_; } > > + const std::map<Stream *, Buffer *> &bufferMap() const { return bufferMap_; } > + > private: > friend class Camera; > friend class PipelineHandler; > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index e0e77e972411..46f9add98fde 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -106,6 +106,16 @@ Buffer *Request::findBuffer(Stream *stream) const > * \return The request completion status > */ > > +/** > + * \fn Request::bufferMap() > + * \brief Retrieve the request' stream to buffer map > + * > + * Return a reference to the map that associates each Stream part of the > + * request to the Buffer the Stream output should be directed to. > + * > + * \return The map of Stream to Buffers > + */ > + > /** > * \brief Prepare the resources for the completion handler > */ > -- > 2.21.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thank you for the patch. On Tue, Apr 09, 2019 at 09:25:41PM +0200, Jacopo Mondi wrote: > Add to the Request class a method to access the map to Stream to Buffer. s/to Stream/of Stream/ > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/request.h | 2 ++ > src/libcamera/request.cpp | 10 ++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > index 0dbd425115e8..2004312a2233 100644 > --- a/include/libcamera/request.h > +++ b/include/libcamera/request.h > @@ -37,6 +37,8 @@ public: > > Status status() const { return status_; } > > + const std::map<Stream *, Buffer *> &bufferMap() const { return bufferMap_; } > + > private: > friend class Camera; > friend class PipelineHandler; > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index e0e77e972411..46f9add98fde 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -106,6 +106,16 @@ Buffer *Request::findBuffer(Stream *stream) const > * \return The request completion status > */ > > +/** > + * \fn Request::bufferMap() > + * \brief Retrieve the request' stream to buffer map s/request' stream to buffer/request's streams to buffer/ > + * > + * Return a reference to the map that associates each Stream part of the > + * request to the Buffer the Stream output should be directed to. > + * > + * \return The map of Stream to Buffers If you use class names I wouldn't make them plural. "streams to buffers" or "Stream to Buffer". Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + > /** > * \brief Prepare the resources for the completion handler > */
Hi Jacopo, On Mon, Apr 15, 2019 at 04:06:43PM +0300, Laurent Pinchart wrote: > On Tue, Apr 09, 2019 at 09:25:41PM +0200, Jacopo Mondi wrote: > > Add to the Request class a method to access the map to Stream to Buffer. > > s/to Stream/of Stream/ > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > include/libcamera/request.h | 2 ++ > > src/libcamera/request.cpp | 10 ++++++++++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > > index 0dbd425115e8..2004312a2233 100644 > > --- a/include/libcamera/request.h > > +++ b/include/libcamera/request.h > > @@ -37,6 +37,8 @@ public: > > > > Status status() const { return status_; } > > > > + const std::map<Stream *, Buffer *> &bufferMap() const { return bufferMap_; } I forgot to mention that I would move this above the setBuffers() method, and maybe rename it to buffers() to match setBuffers(). How about also removing the findBuffer() method ? > > + > > private: > > friend class Camera; > > friend class PipelineHandler; > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > > index e0e77e972411..46f9add98fde 100644 > > --- a/src/libcamera/request.cpp > > +++ b/src/libcamera/request.cpp > > @@ -106,6 +106,16 @@ Buffer *Request::findBuffer(Stream *stream) const > > * \return The request completion status > > */ > > > > +/** > > + * \fn Request::bufferMap() > > + * \brief Retrieve the request' stream to buffer map > > s/request' stream to buffer/request's streams to buffer/ > > > + * > > + * Return a reference to the map that associates each Stream part of the > > + * request to the Buffer the Stream output should be directed to. > > + * > > + * \return The map of Stream to Buffers > > If you use class names I wouldn't make them plural. "streams to buffers" > or "Stream to Buffer". > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + */ > > + > > /** > > * \brief Prepare the resources for the completion handler > > */
Hi Laurent, On Mon, Apr 15, 2019 at 04:25:50PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > On Mon, Apr 15, 2019 at 04:06:43PM +0300, Laurent Pinchart wrote: > > On Tue, Apr 09, 2019 at 09:25:41PM +0200, Jacopo Mondi wrote: > > > Add to the Request class a method to access the map to Stream to Buffer. > > > > s/to Stream/of Stream/ > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > include/libcamera/request.h | 2 ++ > > > src/libcamera/request.cpp | 10 ++++++++++ > > > 2 files changed, 12 insertions(+) > > > > > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > > > index 0dbd425115e8..2004312a2233 100644 > > > --- a/include/libcamera/request.h > > > +++ b/include/libcamera/request.h > > > @@ -37,6 +37,8 @@ public: > > > > > > Status status() const { return status_; } > > > > > > + const std::map<Stream *, Buffer *> &bufferMap() const { return bufferMap_; } > > I forgot to mention that I would move this above the setBuffers() > method, and maybe rename it to buffers() to match setBuffers(). How I considered that, but I was thinking that from a 'buffers()' methods I would expect a list/vector/container of buffers to be returned, not a map.... If you all don't think this is confusing I'll change it > about also removing the findBuffer() method ? That would indeed be stale with a 'buffers()' methods, so let's remove it before it gets used in other places. Thanks j > > > > + > > > private: > > > friend class Camera; > > > friend class PipelineHandler; > > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > > > index e0e77e972411..46f9add98fde 100644 > > > --- a/src/libcamera/request.cpp > > > +++ b/src/libcamera/request.cpp > > > @@ -106,6 +106,16 @@ Buffer *Request::findBuffer(Stream *stream) const > > > * \return The request completion status > > > */ > > > > > > +/** > > > + * \fn Request::bufferMap() > > > + * \brief Retrieve the request' stream to buffer map > > > > s/request' stream to buffer/request's streams to buffer/ > > > > > + * > > > + * Return a reference to the map that associates each Stream part of the > > > + * request to the Buffer the Stream output should be directed to. > > > + * > > > + * \return The map of Stream to Buffers > > > > If you use class names I wouldn't make them plural. "streams to buffers" > > or "Stream to Buffer". > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > + */ > > > + > > > /** > > > * \brief Prepare the resources for the completion handler > > > */ > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Mon, Apr 15, 2019 at 05:05:16PM +0200, Jacopo Mondi wrote: > On Mon, Apr 15, 2019 at 04:25:50PM +0300, Laurent Pinchart wrote: > > On Mon, Apr 15, 2019 at 04:06:43PM +0300, Laurent Pinchart wrote: > >> On Tue, Apr 09, 2019 at 09:25:41PM +0200, Jacopo Mondi wrote: > >>> Add to the Request class a method to access the map to Stream to Buffer. > >> > >> s/to Stream/of Stream/ > >> > >>> > >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > >>> --- > >>> include/libcamera/request.h | 2 ++ > >>> src/libcamera/request.cpp | 10 ++++++++++ > >>> 2 files changed, 12 insertions(+) > >>> > >>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h > >>> index 0dbd425115e8..2004312a2233 100644 > >>> --- a/include/libcamera/request.h > >>> +++ b/include/libcamera/request.h > >>> @@ -37,6 +37,8 @@ public: > >>> > >>> Status status() const { return status_; } > >>> > >>> + const std::map<Stream *, Buffer *> &bufferMap() const { return bufferMap_; } > > > > I forgot to mention that I would move this above the setBuffers() > > method, and maybe rename it to buffers() to match setBuffers(). How > > I considered that, but I was thinking that from a 'buffers()' methods > I would expect a list/vector/container of buffers to be returned, not > a map.... If you all don't think this is confusing I'll change it setBuffers() takes a map already, so I think it should be OK. > > > about also removing the findBuffer() method ? > > That would indeed be stale with a 'buffers()' methods, so let's remove > it before it gets used in other places. Thank you. > >>> + > >>> private: > >>> friend class Camera; > >>> friend class PipelineHandler; > >>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > >>> index e0e77e972411..46f9add98fde 100644 > >>> --- a/src/libcamera/request.cpp > >>> +++ b/src/libcamera/request.cpp > >>> @@ -106,6 +106,16 @@ Buffer *Request::findBuffer(Stream *stream) const > >>> * \return The request completion status > >>> */ > >>> > >>> +/** > >>> + * \fn Request::bufferMap() > >>> + * \brief Retrieve the request' stream to buffer map > >> > >> s/request' stream to buffer/request's streams to buffer/ > >> > >>> + * > >>> + * Return a reference to the map that associates each Stream part of the > >>> + * request to the Buffer the Stream output should be directed to. > >>> + * > >>> + * \return The map of Stream to Buffers > >> > >> If you use class names I wouldn't make them plural. "streams to buffers" > >> or "Stream to Buffer". > >> > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> > >>> + */ > >>> + > >>> /** > >>> * \brief Prepare the resources for the completion handler > >>> */
diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 0dbd425115e8..2004312a2233 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -37,6 +37,8 @@ public: Status status() const { return status_; } + const std::map<Stream *, Buffer *> &bufferMap() const { return bufferMap_; } + private: friend class Camera; friend class PipelineHandler; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index e0e77e972411..46f9add98fde 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -106,6 +106,16 @@ Buffer *Request::findBuffer(Stream *stream) const * \return The request completion status */ +/** + * \fn Request::bufferMap() + * \brief Retrieve the request' stream to buffer map + * + * Return a reference to the map that associates each Stream part of the + * request to the Buffer the Stream output should be directed to. + * + * \return The map of Stream to Buffers + */ + /** * \brief Prepare the resources for the completion handler */
Add to the Request class a method to access the map to Stream to Buffer. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/request.h | 2 ++ src/libcamera/request.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+)