[libcamera-devel] [RFC] libcamera: buffer: Remove copyFrom()
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Sep 21 22:33:02 CEST 2020
On 21/09/2020 20:16, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Mon, Sep 21, 2020 at 08:32:05PM +0200, Niklas Söderlund wrote:
>> There are no user left of the copyFrom() operation, remove it.
>>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>> ---
>> include/libcamera/buffer.h | 2 --
>> src/libcamera/buffer.cpp | 59 --------------------------------------
>> 2 files changed, 61 deletions(-)
>> ---
>>
>> Hello,
>>
>> I'm not sure it's wise to leave this operation in the code while we have
>> no users. But I do fear we might need it the future, but as we don't use
>> or test it I'm sure we will have modified FrameBuffer enough to make
>> this incomplete at that that so I think it's best to remove it. What do
>> you guys think?
>
> Drop it :-)
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> We can always bring code back if needed.
>
Agreed, No users, no tests, no requirement, and an undesired 'feature'.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
What can you envisage in the future that might require bringing it back?
>> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
>> index 6bb2e4f8558f03ac..a26c8927d37a812e 100644
>> --- a/include/libcamera/buffer.h
>> +++ b/include/libcamera/buffer.h
>> @@ -57,8 +57,6 @@ public:
>>
>> unsigned int cookie() const { return cookie_; }
>> void setCookie(unsigned int cookie) { cookie_ = cookie; }
>> -
>> - int copyFrom(const FrameBuffer *src);
>> private:
>> friend class Request; /* Needed to update request_. */
>> friend class V4L2VideoDevice; /* Needed to update metadata_. */
>> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
>> index 03f628e8b585a1f9..75b2693281a74cdc 100644
>> --- a/src/libcamera/buffer.cpp
>> +++ b/src/libcamera/buffer.cpp
>> @@ -226,65 +226,6 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
>> * core never modifies the buffer cookie.
>> */
>>
>> -/**
>> - * \brief Copy the contents from another buffer
>> - * \param[in] src FrameBuffer to copy
>> - *
>> - * Copy the buffer contents and metadata from \a src to this buffer. The
>> - * destination FrameBuffer shall have the same number of planes as the source
>> - * buffer, and each destination plane shall be larger than or equal to the
>> - * corresponding source plane.
>> - *
>> - * The complete metadata of the source buffer is copied to the destination
>> - * buffer. If an error occurs during the copy, the destination buffer's metadata
>> - * status is set to FrameMetadata::FrameError, and other metadata fields are not
>> - * modified.
>> - *
>> - * The operation is performed using memcpy() so is very slow, users needs to
>> - * consider this before copying buffers.
>> - *
>> - * \return 0 on success or a negative error code otherwise
>> - */
>> -int FrameBuffer::copyFrom(const FrameBuffer *src)
>> -{
>> - if (planes_.size() != src->planes_.size()) {
>> - LOG(Buffer, Error) << "Different number of planes";
>> - metadata_.status = FrameMetadata::FrameError;
>> - return -EINVAL;
>> - }
>> -
>> - for (unsigned int i = 0; i < planes_.size(); i++) {
>> - if (planes_[i].length < src->planes_[i].length) {
>> - LOG(Buffer, Error) << "Plane " << i << " is too small";
>> - metadata_.status = FrameMetadata::FrameError;
>> - return -EINVAL;
>> - }
>> - }
>> -
>> - MappedFrameBuffer source(src, PROT_READ);
>> - MappedFrameBuffer destination(this, PROT_WRITE);
>> -
>> - if (!source.isValid()) {
>> - LOG(Buffer, Error) << "Failed to map source planes";
>> - return -EINVAL;
>> - }
>> -
>> - if (!destination.isValid()) {
>> - LOG(Buffer, Error) << "Failed to map destination planes";
>> - return -EINVAL;
>> - }
>> -
>> - for (unsigned int i = 0; i < planes_.size(); i++) {
>> - memcpy(destination.maps()[i].data(),
>> - source.maps()[i].data(),
>> - source.maps()[i].size());
>> - }
>> -
>> - metadata_ = src->metadata_;
>> -
>> - return 0;
>> -}
>> -
>> /**
>> * \class MappedBuffer
>> * \brief Provide an interface to support managing memory mapped buffers
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list