[libcamera-devel] [PATCH 4/9] libcamera: Rename PipelineHandler::allocateBuffers
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jul 8 12:22:35 CEST 2019
Hi Jacopo,
On 06/07/2019 12:47, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2019-07-05 00:53:29 +0200, Jacopo Mondi wrote:
>> Now that the pipeline handlers can inspect the Stream's memory type flag
>> the allocateBuffers() operation name does not match the actual operation
>> purpose, which is to setup buffers to export memory to application or
>> either prepare to use externally allocated buffers.
>>
>> Todo: rename the camera operation as well?
>
> I think you should, and it could be squashed in this commit (I'm as
> surprised as you, I'm not pushing for renames to be in two separate
> commits :-)
If there is actions to take when using external buffers, then yes, this
should be renamed.
Thinking about it - yes of course there will be other/internal buffer
pools to allocate and manage - so it should still be in the call graph
for external buffers.
At which point, I think setupBuffers() might well be appropriate naming yes.
And I agree the camera::allocateBuffers() could be renamed too.
>>
>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>> ---
>> src/libcamera/camera.cpp | 2 +-
>> src/libcamera/include/pipeline_handler.h | 4 ++--
>> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++----
>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++---
>> src/libcamera/pipeline/uvcvideo.cpp | 8 ++++----
>> src/libcamera/pipeline/vimc.cpp | 8 ++++----
>> src/libcamera/pipeline_handler.cpp | 6 ++++--
>> 7 files changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
>> index 5f756d41744a..f3f01d040ecf 100644
>> --- a/src/libcamera/camera.cpp
>> +++ b/src/libcamera/camera.cpp
>> @@ -716,7 +716,7 @@ int Camera::allocateBuffers()
>> return -EINVAL;
>> }
>>
>> - int ret = pipe_->allocateBuffers(this, activeStreams_);
>> + int ret = pipe_->setupBuffers(this, activeStreams_);
>> if (ret) {
>> LOG(Camera, Error) << "Failed to allocate buffers";
>> return ret;
>> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
>> index f836d5d1a600..c40d108eb13c 100644
>> --- a/src/libcamera/include/pipeline_handler.h
>> +++ b/src/libcamera/include/pipeline_handler.h
>> @@ -68,8 +68,8 @@ public:
>> const StreamRoles &roles) = 0;
>> virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
>>
>> - virtual int allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams) = 0;
>> + virtual int setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams) = 0;
>> virtual int freeBuffers(Camera *camera,
>> const std::set<Stream *> &streams) = 0;
>>
>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> index 6f14da1f8c70..28dcefe3d19f 100644
>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> @@ -208,8 +208,8 @@ public:
>> const StreamRoles &roles) override;
>> int configure(Camera *camera, CameraConfiguration *config) override;
>>
>> - int allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams) override;
>> + int setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams) override;
>> int freeBuffers(Camera *camera,
>> const std::set<Stream *> &streams) override;
>>
>> @@ -589,8 +589,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>> * In order to be able to start the 'viewfinder' and 'stat' nodes, we need
>> * memory to be reserved.
>> */
>> -int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams)
>> +int PipelineHandlerIPU3::setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams)
>> {
>> IPU3CameraData *data = cameraData(camera);
>> IPU3Stream *outStream = &data->outStream_;
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> index dbb61a21354d..836de1f61630 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> @@ -80,7 +80,7 @@ public:
>> const StreamRoles &roles) override;
>> int configure(Camera *camera, CameraConfiguration *config) override;
>>
>> - int allocateBuffers(Camera *camera,
>> + int setupBuffers(Camera *camera,
>> const std::set<Stream *> &streams) override;
>> int freeBuffers(Camera *camera,
>> const std::set<Stream *> &streams) override;
>> @@ -315,8 +315,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>> return 0;
>> }
>>
>> -int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams)
>> +int PipelineHandlerRkISP1::setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams)
>> {
>> Stream *stream = *streams.begin();
>> int ret;
>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
>> index 5b3c79dda6dd..bba4e7f8d2a0 100644
>> --- a/src/libcamera/pipeline/uvcvideo.cpp
>> +++ b/src/libcamera/pipeline/uvcvideo.cpp
>> @@ -63,8 +63,8 @@ public:
>> const StreamRoles &roles) override;
>> int configure(Camera *camera, CameraConfiguration *config) override;
>>
>> - int allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams) override;
>> + int setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams) override;
>> int freeBuffers(Camera *camera,
>> const std::set<Stream *> &streams) override;
>>
>> @@ -191,8 +191,8 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
>> return 0;
>> }
>>
>> -int PipelineHandlerUVC::allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams)
>> +int PipelineHandlerUVC::setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams)
>> {
>> UVCCameraData *data = cameraData(camera);
>> Stream *stream = *streams.begin();
>> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
>> index c67bdb0ecbff..3e08768b670f 100644
>> --- a/src/libcamera/pipeline/vimc.cpp
>> +++ b/src/libcamera/pipeline/vimc.cpp
>> @@ -70,8 +70,8 @@ public:
>> const StreamRoles &roles) override;
>> int configure(Camera *camera, CameraConfiguration *config) override;
>>
>> - int allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams) override;
>> + int setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams) override;
>> int freeBuffers(Camera *camera,
>> const std::set<Stream *> &streams) override;
>>
>> @@ -193,8 +193,8 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
>> return 0;
>> }
>>
>> -int PipelineHandlerVimc::allocateBuffers(Camera *camera,
>> - const std::set<Stream *> &streams)
>> +int PipelineHandlerVimc::setupBuffers(Camera *camera,
>> + const std::set<Stream *> &streams)
>> {
>> VimcCameraData *data = cameraData(camera);
>> Stream *stream = *streams.begin();
>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
>> index 0283e4e5ad51..67b215483847 100644
>> --- a/src/libcamera/pipeline_handler.cpp
>> +++ b/src/libcamera/pipeline_handler.cpp
>> @@ -280,11 +280,13 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)
>> */
>>
>> /**
>> - * \fn PipelineHandler::allocateBuffers()
>> - * \brief Allocate buffers for a stream
>> + * \fn PipelineHandler::setupBuffers()
>> + * \brief Setup buffer for a stream
>> * \param[in] camera The camera the \a stream belongs to
>> * \param[in] streams The set of streams to allocate buffers for
>> *
>> + * \todo Change this to describe both allocation and importing
>> + *
I agree, and that would block an RB tag I think here.
But the rename is appropriate, I think.
The documentation update will depend on what action is actually
necessary for importing buffers at this point in the API sequence...
>> * This method allocates buffers internally in the pipeline handler for each
>> * stream in the \a streams buffer set, and associates them with the stream's
>> * buffer pool.
>> --
>> 2.21.0
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list