[libcamera-devel] [PATCH v2 2/2] libcamera: request: Make Stream pointer const
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Aug 13 17:06:15 CEST 2020
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 at 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 at 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();
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list