| Message ID | 20251202-cam-control-override-v3-3-eacab052798d@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
2025. 12. 02. 15:49 keltezéssel, Jacopo Mondi írta: > The metadata_ member variable is a pointer, for no specific reason. > Make it an instance and simplify the class destructor. > > Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > include/libcamera/internal/request.h | 4 ++-- > src/libcamera/request.cpp | 13 +++++-------- > 2 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h > index 643f67cabf5778f7515c0117d06d4e0a3fdec4f8..693097ee9a263be5b4217d5e5393ea93cc30a239 100644 > --- a/include/libcamera/internal/request.h > +++ b/include/libcamera/internal/request.h > @@ -36,7 +36,7 @@ public: > Camera *camera() const { return camera_; } > bool hasPendingBuffers() const; > > - ControlList &metadata() { return *metadata_; } > + ControlList &metadata() { return metadata_; } > > bool completeBuffer(FrameBuffer *buffer); > void complete(); > @@ -63,7 +63,7 @@ private: > std::unordered_set<FrameBuffer *> pending_; > std::map<FrameBuffer *, EventNotifier> notifiers_; > std::unique_ptr<Timer> timer_; > - ControlList *metadata_; > + ControlList metadata_; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930..0d6c4a0ce9aec5f328c1f66cea43abb37c82544a 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -53,20 +53,17 @@ LOG_DEFINE_CATEGORY(Request) > /** > * \brief Create a Request::Private > * \param camera The Camera that creates the request > + * > + * \todo Add a validator for metadata controls. > */ > Request::Private::Private(Camera *camera) > - : camera_(camera), cancelled_(false) > + : camera_(camera), cancelled_(false), metadata_(controls::controls) > { > - /** > - * \todo Add a validator for metadata controls. > - */ > - metadata_ = new ControlList(controls::controls); > } > > Request::Private::~Private() > { > doCancelRequest(); > - delete metadata_; > } > > /** > @@ -410,7 +407,7 @@ void Request::reuse(ReuseFlag flags) > status_ = RequestPending; > > controls_->clear(); > - _d()->metadata_->clear(); > + _d()->metadata_.clear(); > } > > /** > @@ -435,7 +432,7 @@ void Request::reuse(ReuseFlag flags) > */ > const ControlList &Request::metadata() const > { > - return *_d()->metadata_; > + return _d()->metadata_; > } > > /** >
Hi Jacopo On 02/12/2025 14:49, Jacopo Mondi wrote: > The metadata_ member variable is a pointer, for no specific reason. > Make it an instance and simplify the class destructor. > > Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- LGTM: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > include/libcamera/internal/request.h | 4 ++-- > src/libcamera/request.cpp | 13 +++++-------- > 2 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h > index 643f67cabf5778f7515c0117d06d4e0a3fdec4f8..693097ee9a263be5b4217d5e5393ea93cc30a239 100644 > --- a/include/libcamera/internal/request.h > +++ b/include/libcamera/internal/request.h > @@ -36,7 +36,7 @@ public: > Camera *camera() const { return camera_; } > bool hasPendingBuffers() const; > > - ControlList &metadata() { return *metadata_; } > + ControlList &metadata() { return metadata_; } > > bool completeBuffer(FrameBuffer *buffer); > void complete(); > @@ -63,7 +63,7 @@ private: > std::unordered_set<FrameBuffer *> pending_; > std::map<FrameBuffer *, EventNotifier> notifiers_; > std::unique_ptr<Timer> timer_; > - ControlList *metadata_; > + ControlList metadata_; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930..0d6c4a0ce9aec5f328c1f66cea43abb37c82544a 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -53,20 +53,17 @@ LOG_DEFINE_CATEGORY(Request) > /** > * \brief Create a Request::Private > * \param camera The Camera that creates the request > + * > + * \todo Add a validator for metadata controls. > */ > Request::Private::Private(Camera *camera) > - : camera_(camera), cancelled_(false) > + : camera_(camera), cancelled_(false), metadata_(controls::controls) > { > - /** > - * \todo Add a validator for metadata controls. > - */ > - metadata_ = new ControlList(controls::controls); > } > > Request::Private::~Private() > { > doCancelRequest(); > - delete metadata_; > } > > /** > @@ -410,7 +407,7 @@ void Request::reuse(ReuseFlag flags) > status_ = RequestPending; > > controls_->clear(); > - _d()->metadata_->clear(); > + _d()->metadata_.clear(); > } > > /** > @@ -435,7 +432,7 @@ void Request::reuse(ReuseFlag flags) > */ > const ControlList &Request::metadata() const > { > - return *_d()->metadata_; > + return _d()->metadata_; > } > > /** >
On 15/12/2025 11:56, Dan Scally wrote: > Hi Jacopo > > On 02/12/2025 14:49, Jacopo Mondi wrote: >> The metadata_ member variable is a pointer, for no specific reason. >> Make it an instance and simplify the class destructor. Oh, except I think s/destructor/constructor?>> >> Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >> --- > > LGTM: > > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > >> include/libcamera/internal/request.h | 4 ++-- >> src/libcamera/request.cpp | 13 +++++-------- >> 2 files changed, 7 insertions(+), 10 deletions(-) >> >> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h >> index 643f67cabf5778f7515c0117d06d4e0a3fdec4f8..693097ee9a263be5b4217d5e5393ea93cc30a239 100644 >> --- a/include/libcamera/internal/request.h >> +++ b/include/libcamera/internal/request.h >> @@ -36,7 +36,7 @@ public: >> Camera *camera() const { return camera_; } >> bool hasPendingBuffers() const; >> - ControlList &metadata() { return *metadata_; } >> + ControlList &metadata() { return metadata_; } >> bool completeBuffer(FrameBuffer *buffer); >> void complete(); >> @@ -63,7 +63,7 @@ private: >> std::unordered_set<FrameBuffer *> pending_; >> std::map<FrameBuffer *, EventNotifier> notifiers_; >> std::unique_ptr<Timer> timer_; >> - ControlList *metadata_; >> + ControlList metadata_; >> }; >> } /* namespace libcamera */ >> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp >> index a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930..0d6c4a0ce9aec5f328c1f66cea43abb37c82544a 100644 >> --- a/src/libcamera/request.cpp >> +++ b/src/libcamera/request.cpp >> @@ -53,20 +53,17 @@ LOG_DEFINE_CATEGORY(Request) >> /** >> * \brief Create a Request::Private >> * \param camera The Camera that creates the request >> + * >> + * \todo Add a validator for metadata controls. >> */ >> Request::Private::Private(Camera *camera) >> - : camera_(camera), cancelled_(false) >> + : camera_(camera), cancelled_(false), metadata_(controls::controls) >> { >> - /** >> - * \todo Add a validator for metadata controls. >> - */ >> - metadata_ = new ControlList(controls::controls); >> } >> Request::Private::~Private() >> { >> doCancelRequest(); >> - delete metadata_; >> } >> /** >> @@ -410,7 +407,7 @@ void Request::reuse(ReuseFlag flags) >> status_ = RequestPending; >> controls_->clear(); >> - _d()->metadata_->clear(); >> + _d()->metadata_.clear(); >> } >> /** >> @@ -435,7 +432,7 @@ void Request::reuse(ReuseFlag flags) >> */ >> const ControlList &Request::metadata() const >> { >> - return *_d()->metadata_; >> + return _d()->metadata_; >> } >> /** >> >
diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h index 643f67cabf5778f7515c0117d06d4e0a3fdec4f8..693097ee9a263be5b4217d5e5393ea93cc30a239 100644 --- a/include/libcamera/internal/request.h +++ b/include/libcamera/internal/request.h @@ -36,7 +36,7 @@ public: Camera *camera() const { return camera_; } bool hasPendingBuffers() const; - ControlList &metadata() { return *metadata_; } + ControlList &metadata() { return metadata_; } bool completeBuffer(FrameBuffer *buffer); void complete(); @@ -63,7 +63,7 @@ private: std::unordered_set<FrameBuffer *> pending_; std::map<FrameBuffer *, EventNotifier> notifiers_; std::unique_ptr<Timer> timer_; - ControlList *metadata_; + ControlList metadata_; }; } /* namespace libcamera */ diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930..0d6c4a0ce9aec5f328c1f66cea43abb37c82544a 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -53,20 +53,17 @@ LOG_DEFINE_CATEGORY(Request) /** * \brief Create a Request::Private * \param camera The Camera that creates the request + * + * \todo Add a validator for metadata controls. */ Request::Private::Private(Camera *camera) - : camera_(camera), cancelled_(false) + : camera_(camera), cancelled_(false), metadata_(controls::controls) { - /** - * \todo Add a validator for metadata controls. - */ - metadata_ = new ControlList(controls::controls); } Request::Private::~Private() { doCancelRequest(); - delete metadata_; } /** @@ -410,7 +407,7 @@ void Request::reuse(ReuseFlag flags) status_ = RequestPending; controls_->clear(); - _d()->metadata_->clear(); + _d()->metadata_.clear(); } /** @@ -435,7 +432,7 @@ void Request::reuse(ReuseFlag flags) */ const ControlList &Request::metadata() const { - return *_d()->metadata_; + return _d()->metadata_; } /**
The metadata_ member variable is a pointer, for no specific reason. Make it an instance and simplify the class destructor. Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- include/libcamera/internal/request.h | 4 ++-- src/libcamera/request.cpp | 13 +++++-------- 2 files changed, 7 insertions(+), 10 deletions(-)