| Message ID | 20201209173520.284266-2-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On 09/12/2020 17:35, Jacopo Mondi wrote: > Mark the metadata() and controls() operations as const function to make > it possible to retrieve them from const Request instances. > Is this accurate? How do applications set controls on the request if it's const? (before queueing it) Or is that done somewhere else? Metadata certainly seems like it should be const though... -- Kieran > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/request.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > index 655b1324bae8..1a89622ab989 100644 > --- a/include/libcamera/request.h > +++ b/include/libcamera/request.h > @@ -45,8 +45,8 @@ public: > > void reuse(ReuseFlag flags = Default); > > - ControlList &controls() { return *controls_; } > - ControlList &metadata() { return *metadata_; } > + ControlList &controls() const { return *controls_; } > + ControlList &metadata() const { return *metadata_; } > const BufferMap &buffers() const { return bufferMap_; } > int addBuffer(const Stream *stream, FrameBuffer *buffer); > FrameBuffer *findBuffer(const Stream *stream) const; >
Hi Kieran On Thu, Dec 10, 2020 at 09:35:16AM +0000, Kieran Bingham wrote: > Hi Jacopo, > > On 09/12/2020 17:35, Jacopo Mondi wrote: > > Mark the metadata() and controls() operations as const function to make > > it possible to retrieve them from const Request instances. > > > > Is this accurate? How do applications set controls on the request if > it's const? (before queueing it) Probably from a 'const Request' you should only be able to get a 'const ControlList &' ? > > Or is that done somewhere else? > > Metadata certainly seems like it should be const though... Happy to drop the const for controls(). I added it there expecting some questions :) > > -- > Kieran > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > include/libcamera/request.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > > index 655b1324bae8..1a89622ab989 100644 > > --- a/include/libcamera/request.h > > +++ b/include/libcamera/request.h > > @@ -45,8 +45,8 @@ public: > > > > void reuse(ReuseFlag flags = Default); > > > > - ControlList &controls() { return *controls_; } > > - ControlList &metadata() { return *metadata_; } > > + ControlList &controls() const { return *controls_; } > > + ControlList &metadata() const { return *metadata_; } > > const BufferMap &buffers() const { return bufferMap_; } > > int addBuffer(const Stream *stream, FrameBuffer *buffer); > > FrameBuffer *findBuffer(const Stream *stream) const; > > > > -- > Regards > -- > Kieran
Hi Jacopo, On 10/12/2020 09:43, Jacopo Mondi wrote: > Hi Kieran > > On Thu, Dec 10, 2020 at 09:35:16AM +0000, Kieran Bingham wrote: >> Hi Jacopo, >> >> On 09/12/2020 17:35, Jacopo Mondi wrote: >>> Mark the metadata() and controls() operations as const function to make >>> it possible to retrieve them from const Request instances. >>> >> >> Is this accurate? How do applications set controls on the request if >> it's const? (before queueing it) > > Probably from a 'const Request' you should only be able to get a > 'const ControlList &' ? Do applications have a const Request? Aren't they able to modify it to queue as they need? err ... Do we not have any /setting/ of controls in any of our codebase already? (From the public-api viewpoint) I don't think we do... No current compiling code /sets/ a control in a request. I think we need to do something about that ;-) -- Kieran > >> >> Or is that done somewhere else? >> >> Metadata certainly seems like it should be const though... > > Happy to drop the const for controls(). I added it there expecting > some questions :) > >> >> -- >> Kieran >> >> >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> >>> --- >>> include/libcamera/request.h | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h >>> index 655b1324bae8..1a89622ab989 100644 >>> --- a/include/libcamera/request.h >>> +++ b/include/libcamera/request.h >>> @@ -45,8 +45,8 @@ public: >>> >>> void reuse(ReuseFlag flags = Default); >>> >>> - ControlList &controls() { return *controls_; } >>> - ControlList &metadata() { return *metadata_; } >>> + ControlList &controls() const { return *controls_; } >>> + ControlList &metadata() const { return *metadata_; } >>> const BufferMap &buffers() const { return bufferMap_; } >>> int addBuffer(const Stream *stream, FrameBuffer *buffer); >>> FrameBuffer *findBuffer(const Stream *stream) const; >>> >> >> -- >> Regards >> -- >> Kieran
Hi Jacopo, On Thu, Dec 10, 2020 at 10:43:08AM +0100, Jacopo Mondi wrote: > On Thu, Dec 10, 2020 at 09:35:16AM +0000, Kieran Bingham wrote: > > On 09/12/2020 17:35, Jacopo Mondi wrote: > > > Mark the metadata() and controls() operations as const function to make > > > it possible to retrieve them from const Request instances. > > > > Is this accurate? How do applications set controls on the request if > > it's const? (before queueing it) > > Probably from a 'const Request' you should only be able to get a > 'const ControlList &' ? Yes, otherwise the user of a const Request would be allowed to modify the controls and metadata stored in the request, which isn't nice. There's no issue having both ControlList &metadata() { return *metadata_; } const ControlList &metadata() const { return *metadata_; } This however means duplicating the documentation I'm afraid, but we can use copydoc. See ControlValue::data(). > > Or is that done somewhere else? > > > > Metadata certainly seems like it should be const though... > > Happy to drop the const for controls(). I added it there expecting > some questions :) Up to you. I don't expect there would be lots of use cases for accessing controls in a const request, but who knows ? > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > include/libcamera/request.h | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > > > index 655b1324bae8..1a89622ab989 100644 > > > --- a/include/libcamera/request.h > > > +++ b/include/libcamera/request.h > > > @@ -45,8 +45,8 @@ public: > > > > > > void reuse(ReuseFlag flags = Default); > > > > > > - ControlList &controls() { return *controls_; } > > > - ControlList &metadata() { return *metadata_; } > > > + ControlList &controls() const { return *controls_; } > > > + ControlList &metadata() const { return *metadata_; } > > > const BufferMap &buffers() const { return bufferMap_; } > > > int addBuffer(const Stream *stream, FrameBuffer *buffer); > > > FrameBuffer *findBuffer(const Stream *stream) const;
diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 655b1324bae8..1a89622ab989 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -45,8 +45,8 @@ public: void reuse(ReuseFlag flags = Default); - ControlList &controls() { return *controls_; } - ControlList &metadata() { return *metadata_; } + ControlList &controls() const { return *controls_; } + ControlList &metadata() const { return *metadata_; } const BufferMap &buffers() const { return bufferMap_; } int addBuffer(const Stream *stream, FrameBuffer *buffer); FrameBuffer *findBuffer(const Stream *stream) const;
Mark the metadata() and controls() operations as const function to make it possible to retrieve them from const Request instances. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/request.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)