| Message ID | 20200720224232.153717-3-kieran.bingham@ideasonboard.com |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Mon, Jul 20, 2020 at 11:42:26PM +0100, Kieran Bingham wrote: > Utilise the new MappedFrameBuffer helper to handle all mapping and > unmapping of the copyFrom helper function. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > > This shows how the MappedFrameBuffer can then be utilised, even though I > believe this particular copyFrom() function is expected to disappear > sometime soon. > > src/libcamera/buffer.cpp | 37 ++++++++++++++----------------------- > 1 file changed, 14 insertions(+), 23 deletions(-) > > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp > index 28b43d937f57..af5345b91195 100644 > --- a/src/libcamera/buffer.cpp > +++ b/src/libcamera/buffer.cpp > @@ -257,32 +257,23 @@ int FrameBuffer::copyFrom(const FrameBuffer *src) > } > } > > - for (unsigned int i = 0; i < planes_.size(); i++) { > - void *dstmem = mmap(nullptr, planes_[i].length, PROT_WRITE, > - MAP_SHARED, planes_[i].fd.fd(), 0); > + MappedFrameBuffer source(src, PROT_READ); > + MappedFrameBuffer destination(this, PROT_WRITE); > > - if (dstmem == MAP_FAILED) { > - LOG(Buffer, Error) > - << "Failed to map destination plane " << i; > - metadata_.status = FrameMetadata::FrameError; > - return -EINVAL; > - } > - > - void *srcmem = mmap(nullptr, src->planes_[i].length, PROT_READ, > - MAP_SHARED, src->planes_[i].fd.fd(), 0); > - > - if (srcmem == MAP_FAILED) { > - munmap(dstmem, planes_[i].length); > - LOG(Buffer, Error) > - << "Failed to map source plane " << i; > - metadata_.status = FrameMetadata::FrameError; > - return -EINVAL; > - } > + if (!source.isValid()) { > + LOG(Buffer, Error) << "Failed to map source planes"; > + return -EINVAL; > + } > > - memcpy(dstmem, srcmem, src->planes_[i].length); > + if (!destination.isValid()) { > + LOG(Buffer, Error) << "Failed to map destination planes"; > + return -EINVAL; > + } > > - munmap(srcmem, src->planes_[i].length); > - munmap(dstmem, planes_[i].length); > + for (unsigned int i = 0; i < planes_.size(); i++) { > + memcpy(destination.maps()[i].address, > + source.maps()[i].address, > + source.maps()[i].length); > } > > metadata_ = src->metadata_;
diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 28b43d937f57..af5345b91195 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -257,32 +257,23 @@ int FrameBuffer::copyFrom(const FrameBuffer *src) } } - for (unsigned int i = 0; i < planes_.size(); i++) { - void *dstmem = mmap(nullptr, planes_[i].length, PROT_WRITE, - MAP_SHARED, planes_[i].fd.fd(), 0); + MappedFrameBuffer source(src, PROT_READ); + MappedFrameBuffer destination(this, PROT_WRITE); - if (dstmem == MAP_FAILED) { - LOG(Buffer, Error) - << "Failed to map destination plane " << i; - metadata_.status = FrameMetadata::FrameError; - return -EINVAL; - } - - void *srcmem = mmap(nullptr, src->planes_[i].length, PROT_READ, - MAP_SHARED, src->planes_[i].fd.fd(), 0); - - if (srcmem == MAP_FAILED) { - munmap(dstmem, planes_[i].length); - LOG(Buffer, Error) - << "Failed to map source plane " << i; - metadata_.status = FrameMetadata::FrameError; - return -EINVAL; - } + if (!source.isValid()) { + LOG(Buffer, Error) << "Failed to map source planes"; + return -EINVAL; + } - memcpy(dstmem, srcmem, src->planes_[i].length); + if (!destination.isValid()) { + LOG(Buffer, Error) << "Failed to map destination planes"; + return -EINVAL; + } - munmap(srcmem, src->planes_[i].length); - munmap(dstmem, planes_[i].length); + for (unsigned int i = 0; i < planes_.size(); i++) { + memcpy(destination.maps()[i].address, + source.maps()[i].address, + source.maps()[i].length); } metadata_ = src->metadata_;
Utilise the new MappedFrameBuffer helper to handle all mapping and unmapping of the copyFrom helper function. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- This shows how the MappedFrameBuffer can then be utilised, even though I believe this particular copyFrom() function is expected to disappear sometime soon. src/libcamera/buffer.cpp | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-)