[libcamera-devel,v3,5/8] libcamera: request: Add streams() method

910 diff mbox series

Add a method to retrieve the list of streams contained in a requests.

This is useful for pipeline handler willing to cycle on all the streams
a request contains at queueRequest() time.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/request.h |  3 +++
 src/libcamera/request.cpp   | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

Hi Jacopo,

Thanks for your work.

On 2019-04-03 17:07:32 +0200, Jacopo Mondi wrote:
> Add a method to retrieve the list of streams contained in a requests.
> 
> This is useful for pipeline handler willing to cycle on all the streams
> a request contains at queueRequest() time.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/request.h |  3 +++
>  src/libcamera/request.cpp   | 15 +++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 0dbd425115e8..5ac4d20d1d9f 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -7,6 +7,7 @@
>  #ifndef __LIBCAMERA_REQUEST_H__
>  #define __LIBCAMERA_REQUEST_H__
>  
> +#include <list>
>  #include <map>
>  #include <unordered_set>
>  
> @@ -35,6 +36,8 @@ public:
>  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
>  	Buffer *findBuffer(Stream *stream) const;
>  
> +	const std::list<Stream *> streams() const;
> +
>  	Status status() const { return status_; }
>  
>  private:
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index e0e77e972411..3a7841fb2bb3 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -93,6 +93,21 @@ Buffer *Request::findBuffer(Stream *stream) const
>  	return it->second;
>  }
>  
> +/**
> + * \brief Retrieve the set of streams contained in the request
> + *
> + * \return The set of streams contained in the request
> + */
> +const std::list<Stream *> Request::streams() const

I would make this std::set<Stream *> to have a API guarantee there are 
no duplicated Stream *. Whit this fixed,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> +{
> +	std::list<Stream *> streams = {};
> +
> +	for (auto const &it : bufferMap_)
> +		streams.push_front(it.first);
> +
> +	return streams;
> +}
> +
>  /**
>   * \fn Request::status()
>   * \brief Retrieve the request completion status
> -- 
> 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 Fri, Apr 05, 2019 at 01:34:33PM +0200, Niklas Söderlund wrote:
> On 2019-04-03 17:07:32 +0200, Jacopo Mondi wrote:
> > Add a method to retrieve the list of streams contained in a requests.

s/requests/request/

> > 
> > This is useful for pipeline handler willing to cycle on all the streams

s/handler/handlers/
s/willing/that need to/

> > a request contains at queueRequest() time.

Wouldn't it be better to expose the streams to buffers map instead ?
The findBuffer() method doesn't seem very nice to use.

> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  include/libcamera/request.h |  3 +++
> >  src/libcamera/request.cpp   | 15 +++++++++++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index 0dbd425115e8..5ac4d20d1d9f 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -7,6 +7,7 @@
> >  #ifndef __LIBCAMERA_REQUEST_H__
> >  #define __LIBCAMERA_REQUEST_H__
> >  
> > +#include <list>
> >  #include <map>
> >  #include <unordered_set>
> >  
> > @@ -35,6 +36,8 @@ public:
> >  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
> >  	Buffer *findBuffer(Stream *stream) const;
> >  
> > +	const std::list<Stream *> streams() const;
> > +
> >  	Status status() const { return status_; }
> >  
> >  private:
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index e0e77e972411..3a7841fb2bb3 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -93,6 +93,21 @@ Buffer *Request::findBuffer(Stream *stream) const
> >  	return it->second;
> >  }
> >  
> > +/**
> > + * \brief Retrieve the set of streams contained in the request
> > + *
> > + * \return The set of streams contained in the request
> > + */
> > +const std::list<Stream *> Request::streams() const
> 
> I would make this std::set<Stream *> to have a API guarantee there are 
> no duplicated Stream *. Whit this fixed,
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> > +{
> > +	std::list<Stream *> streams = {};
> > +
> > +	for (auto const &it : bufferMap_)
> > +		streams.push_front(it.first);
> > +
> > +	return streams;
> > +}
> > +
> >  /**
> >   * \fn Request::status()
> >   * \brief Retrieve the request completion status