| Message ID | 20210818002208.13631-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 6f09680b256025923cedd50b5fd1a878af2dffd4 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Wed, Aug 18, 2021 at 03:22:08AM +0300, Laurent Pinchart wrote: > The MappedBuffer structure is a custom container that binds a data > pointer with a length. This is exactly what Span is. Use it instead. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/qcam/main_window.cpp | 17 +++++++++-------- > src/qcam/main_window.h | 2 +- > src/qcam/viewfinder.h | 9 +++------ > src/qcam/viewfinder_gl.cpp | 5 +++-- > src/qcam/viewfinder_gl.h | 2 +- > src/qcam/viewfinder_qt.cpp | 5 +++-- > src/qcam/viewfinder_qt.h | 2 +- > 7 files changed, 21 insertions(+), 21 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 39d034de6bb2..1adaae60d83b 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -474,7 +474,8 @@ int MainWindow::startCapture() > const FrameBuffer::Plane &plane = buffer->planes().front(); > void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, > plane.fd.fd(), 0); > - mappedBuffers_[buffer.get()] = { memory, plane.length }; > + mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), > + plane.length }; > > /* Store buffers on the free list. */ > freeBuffers_[stream].enqueue(buffer.get()); > @@ -537,8 +538,8 @@ error: > requests_.clear(); > > for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > + const Span<uint8_t> &buffer = iter.second; > + munmap(buffer.data(), buffer.size()); > } > mappedBuffers_.clear(); > > @@ -573,8 +574,8 @@ void MainWindow::stopCapture() > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); > > for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > + const Span<uint8_t> &buffer = iter.second; > + munmap(buffer.data(), buffer.size()); > } > mappedBuffers_.clear(); > > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, > "DNG Files (*.dng)"); > > if (!filename.isEmpty()) { > - const MappedBuffer &mapped = mappedBuffers_[buffer]; > + const Span<uint8_t> &mapped = mappedBuffers_[buffer]; > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > rawStream_->configuration(), metadata, buffer, > - mapped.memory); > + mapped.data()); > } > #endif > > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) > << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; > > /* Render the frame on the viewfinder. */ > - viewfinder_->render(buffer, &mappedBuffers_[buffer]); > + viewfinder_->render(buffer, mappedBuffers_[buffer]); > } > > void MainWindow::queueRequest(FrameBuffer *buffer) > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index 85d56ce49abe..6788de8ddde9 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -106,7 +106,7 @@ private: > FrameBufferAllocator *allocator_; > > std::unique_ptr<CameraConfiguration> config_; > - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; > + std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; > > /* Capture state, buffers queue and statistics */ > bool isCapturing_; > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > index 46747c227d0c..42d40f1f33f0 100644 > --- a/src/qcam/viewfinder.h > +++ b/src/qcam/viewfinder.h > @@ -11,14 +11,11 @@ > #include <QList> > #include <QSize> > > +#include <libcamera/base/span.h> > + > #include <libcamera/formats.h> > #include <libcamera/framebuffer.h> > > -struct MappedBuffer { > - void *memory; > - size_t size; > -}; > - > class ViewFinder > { > public: > @@ -27,7 +24,7 @@ public: > virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0; > > virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; > - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; > + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) = 0; > virtual void stop() = 0; > > virtual QImage getCurrentImage() = 0; > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index add87db88207..40226601f9fd 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage() > return grabFramebuffer(); > } > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > + libcamera::Span<uint8_t> mem) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > if (buffer_) > renderComplete(buffer_); > > - data_ = static_cast<unsigned char *>(map->memory); > + data_ = mem.data(); > /* > * \todo Get the stride from the buffer instead of computing it naively > */ > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h > index 4a0f8ca58f29..3334549e0be4 100644 > --- a/src/qcam/viewfinder_gl.h > +++ b/src/qcam/viewfinder_gl.h > @@ -39,7 +39,7 @@ public: > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > void stop() override; > > QImage getCurrentImage() override; > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > index e436714c6bdb..efa1d412584b 100644 > --- a/src/qcam/viewfinder_qt.cpp > +++ b/src/qcam/viewfinder_qt.cpp > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, > return 0; > } > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, > + libcamera::Span<uint8_t> mem) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > return; > } > > - unsigned char *memory = static_cast<unsigned char *>(map->memory); > + unsigned char *memory = mem.data(); > size_t size = buffer->metadata().planes[0].bytesused; > > { > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > index 501c72a70dec..1a569b9cee6e 100644 > --- a/src/qcam/viewfinder_qt.h > +++ b/src/qcam/viewfinder_qt.h > @@ -32,7 +32,7 @@ public: > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > void stop() override; > > QImage getCurrentImage() override; > -- > Regards, > > Laurent Pinchart >
Hi Laurent, Thank you for the patch. On 18/08/2021 02:22, Laurent Pinchart wrote: > The MappedBuffer structure is a custom container that binds a data > pointer with a length. This is exactly what Span is. Use it instead. > Could you explain why it is better with a Span than with a buffer and length ? As far as I can tell, it is only a matter of how it is declared :-) ? > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/qcam/main_window.cpp | 17 +++++++++-------- > src/qcam/main_window.h | 2 +- > src/qcam/viewfinder.h | 9 +++------ > src/qcam/viewfinder_gl.cpp | 5 +++-- > src/qcam/viewfinder_gl.h | 2 +- > src/qcam/viewfinder_qt.cpp | 5 +++-- > src/qcam/viewfinder_qt.h | 2 +- > 7 files changed, 21 insertions(+), 21 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 39d034de6bb2..1adaae60d83b 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -474,7 +474,8 @@ int MainWindow::startCapture() > const FrameBuffer::Plane &plane = buffer->planes().front(); > void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, > plane.fd.fd(), 0); > - mappedBuffers_[buffer.get()] = { memory, plane.length }; > + mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), > + plane.length }; > > /* Store buffers on the free list. */ > freeBuffers_[stream].enqueue(buffer.get()); > @@ -537,8 +538,8 @@ error: > requests_.clear(); > > for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > + const Span<uint8_t> &buffer = iter.second; > + munmap(buffer.data(), buffer.size()); > } > mappedBuffers_.clear(); > > @@ -573,8 +574,8 @@ void MainWindow::stopCapture() > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); > > for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > + const Span<uint8_t> &buffer = iter.second; > + munmap(buffer.data(), buffer.size()); > } > mappedBuffers_.clear(); > > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, > "DNG Files (*.dng)"); > > if (!filename.isEmpty()) { > - const MappedBuffer &mapped = mappedBuffers_[buffer]; > + const Span<uint8_t> &mapped = mappedBuffers_[buffer]; > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > rawStream_->configuration(), metadata, buffer, > - mapped.memory); > + mapped.data()); > } > #endif > > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) > << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; > > /* Render the frame on the viewfinder. */ > - viewfinder_->render(buffer, &mappedBuffers_[buffer]); > + viewfinder_->render(buffer, mappedBuffers_[buffer]); > } > > void MainWindow::queueRequest(FrameBuffer *buffer) > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index 85d56ce49abe..6788de8ddde9 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -106,7 +106,7 @@ private: > FrameBufferAllocator *allocator_; > > std::unique_ptr<CameraConfiguration> config_; > - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; > + std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; > > /* Capture state, buffers queue and statistics */ > bool isCapturing_; > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > index 46747c227d0c..42d40f1f33f0 100644 > --- a/src/qcam/viewfinder.h > +++ b/src/qcam/viewfinder.h > @@ -11,14 +11,11 @@ > #include <QList> > #include <QSize> > > +#include <libcamera/base/span.h> > + > #include <libcamera/formats.h> > #include <libcamera/framebuffer.h> > > -struct MappedBuffer { > - void *memory; > - size_t size; > -}; > - > class ViewFinder > { > public: > @@ -27,7 +24,7 @@ public: > virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0; > > virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; > - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; > + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) = 0; > virtual void stop() = 0; > > virtual QImage getCurrentImage() = 0; > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index add87db88207..40226601f9fd 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage() > return grabFramebuffer(); > } > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > + libcamera::Span<uint8_t> mem) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > if (buffer_) > renderComplete(buffer_); > > - data_ = static_cast<unsigned char *>(map->memory); > + data_ = mem.data(); > /* > * \todo Get the stride from the buffer instead of computing it naively > */ > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h > index 4a0f8ca58f29..3334549e0be4 100644 > --- a/src/qcam/viewfinder_gl.h > +++ b/src/qcam/viewfinder_gl.h > @@ -39,7 +39,7 @@ public: > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > void stop() override; > > QImage getCurrentImage() override; > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > index e436714c6bdb..efa1d412584b 100644 > --- a/src/qcam/viewfinder_qt.cpp > +++ b/src/qcam/viewfinder_qt.cpp > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, > return 0; > } > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, > + libcamera::Span<uint8_t> mem) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > return; > } > > - unsigned char *memory = static_cast<unsigned char *>(map->memory); > + unsigned char *memory = mem.data(); > size_t size = buffer->metadata().planes[0].bytesused; > > { > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > index 501c72a70dec..1a569b9cee6e 100644 > --- a/src/qcam/viewfinder_qt.h > +++ b/src/qcam/viewfinder_qt.h > @@ -32,7 +32,7 @@ public: > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > void stop() override; > > QImage getCurrentImage() override; >
Hi Jean-Michel, On Wed, Aug 18, 2021 at 07:29:36AM +0200, Jean-Michel Hautbois wrote: > On 18/08/2021 02:22, Laurent Pinchart wrote: > > The MappedBuffer structure is a custom container that binds a data > > pointer with a length. This is exactly what Span is. Use it instead. > > Could you explain why it is better with a Span than with a buffer and > length ? As far as I can tell, it is only a matter of how it is declared > :-) ? Because a span *is* a buffer pointer and length. It's a standard class that groups both fields, so it's better than a ad-hoc class that does the same. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/qcam/main_window.cpp | 17 +++++++++-------- > > src/qcam/main_window.h | 2 +- > > src/qcam/viewfinder.h | 9 +++------ > > src/qcam/viewfinder_gl.cpp | 5 +++-- > > src/qcam/viewfinder_gl.h | 2 +- > > src/qcam/viewfinder_qt.cpp | 5 +++-- > > src/qcam/viewfinder_qt.h | 2 +- > > 7 files changed, 21 insertions(+), 21 deletions(-) > > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > index 39d034de6bb2..1adaae60d83b 100644 > > --- a/src/qcam/main_window.cpp > > +++ b/src/qcam/main_window.cpp > > @@ -474,7 +474,8 @@ int MainWindow::startCapture() > > const FrameBuffer::Plane &plane = buffer->planes().front(); > > void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, > > plane.fd.fd(), 0); > > - mappedBuffers_[buffer.get()] = { memory, plane.length }; > > + mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), > > + plane.length }; > > > > /* Store buffers on the free list. */ > > freeBuffers_[stream].enqueue(buffer.get()); > > @@ -537,8 +538,8 @@ error: > > requests_.clear(); > > > > for (auto &iter : mappedBuffers_) { > > - const MappedBuffer &buffer = iter.second; > > - munmap(buffer.memory, buffer.size); > > + const Span<uint8_t> &buffer = iter.second; > > + munmap(buffer.data(), buffer.size()); > > } > > mappedBuffers_.clear(); > > > > @@ -573,8 +574,8 @@ void MainWindow::stopCapture() > > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); > > > > for (auto &iter : mappedBuffers_) { > > - const MappedBuffer &buffer = iter.second; > > - munmap(buffer.memory, buffer.size); > > + const Span<uint8_t> &buffer = iter.second; > > + munmap(buffer.data(), buffer.size()); > > } > > mappedBuffers_.clear(); > > > > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, > > "DNG Files (*.dng)"); > > > > if (!filename.isEmpty()) { > > - const MappedBuffer &mapped = mappedBuffers_[buffer]; > > + const Span<uint8_t> &mapped = mappedBuffers_[buffer]; > > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > > rawStream_->configuration(), metadata, buffer, > > - mapped.memory); > > + mapped.data()); > > } > > #endif > > > > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) > > << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; > > > > /* Render the frame on the viewfinder. */ > > - viewfinder_->render(buffer, &mappedBuffers_[buffer]); > > + viewfinder_->render(buffer, mappedBuffers_[buffer]); > > } > > > > void MainWindow::queueRequest(FrameBuffer *buffer) > > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > > index 85d56ce49abe..6788de8ddde9 100644 > > --- a/src/qcam/main_window.h > > +++ b/src/qcam/main_window.h > > @@ -106,7 +106,7 @@ private: > > FrameBufferAllocator *allocator_; > > > > std::unique_ptr<CameraConfiguration> config_; > > - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; > > + std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; > > > > /* Capture state, buffers queue and statistics */ > > bool isCapturing_; > > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > > index 46747c227d0c..42d40f1f33f0 100644 > > --- a/src/qcam/viewfinder.h > > +++ b/src/qcam/viewfinder.h > > @@ -11,14 +11,11 @@ > > #include <QList> > > #include <QSize> > > > > +#include <libcamera/base/span.h> > > + > > #include <libcamera/formats.h> > > #include <libcamera/framebuffer.h> > > > > -struct MappedBuffer { > > - void *memory; > > - size_t size; > > -}; > > - > > class ViewFinder > > { > > public: > > @@ -27,7 +24,7 @@ public: > > virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0; > > > > virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; > > - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; > > + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) = 0; > > virtual void stop() = 0; > > > > virtual QImage getCurrentImage() = 0; > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > > index add87db88207..40226601f9fd 100644 > > --- a/src/qcam/viewfinder_gl.cpp > > +++ b/src/qcam/viewfinder_gl.cpp > > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage() > > return grabFramebuffer(); > > } > > > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > > + libcamera::Span<uint8_t> mem) > > { > > if (buffer->planes().size() != 1) { > > qWarning() << "Multi-planar buffers are not supported"; > > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > if (buffer_) > > renderComplete(buffer_); > > > > - data_ = static_cast<unsigned char *>(map->memory); > > + data_ = mem.data(); > > /* > > * \todo Get the stride from the buffer instead of computing it naively > > */ > > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h > > index 4a0f8ca58f29..3334549e0be4 100644 > > --- a/src/qcam/viewfinder_gl.h > > +++ b/src/qcam/viewfinder_gl.h > > @@ -39,7 +39,7 @@ public: > > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > > void stop() override; > > > > QImage getCurrentImage() override; > > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > > index e436714c6bdb..efa1d412584b 100644 > > --- a/src/qcam/viewfinder_qt.cpp > > +++ b/src/qcam/viewfinder_qt.cpp > > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, > > return 0; > > } > > > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, > > + libcamera::Span<uint8_t> mem) > > { > > if (buffer->planes().size() != 1) { > > qWarning() << "Multi-planar buffers are not supported"; > > return; > > } > > > > - unsigned char *memory = static_cast<unsigned char *>(map->memory); > > + unsigned char *memory = mem.data(); > > size_t size = buffer->metadata().planes[0].bytesused; > > > > { > > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > > index 501c72a70dec..1a569b9cee6e 100644 > > --- a/src/qcam/viewfinder_qt.h > > +++ b/src/qcam/viewfinder_qt.h > > @@ -32,7 +32,7 @@ public: > > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > > void stop() override; > > > > QImage getCurrentImage() override;
On 18/08/2021 01:22, Laurent Pinchart wrote: > The MappedBuffer structure is a custom container that binds a data > pointer with a length. This is exactly what Span is. Use it instead. It also potentially conflicts (thought different namespaces, and code tree so it's fine) with the one in mapped_framebuffer.h - so I'm happy to see this change. Of course I see potential to use MappedFrameBuffer in the future, if it ever gets exposed as an API (either through libcamera, or a helpers library) to avoid duplication of support for mapping buffers, and allow support for multiplanar formats... but that's all futureware so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/qcam/main_window.cpp | 17 +++++++++-------- > src/qcam/main_window.h | 2 +- > src/qcam/viewfinder.h | 9 +++------ > src/qcam/viewfinder_gl.cpp | 5 +++-- > src/qcam/viewfinder_gl.h | 2 +- > src/qcam/viewfinder_qt.cpp | 5 +++-- > src/qcam/viewfinder_qt.h | 2 +- > 7 files changed, 21 insertions(+), 21 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 39d034de6bb2..1adaae60d83b 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -474,7 +474,8 @@ int MainWindow::startCapture() > const FrameBuffer::Plane &plane = buffer->planes().front(); > void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, > plane.fd.fd(), 0); > - mappedBuffers_[buffer.get()] = { memory, plane.length }; > + mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), > + plane.length }; > > /* Store buffers on the free list. */ > freeBuffers_[stream].enqueue(buffer.get()); > @@ -537,8 +538,8 @@ error: > requests_.clear(); > > for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > + const Span<uint8_t> &buffer = iter.second; > + munmap(buffer.data(), buffer.size()); > } > mappedBuffers_.clear(); > > @@ -573,8 +574,8 @@ void MainWindow::stopCapture() > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); > > for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > + const Span<uint8_t> &buffer = iter.second; > + munmap(buffer.data(), buffer.size()); > } > mappedBuffers_.clear(); > > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, > "DNG Files (*.dng)"); > > if (!filename.isEmpty()) { > - const MappedBuffer &mapped = mappedBuffers_[buffer]; > + const Span<uint8_t> &mapped = mappedBuffers_[buffer]; > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > rawStream_->configuration(), metadata, buffer, > - mapped.memory); > + mapped.data()); > } > #endif > > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) > << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; > > /* Render the frame on the viewfinder. */ > - viewfinder_->render(buffer, &mappedBuffers_[buffer]); > + viewfinder_->render(buffer, mappedBuffers_[buffer]); > } > > void MainWindow::queueRequest(FrameBuffer *buffer) > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index 85d56ce49abe..6788de8ddde9 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -106,7 +106,7 @@ private: > FrameBufferAllocator *allocator_; > > std::unique_ptr<CameraConfiguration> config_; > - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; > + std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; > > /* Capture state, buffers queue and statistics */ > bool isCapturing_; > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > index 46747c227d0c..42d40f1f33f0 100644 > --- a/src/qcam/viewfinder.h > +++ b/src/qcam/viewfinder.h > @@ -11,14 +11,11 @@ > #include <QList> > #include <QSize> > > +#include <libcamera/base/span.h> > + > #include <libcamera/formats.h> > #include <libcamera/framebuffer.h> > > -struct MappedBuffer { > - void *memory; > - size_t size; > -}; > - > class ViewFinder > { > public: > @@ -27,7 +24,7 @@ public: > virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0; > > virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; > - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; > + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) = 0; > virtual void stop() = 0; > > virtual QImage getCurrentImage() = 0; > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index add87db88207..40226601f9fd 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage() > return grabFramebuffer(); > } > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > + libcamera::Span<uint8_t> mem) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > if (buffer_) > renderComplete(buffer_); > > - data_ = static_cast<unsigned char *>(map->memory); > + data_ = mem.data(); > /* > * \todo Get the stride from the buffer instead of computing it naively > */ > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h > index 4a0f8ca58f29..3334549e0be4 100644 > --- a/src/qcam/viewfinder_gl.h > +++ b/src/qcam/viewfinder_gl.h > @@ -39,7 +39,7 @@ public: > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > void stop() override; > > QImage getCurrentImage() override; > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > index e436714c6bdb..efa1d412584b 100644 > --- a/src/qcam/viewfinder_qt.cpp > +++ b/src/qcam/viewfinder_qt.cpp > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, > return 0; > } > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, > + libcamera::Span<uint8_t> mem) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > return; > } > > - unsigned char *memory = static_cast<unsigned char *>(map->memory); > + unsigned char *memory = mem.data(); > size_t size = buffer->metadata().planes[0].bytesused; > > { > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > index 501c72a70dec..1a569b9cee6e 100644 > --- a/src/qcam/viewfinder_qt.h > +++ b/src/qcam/viewfinder_qt.h > @@ -32,7 +32,7 @@ public: > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > void stop() override; > > QImage getCurrentImage() override; >
Hi Kieran, On Wed, Aug 18, 2021 at 10:02:40AM +0100, Kieran Bingham wrote: > On 18/08/2021 01:22, Laurent Pinchart wrote: > > The MappedBuffer structure is a custom container that binds a data > > pointer with a length. This is exactly what Span is. Use it instead. > > It also potentially conflicts (thought different namespaces, and code > tree so it's fine) with the one in mapped_framebuffer.h - so I'm happy > to see this change. > > Of course I see potential to use MappedFrameBuffer in the future, if it > ever gets exposed as an API (either through libcamera, or a helpers > library) to avoid duplication of support for mapping buffers, and allow > support for multiplanar formats... but that's all futureware so: I have an increasingly strong feeling it will happen one way or another :-) > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/qcam/main_window.cpp | 17 +++++++++-------- > > src/qcam/main_window.h | 2 +- > > src/qcam/viewfinder.h | 9 +++------ > > src/qcam/viewfinder_gl.cpp | 5 +++-- > > src/qcam/viewfinder_gl.h | 2 +- > > src/qcam/viewfinder_qt.cpp | 5 +++-- > > src/qcam/viewfinder_qt.h | 2 +- > > 7 files changed, 21 insertions(+), 21 deletions(-) > > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > index 39d034de6bb2..1adaae60d83b 100644 > > --- a/src/qcam/main_window.cpp > > +++ b/src/qcam/main_window.cpp > > @@ -474,7 +474,8 @@ int MainWindow::startCapture() > > const FrameBuffer::Plane &plane = buffer->planes().front(); > > void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, > > plane.fd.fd(), 0); > > - mappedBuffers_[buffer.get()] = { memory, plane.length }; > > + mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), > > + plane.length }; > > > > /* Store buffers on the free list. */ > > freeBuffers_[stream].enqueue(buffer.get()); > > @@ -537,8 +538,8 @@ error: > > requests_.clear(); > > > > for (auto &iter : mappedBuffers_) { > > - const MappedBuffer &buffer = iter.second; > > - munmap(buffer.memory, buffer.size); > > + const Span<uint8_t> &buffer = iter.second; > > + munmap(buffer.data(), buffer.size()); > > } > > mappedBuffers_.clear(); > > > > @@ -573,8 +574,8 @@ void MainWindow::stopCapture() > > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); > > > > for (auto &iter : mappedBuffers_) { > > - const MappedBuffer &buffer = iter.second; > > - munmap(buffer.memory, buffer.size); > > + const Span<uint8_t> &buffer = iter.second; > > + munmap(buffer.data(), buffer.size()); > > } > > mappedBuffers_.clear(); > > > > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, > > "DNG Files (*.dng)"); > > > > if (!filename.isEmpty()) { > > - const MappedBuffer &mapped = mappedBuffers_[buffer]; > > + const Span<uint8_t> &mapped = mappedBuffers_[buffer]; > > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > > rawStream_->configuration(), metadata, buffer, > > - mapped.memory); > > + mapped.data()); > > } > > #endif > > > > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) > > << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; > > > > /* Render the frame on the viewfinder. */ > > - viewfinder_->render(buffer, &mappedBuffers_[buffer]); > > + viewfinder_->render(buffer, mappedBuffers_[buffer]); > > } > > > > void MainWindow::queueRequest(FrameBuffer *buffer) > > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > > index 85d56ce49abe..6788de8ddde9 100644 > > --- a/src/qcam/main_window.h > > +++ b/src/qcam/main_window.h > > @@ -106,7 +106,7 @@ private: > > FrameBufferAllocator *allocator_; > > > > std::unique_ptr<CameraConfiguration> config_; > > - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; > > + std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; > > > > /* Capture state, buffers queue and statistics */ > > bool isCapturing_; > > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > > index 46747c227d0c..42d40f1f33f0 100644 > > --- a/src/qcam/viewfinder.h > > +++ b/src/qcam/viewfinder.h > > @@ -11,14 +11,11 @@ > > #include <QList> > > #include <QSize> > > > > +#include <libcamera/base/span.h> > > + > > #include <libcamera/formats.h> > > #include <libcamera/framebuffer.h> > > > > -struct MappedBuffer { > > - void *memory; > > - size_t size; > > -}; > > - > > class ViewFinder > > { > > public: > > @@ -27,7 +24,7 @@ public: > > virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0; > > > > virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; > > - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; > > + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) = 0; > > virtual void stop() = 0; > > > > virtual QImage getCurrentImage() = 0; > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > > index add87db88207..40226601f9fd 100644 > > --- a/src/qcam/viewfinder_gl.cpp > > +++ b/src/qcam/viewfinder_gl.cpp > > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage() > > return grabFramebuffer(); > > } > > > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > > + libcamera::Span<uint8_t> mem) > > { > > if (buffer->planes().size() != 1) { > > qWarning() << "Multi-planar buffers are not supported"; > > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > if (buffer_) > > renderComplete(buffer_); > > > > - data_ = static_cast<unsigned char *>(map->memory); > > + data_ = mem.data(); > > /* > > * \todo Get the stride from the buffer instead of computing it naively > > */ > > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h > > index 4a0f8ca58f29..3334549e0be4 100644 > > --- a/src/qcam/viewfinder_gl.h > > +++ b/src/qcam/viewfinder_gl.h > > @@ -39,7 +39,7 @@ public: > > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > > void stop() override; > > > > QImage getCurrentImage() override; > > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > > index e436714c6bdb..efa1d412584b 100644 > > --- a/src/qcam/viewfinder_qt.cpp > > +++ b/src/qcam/viewfinder_qt.cpp > > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, > > return 0; > > } > > > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, > > + libcamera::Span<uint8_t> mem) > > { > > if (buffer->planes().size() != 1) { > > qWarning() << "Multi-planar buffers are not supported"; > > return; > > } > > > > - unsigned char *memory = static_cast<unsigned char *>(map->memory); > > + unsigned char *memory = mem.data(); > > size_t size = buffer->metadata().planes[0].bytesused; > > > > { > > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > > index 501c72a70dec..1a569b9cee6e 100644 > > --- a/src/qcam/viewfinder_qt.h > > +++ b/src/qcam/viewfinder_qt.h > > @@ -32,7 +32,7 @@ public: > > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > > void stop() override; > > > > QImage getCurrentImage() override;
Hi Laurent, thank you for the patch. On Wed, Aug 18, 2021 at 6:08 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Kieran, > > On Wed, Aug 18, 2021 at 10:02:40AM +0100, Kieran Bingham wrote: > > On 18/08/2021 01:22, Laurent Pinchart wrote: > > > The MappedBuffer structure is a custom container that binds a data > > > pointer with a length. This is exactly what Span is. Use it instead. > > > > It also potentially conflicts (thought different namespaces, and code > > tree so it's fine) with the one in mapped_framebuffer.h - so I'm happy > > to see this change. > > > > Of course I see potential to use MappedFrameBuffer in the future, if it > > ever gets exposed as an API (either through libcamera, or a helpers > > library) to avoid duplication of support for mapping buffers, and allow > > support for multiplanar formats... but that's all futureware so: > > I have an increasingly strong feeling it will happen one way or another > :-) > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > --- > > > src/qcam/main_window.cpp | 17 +++++++++-------- > > > src/qcam/main_window.h | 2 +- > > > src/qcam/viewfinder.h | 9 +++------ > > > src/qcam/viewfinder_gl.cpp | 5 +++-- > > > src/qcam/viewfinder_gl.h | 2 +- > > > src/qcam/viewfinder_qt.cpp | 5 +++-- > > > src/qcam/viewfinder_qt.h | 2 +- > > > 7 files changed, 21 insertions(+), 21 deletions(-) > > > > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > > index 39d034de6bb2..1adaae60d83b 100644 > > > --- a/src/qcam/main_window.cpp > > > +++ b/src/qcam/main_window.cpp > > > @@ -474,7 +474,8 @@ int MainWindow::startCapture() > > > const FrameBuffer::Plane &plane = buffer->planes().front(); > > > void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, > > > plane.fd.fd(), 0); > > > - mappedBuffers_[buffer.get()] = { memory, plane.length }; > > > + mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), > > > + plane.length }; > > > > > > /* Store buffers on the free list. */ > > > freeBuffers_[stream].enqueue(buffer.get()); > > > @@ -537,8 +538,8 @@ error: > > > requests_.clear(); > > > > > > for (auto &iter : mappedBuffers_) { > > > - const MappedBuffer &buffer = iter.second; > > > - munmap(buffer.memory, buffer.size); > > > + const Span<uint8_t> &buffer = iter.second; > > > + munmap(buffer.data(), buffer.size()); > > > } > > > mappedBuffers_.clear(); > > > > > > @@ -573,8 +574,8 @@ void MainWindow::stopCapture() > > > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); > > > > > > for (auto &iter : mappedBuffers_) { > > > - const MappedBuffer &buffer = iter.second; > > > - munmap(buffer.memory, buffer.size); > > > + const Span<uint8_t> &buffer = iter.second; > > > + munmap(buffer.data(), buffer.size()); > > > } > > > mappedBuffers_.clear(); > > > > > > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, > > > "DNG Files (*.dng)"); > > > > > > if (!filename.isEmpty()) { > > > - const MappedBuffer &mapped = mappedBuffers_[buffer]; > > > + const Span<uint8_t> &mapped = mappedBuffers_[buffer]; > > > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > > > rawStream_->configuration(), metadata, buffer, > > > - mapped.memory); > > > + mapped.data()); > > > } > > > #endif > > > > > > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) > > > << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; > > > > > > /* Render the frame on the viewfinder. */ > > > - viewfinder_->render(buffer, &mappedBuffers_[buffer]); > > > + viewfinder_->render(buffer, mappedBuffers_[buffer]); > > > } > > > > > > void MainWindow::queueRequest(FrameBuffer *buffer) > > > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > > > index 85d56ce49abe..6788de8ddde9 100644 > > > --- a/src/qcam/main_window.h > > > +++ b/src/qcam/main_window.h > > > @@ -106,7 +106,7 @@ private: > > > FrameBufferAllocator *allocator_; > > > > > > std::unique_ptr<CameraConfiguration> config_; > > > - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; > > > + std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; > > > > > > /* Capture state, buffers queue and statistics */ > > > bool isCapturing_; > > > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > > > index 46747c227d0c..42d40f1f33f0 100644 > > > --- a/src/qcam/viewfinder.h > > > +++ b/src/qcam/viewfinder.h > > > @@ -11,14 +11,11 @@ > > > #include <QList> > > > #include <QSize> > > > > > > +#include <libcamera/base/span.h> > > > + > > > #include <libcamera/formats.h> > > > #include <libcamera/framebuffer.h> > > > > > > -struct MappedBuffer { > > > - void *memory; > > > - size_t size; > > > -}; > > > - > > > class ViewFinder > > > { > > > public: > > > @@ -27,7 +24,7 @@ public: > > > virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0; > > > > > > virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; > > > - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; > > > + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) = 0; > > > virtual void stop() = 0; > > > > > > virtual QImage getCurrentImage() = 0; > > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > > > index add87db88207..40226601f9fd 100644 > > > --- a/src/qcam/viewfinder_gl.cpp > > > +++ b/src/qcam/viewfinder_gl.cpp > > > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage() > > > return grabFramebuffer(); > > > } > > > > > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > > > + libcamera::Span<uint8_t> mem) > > > { > > > if (buffer->planes().size() != 1) { > > > qWarning() << "Multi-planar buffers are not supported"; > > > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > > if (buffer_) > > > renderComplete(buffer_); > > > > > > - data_ = static_cast<unsigned char *>(map->memory); > > > + data_ = mem.data(); > > > /* > > > * \todo Get the stride from the buffer instead of computing it naively > > > */ > > > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h > > > index 4a0f8ca58f29..3334549e0be4 100644 > > > --- a/src/qcam/viewfinder_gl.h > > > +++ b/src/qcam/viewfinder_gl.h > > > @@ -39,7 +39,7 @@ public: > > > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > > > > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > > > void stop() override; > > > > > > QImage getCurrentImage() override; > > > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > > > index e436714c6bdb..efa1d412584b 100644 > > > --- a/src/qcam/viewfinder_qt.cpp > > > +++ b/src/qcam/viewfinder_qt.cpp > > > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, > > > return 0; > > > } > > > > > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > > > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, > > > + libcamera::Span<uint8_t> mem) > > > { > > > if (buffer->planes().size() != 1) { > > > qWarning() << "Multi-planar buffers are not supported"; > > > return; > > > } > > > > > > - unsigned char *memory = static_cast<unsigned char *>(map->memory); > > > + unsigned char *memory = mem.data(); > > > size_t size = buffer->metadata().planes[0].bytesused; > > > > > > { > > > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > > > index 501c72a70dec..1a569b9cee6e 100644 > > > --- a/src/qcam/viewfinder_qt.h > > > +++ b/src/qcam/viewfinder_qt.h > > > @@ -32,7 +32,7 @@ public: > > > const QList<libcamera::PixelFormat> &nativeFormats() const override; > > > > > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; > > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; > > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; > > > void stop() override; > > > > > > QImage getCurrentImage() override; > > -- > Regards, > > Laurent Pinchart
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 39d034de6bb2..1adaae60d83b 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -474,7 +474,8 @@ int MainWindow::startCapture() const FrameBuffer::Plane &plane = buffer->planes().front(); void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, plane.fd.fd(), 0); - mappedBuffers_[buffer.get()] = { memory, plane.length }; + mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), + plane.length }; /* Store buffers on the free list. */ freeBuffers_[stream].enqueue(buffer.get()); @@ -537,8 +538,8 @@ error: requests_.clear(); for (auto &iter : mappedBuffers_) { - const MappedBuffer &buffer = iter.second; - munmap(buffer.memory, buffer.size); + const Span<uint8_t> &buffer = iter.second; + munmap(buffer.data(), buffer.size()); } mappedBuffers_.clear(); @@ -573,8 +574,8 @@ void MainWindow::stopCapture() camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); for (auto &iter : mappedBuffers_) { - const MappedBuffer &buffer = iter.second; - munmap(buffer.memory, buffer.size); + const Span<uint8_t> &buffer = iter.second; + munmap(buffer.data(), buffer.size()); } mappedBuffers_.clear(); @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, "DNG Files (*.dng)"); if (!filename.isEmpty()) { - const MappedBuffer &mapped = mappedBuffers_[buffer]; + const Span<uint8_t> &mapped = mappedBuffers_[buffer]; DNGWriter::write(filename.toStdString().c_str(), camera_.get(), rawStream_->configuration(), metadata, buffer, - mapped.memory); + mapped.data()); } #endif @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; /* Render the frame on the viewfinder. */ - viewfinder_->render(buffer, &mappedBuffers_[buffer]); + viewfinder_->render(buffer, mappedBuffers_[buffer]); } void MainWindow::queueRequest(FrameBuffer *buffer) diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 85d56ce49abe..6788de8ddde9 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -106,7 +106,7 @@ private: FrameBufferAllocator *allocator_; std::unique_ptr<CameraConfiguration> config_; - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; + std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; /* Capture state, buffers queue and statistics */ bool isCapturing_; diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h index 46747c227d0c..42d40f1f33f0 100644 --- a/src/qcam/viewfinder.h +++ b/src/qcam/viewfinder.h @@ -11,14 +11,11 @@ #include <QList> #include <QSize> +#include <libcamera/base/span.h> + #include <libcamera/formats.h> #include <libcamera/framebuffer.h> -struct MappedBuffer { - void *memory; - size_t size; -}; - class ViewFinder { public: @@ -27,7 +24,7 @@ public: virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0; virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) = 0; virtual void stop() = 0; virtual QImage getCurrentImage() = 0; diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp index add87db88207..40226601f9fd 100644 --- a/src/qcam/viewfinder_gl.cpp +++ b/src/qcam/viewfinder_gl.cpp @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage() return grabFramebuffer(); } -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, + libcamera::Span<uint8_t> mem) { if (buffer->planes().size() != 1) { qWarning() << "Multi-planar buffers are not supported"; @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) if (buffer_) renderComplete(buffer_); - data_ = static_cast<unsigned char *>(map->memory); + data_ = mem.data(); /* * \todo Get the stride from the buffer instead of computing it naively */ diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h index 4a0f8ca58f29..3334549e0be4 100644 --- a/src/qcam/viewfinder_gl.h +++ b/src/qcam/viewfinder_gl.h @@ -39,7 +39,7 @@ public: const QList<libcamera::PixelFormat> &nativeFormats() const override; int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; void stop() override; QImage getCurrentImage() override; diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp index e436714c6bdb..efa1d412584b 100644 --- a/src/qcam/viewfinder_qt.cpp +++ b/src/qcam/viewfinder_qt.cpp @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, return 0; } -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, + libcamera::Span<uint8_t> mem) { if (buffer->planes().size() != 1) { qWarning() << "Multi-planar buffers are not supported"; return; } - unsigned char *memory = static_cast<unsigned char *>(map->memory); + unsigned char *memory = mem.data(); size_t size = buffer->metadata().planes[0].bytesused; { diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h index 501c72a70dec..1a569b9cee6e 100644 --- a/src/qcam/viewfinder_qt.h +++ b/src/qcam/viewfinder_qt.h @@ -32,7 +32,7 @@ public: const QList<libcamera::PixelFormat> &nativeFormats() const override; int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; + void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override; void stop() override; QImage getCurrentImage() override;
The MappedBuffer structure is a custom container that binds a data pointer with a length. This is exactly what Span is. Use it instead. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/qcam/main_window.cpp | 17 +++++++++-------- src/qcam/main_window.h | 2 +- src/qcam/viewfinder.h | 9 +++------ src/qcam/viewfinder_gl.cpp | 5 +++-- src/qcam/viewfinder_gl.h | 2 +- src/qcam/viewfinder_qt.cpp | 5 +++-- src/qcam/viewfinder_qt.h | 2 +- 7 files changed, 21 insertions(+), 21 deletions(-)