| Message ID | 20200813095344.3495212-3-niklas.soderlund@ragnatech.se |
|---|---|
| State | Accepted |
| Commit | 27869c5f649c4b524e142014be073b40230ecbc4 |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On 13/08/2020 10:53, Niklas Söderlund wrote: > The Stream pointer just acts as a key in the Request object. There is no > good use-case to modify a stream from a pointer retrieved from the > Request, make it const. This allow pipeline handlers to better express s/allow/allows/ > that the Stream pointer is retrieved in a Request should just be treated > as a key. > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Only a random not-related to this patch question below: But I think this makes complete sense to be const. The users here shouldn't be modifying the stream. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/request.h | 6 +++--- > src/cam/capture.cpp | 4 ++-- > src/cam/capture.h | 2 +- > src/libcamera/camera.cpp | 4 ++-- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > src/libcamera/request.cpp | 6 +++--- > src/qcam/main_window.h | 2 +- > test/camera/buffer_import.cpp | 2 +- > test/camera/capture.cpp | 2 +- > 9 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > index e74f56a7d6415315..5976ac50c9d3c42b 100644 > --- a/include/libcamera/request.h > +++ b/include/libcamera/request.h > @@ -31,7 +31,7 @@ public: > RequestCancelled, > }; > > - using BufferMap = std::map<Stream *, FrameBuffer *>; > + using BufferMap = std::map<const Stream *, FrameBuffer *>; > > Request(Camera *camera, uint64_t cookie = 0); > Request(const Request &) = delete; > @@ -41,8 +41,8 @@ public: > ControlList &controls() { return *controls_; } > ControlList &metadata() { return *metadata_; } > const BufferMap &buffers() const { return bufferMap_; } > - int addBuffer(Stream *stream, FrameBuffer *buffer); > - FrameBuffer *findBuffer(Stream *stream) const; > + int addBuffer(const Stream *stream, FrameBuffer *buffer); > + FrameBuffer *findBuffer(const Stream *stream) const; > > uint64_t cookie() const { return cookie_; } > Status status() const { return status_; } > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > index 0720376983470f2f..af9029b743de606b 100644 > --- a/src/cam/capture.cpp > +++ b/src/cam/capture.cpp > @@ -169,7 +169,7 @@ void Capture::requestComplete(Request *request) > info << "fps: " << std::fixed << std::setprecision(2) << fps; > > for (auto it = buffers.begin(); it != buffers.end(); ++it) { > - Stream *stream = it->first; > + const Stream *stream = it->first; > FrameBuffer *buffer = it->second; > const std::string &name = streamName_[stream]; > > @@ -209,7 +209,7 @@ void Capture::requestComplete(Request *request) > } > > for (auto it = buffers.begin(); it != buffers.end(); ++it) { > - Stream *stream = it->first; > + const Stream *stream = it->first; > FrameBuffer *buffer = it->second; > > request->addBuffer(stream, buffer); > diff --git a/src/cam/capture.h b/src/cam/capture.h > index 32940a2a12138887..b4e39d51fdfa9512 100644 > --- a/src/cam/capture.h > +++ b/src/cam/capture.h > @@ -36,7 +36,7 @@ private: > std::shared_ptr<libcamera::Camera> camera_; > libcamera::CameraConfiguration *config_; > > - std::map<libcamera::Stream *, std::string> streamName_; > + std::map<const libcamera::Stream *, std::string> streamName_; > BufferWriter *writer_; > std::chrono::steady_clock::time_point last_; > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 820fa1e3065e5f88..afdd47750f58f190 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -279,7 +279,7 @@ public: > std::shared_ptr<PipelineHandler> pipe_; > std::string id_; > std::set<Stream *> streams_; Should streams_ also be const (if so, in a different patch, I haven't looked back to see how it's used though so it might not be) > - std::set<Stream *> activeStreams_; > + std::set<const Stream *> activeStreams_; > > private: > bool disconnected_; > @@ -889,7 +889,7 @@ int Camera::queueRequest(Request *request) > } > > for (auto const &it : request->buffers()) { > - Stream *stream = it.first; > + const Stream *stream = it.first; > > if (p_->activeStreams_.find(stream) == p_->activeStreams_.end()) { > LOG(Camera, Error) << "Invalid request"; > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index d931ed333b4a1df6..019e50b8f444c153 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -663,7 +663,7 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > > /* Queue all buffers from the request aimed for the ImgU. */ > for (auto it : request->buffers()) { > - Stream *stream = static_cast<Stream *>(it.first); > + const Stream *stream = it.first; > FrameBuffer *buffer = it.second; > int ret; > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index d5f11e8c43c32766..60b306922de2fe06 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -127,7 +127,7 @@ Request::~Request() > * \retval -EEXIST The request already contains a buffer for the stream > * \retval -EINVAL The buffer does not reference a valid Stream > */ > -int Request::addBuffer(Stream *stream, FrameBuffer *buffer) > +int Request::addBuffer(const Stream *stream, FrameBuffer *buffer) > { > if (!stream) { > LOG(Request, Error) << "Invalid stream reference"; > @@ -162,9 +162,9 @@ int Request::addBuffer(Stream *stream, FrameBuffer *buffer) > * \return The buffer associated with the stream, or nullptr if the stream is > * not part of this request > */ > -FrameBuffer *Request::findBuffer(Stream *stream) const > +FrameBuffer *Request::findBuffer(const Stream *stream) const > { > - auto it = bufferMap_.find(stream); > + const auto it = bufferMap_.find(stream); > if (it == bufferMap_.end()) > return nullptr; > > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index 6e1bcd76a2438b4c..3d21779e3f4a7c44 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -126,7 +126,7 @@ private: > bool captureRaw_; > Stream *vfStream_; > Stream *rawStream_; > - std::map<Stream *, QQueue<FrameBuffer *>> freeBuffers_; > + std::map<const Stream *, QQueue<FrameBuffer *>> freeBuffers_; > QQueue<CaptureRequest> doneQueue_; > QMutex mutex_; /* Protects freeBuffers_ and doneQueue_ */ > > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp > index 726d2cb2fe757066..97a8582761a74dc6 100644 > --- a/test/camera/buffer_import.cpp > +++ b/test/camera/buffer_import.cpp > @@ -51,7 +51,7 @@ protected: > completeRequestsCount_++; > > /* Create a new request. */ > - Stream *stream = buffers.begin()->first; > + const Stream *stream = buffers.begin()->first; > FrameBuffer *buffer = buffers.begin()->second; > > request = camera_->createRequest(); > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp > index ae572eb955753151..0fe3bf9be7f13f4f 100644 > --- a/test/camera/capture.cpp > +++ b/test/camera/capture.cpp > @@ -44,7 +44,7 @@ protected: > completeRequestsCount_++; > > /* Create a new request. */ > - Stream *stream = buffers.begin()->first; > + const Stream *stream = buffers.begin()->first; > FrameBuffer *buffer = buffers.begin()->second; > > request = camera_->createRequest(); >
diff --git a/include/libcamera/request.h b/include/libcamera/request.h index e74f56a7d6415315..5976ac50c9d3c42b 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -31,7 +31,7 @@ public: RequestCancelled, }; - using BufferMap = std::map<Stream *, FrameBuffer *>; + using BufferMap = std::map<const Stream *, FrameBuffer *>; Request(Camera *camera, uint64_t cookie = 0); Request(const Request &) = delete; @@ -41,8 +41,8 @@ public: ControlList &controls() { return *controls_; } ControlList &metadata() { return *metadata_; } const BufferMap &buffers() const { return bufferMap_; } - int addBuffer(Stream *stream, FrameBuffer *buffer); - FrameBuffer *findBuffer(Stream *stream) const; + int addBuffer(const Stream *stream, FrameBuffer *buffer); + FrameBuffer *findBuffer(const Stream *stream) const; uint64_t cookie() const { return cookie_; } Status status() const { return status_; } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 0720376983470f2f..af9029b743de606b 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -169,7 +169,7 @@ void Capture::requestComplete(Request *request) info << "fps: " << std::fixed << std::setprecision(2) << fps; for (auto it = buffers.begin(); it != buffers.end(); ++it) { - Stream *stream = it->first; + const Stream *stream = it->first; FrameBuffer *buffer = it->second; const std::string &name = streamName_[stream]; @@ -209,7 +209,7 @@ void Capture::requestComplete(Request *request) } for (auto it = buffers.begin(); it != buffers.end(); ++it) { - Stream *stream = it->first; + const Stream *stream = it->first; FrameBuffer *buffer = it->second; request->addBuffer(stream, buffer); diff --git a/src/cam/capture.h b/src/cam/capture.h index 32940a2a12138887..b4e39d51fdfa9512 100644 --- a/src/cam/capture.h +++ b/src/cam/capture.h @@ -36,7 +36,7 @@ private: std::shared_ptr<libcamera::Camera> camera_; libcamera::CameraConfiguration *config_; - std::map<libcamera::Stream *, std::string> streamName_; + std::map<const libcamera::Stream *, std::string> streamName_; BufferWriter *writer_; std::chrono::steady_clock::time_point last_; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 820fa1e3065e5f88..afdd47750f58f190 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -279,7 +279,7 @@ public: std::shared_ptr<PipelineHandler> pipe_; std::string id_; std::set<Stream *> streams_; - std::set<Stream *> activeStreams_; + std::set<const Stream *> activeStreams_; private: bool disconnected_; @@ -889,7 +889,7 @@ int Camera::queueRequest(Request *request) } for (auto const &it : request->buffers()) { - Stream *stream = it.first; + const Stream *stream = it.first; if (p_->activeStreams_.find(stream) == p_->activeStreams_.end()) { LOG(Camera, Error) << "Invalid request"; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index d931ed333b4a1df6..019e50b8f444c153 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -663,7 +663,7 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) /* Queue all buffers from the request aimed for the ImgU. */ for (auto it : request->buffers()) { - Stream *stream = static_cast<Stream *>(it.first); + const Stream *stream = it.first; FrameBuffer *buffer = it.second; int ret; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index d5f11e8c43c32766..60b306922de2fe06 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -127,7 +127,7 @@ Request::~Request() * \retval -EEXIST The request already contains a buffer for the stream * \retval -EINVAL The buffer does not reference a valid Stream */ -int Request::addBuffer(Stream *stream, FrameBuffer *buffer) +int Request::addBuffer(const Stream *stream, FrameBuffer *buffer) { if (!stream) { LOG(Request, Error) << "Invalid stream reference"; @@ -162,9 +162,9 @@ int Request::addBuffer(Stream *stream, FrameBuffer *buffer) * \return The buffer associated with the stream, or nullptr if the stream is * not part of this request */ -FrameBuffer *Request::findBuffer(Stream *stream) const +FrameBuffer *Request::findBuffer(const Stream *stream) const { - auto it = bufferMap_.find(stream); + const auto it = bufferMap_.find(stream); if (it == bufferMap_.end()) return nullptr; diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 6e1bcd76a2438b4c..3d21779e3f4a7c44 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -126,7 +126,7 @@ private: bool captureRaw_; Stream *vfStream_; Stream *rawStream_; - std::map<Stream *, QQueue<FrameBuffer *>> freeBuffers_; + std::map<const Stream *, QQueue<FrameBuffer *>> freeBuffers_; QQueue<CaptureRequest> doneQueue_; QMutex mutex_; /* Protects freeBuffers_ and doneQueue_ */ diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 726d2cb2fe757066..97a8582761a74dc6 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -51,7 +51,7 @@ protected: completeRequestsCount_++; /* Create a new request. */ - Stream *stream = buffers.begin()->first; + const Stream *stream = buffers.begin()->first; FrameBuffer *buffer = buffers.begin()->second; request = camera_->createRequest(); diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index ae572eb955753151..0fe3bf9be7f13f4f 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -44,7 +44,7 @@ protected: completeRequestsCount_++; /* Create a new request. */ - Stream *stream = buffers.begin()->first; + const Stream *stream = buffers.begin()->first; FrameBuffer *buffer = buffers.begin()->second; request = camera_->createRequest();
The Stream pointer just acts as a key in the Request object. There is no good use-case to modify a stream from a pointer retrieved from the Request, make it const. This allow pipeline handlers to better express that the Stream pointer is retrieved in a Request should just be treated as a key. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- include/libcamera/request.h | 6 +++--- src/cam/capture.cpp | 4 ++-- src/cam/capture.h | 2 +- src/libcamera/camera.cpp | 4 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/request.cpp | 6 +++--- src/qcam/main_window.h | 2 +- test/camera/buffer_import.cpp | 2 +- test/camera/capture.cpp | 2 +- 9 files changed, 15 insertions(+), 15 deletions(-)