| Message ID | 20210906225636.14683-27-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On 06/09/2021 23:56, Laurent Pinchart wrote: > Replace the manual implementation of frame buffer mapping with the Image > class to improve code sharing. The ViewFinder API is updated to take an > Image pointer in the render() function to prepare for multi-planar > buffer support. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> Much better IMO. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/qcam/main_window.cpp | 31 ++++++++----------------------- > src/qcam/main_window.h | 4 ++-- > src/qcam/meson.build | 1 + > src/qcam/viewfinder.h | 6 +++--- > src/qcam/viewfinder_gl.cpp | 7 ++++--- > src/qcam/viewfinder_gl.h | 2 +- > src/qcam/viewfinder_qt.cpp | 6 +++--- > src/qcam/viewfinder_qt.h | 2 +- > 8 files changed, 23 insertions(+), 36 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 0a00b1001570..168dd5ce30e3 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -7,10 +7,9 @@ > > #include "main_window.h" > > +#include <assert.h> > #include <iomanip> > #include <string> > -#include <sys/mman.h> > -#include <unistd.h> > > #include <QComboBox> > #include <QCoreApplication> > @@ -29,6 +28,7 @@ > #include <libcamera/camera_manager.h> > #include <libcamera/version.h> > > +#include "../cam/image.h" > #include "dng_writer.h" > #ifndef QT_NO_OPENGL > #include "viewfinder_gl.h" > @@ -473,15 +473,10 @@ int MainWindow::startCapture() > > for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) { > /* Map memory buffers and cache the mappings. */ > - const FrameBuffer::Plane &plane = buffer->planes().front(); > - size_t length = lseek(plane.fd.fd(), 0, SEEK_END); > - void *memory = mmap(NULL, length, PROT_READ, MAP_SHARED, > - plane.fd.fd(), 0); > - > - mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), > - plane.length }; > - planeData_[buffer.get()] = { static_cast<uint8_t *>(memory) + plane.offset, > - plane.length }; > + std::unique_ptr<Image> image = > + Image::fromFrameBuffer(buffer.get(), Image::MapMode::ReadOnly); > + assert(image != nullptr); > + mappedBuffers_[buffer.get()] = std::move(image); > > /* Store buffers on the free list. */ > freeBuffers_[stream].enqueue(buffer.get()); > @@ -543,12 +538,7 @@ error_disconnect: > error: > requests_.clear(); > > - for (auto &iter : mappedBuffers_) { > - const Span<uint8_t> &buffer = iter.second; > - munmap(buffer.data(), buffer.size()); > - } > mappedBuffers_.clear(); > - planeData_.clear(); > > freeBuffers_.clear(); > > @@ -580,12 +570,7 @@ void MainWindow::stopCapture() > > camera_->requestCompleted.disconnect(this); > > - for (auto &iter : mappedBuffers_) { > - const Span<uint8_t> &buffer = iter.second; > - munmap(buffer.data(), buffer.size()); > - } > mappedBuffers_.clear(); > - planeData_.clear(); > > requests_.clear(); > freeQueue_.clear(); > @@ -682,7 +667,7 @@ void MainWindow::processRaw(FrameBuffer *buffer, > "DNG Files (*.dng)"); > > if (!filename.isEmpty()) { > - uint8_t *memory = planeData_[buffer].data(); > + uint8_t *memory = mappedBuffers_[buffer]->data(0).data(); > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > rawStream_->configuration(), metadata, buffer, > memory); > @@ -766,7 +751,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) > << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; > > /* Render the frame on the viewfinder. */ > - viewfinder_->render(buffer, planeData_[buffer]); > + viewfinder_->render(buffer, mappedBuffers_[buffer].get()); > } > > void MainWindow::queueRequest(FrameBuffer *buffer) > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index 28244bca58b2..a16bea09eadc 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -34,6 +34,7 @@ using namespace libcamera; > class QAction; > class QComboBox; > > +class Image; > class HotplugEvent; > > enum { > @@ -106,8 +107,7 @@ private: > FrameBufferAllocator *allocator_; > > std::unique_ptr<CameraConfiguration> config_; > - std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; > - std::map<FrameBuffer *, Span<uint8_t>> planeData_; > + std::map<FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > > /* Capture state, buffers queue and statistics */ > bool isCapturing_; > diff --git a/src/qcam/meson.build b/src/qcam/meson.build > index 7d3621c93d41..c46f463130cd 100644 > --- a/src/qcam/meson.build > +++ b/src/qcam/meson.build > @@ -15,6 +15,7 @@ endif > qcam_enabled = true > > qcam_sources = files([ > + '../cam/image.cpp', > '../cam/options.cpp', > '../cam/stream_options.cpp', > 'format_converter.cpp', > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > index 42d40f1f33f0..fb462835fb5f 100644 > --- a/src/qcam/viewfinder.h > +++ b/src/qcam/viewfinder.h > @@ -11,11 +11,11 @@ > #include <QList> > #include <QSize> > > -#include <libcamera/base/span.h> > - > #include <libcamera/formats.h> > #include <libcamera/framebuffer.h> > > +class Image; > + > class ViewFinder > { > public: > @@ -24,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, libcamera::Span<uint8_t> mem) = 0; > + virtual void render(libcamera::FrameBuffer *buffer, Image *image) = 0; > virtual void stop() = 0; > > virtual QImage getCurrentImage() = 0; > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index d2ef036974f4..87e4fe03cb8d 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -13,6 +13,8 @@ > > #include <libcamera/formats.h> > > +#include "../cam/image.h" > + > static const QList<libcamera::PixelFormat> supportedFormats{ > /* YUV - packed (single plane) */ > libcamera::formats::UYVY, > @@ -110,8 +112,7 @@ QImage ViewFinderGL::getCurrentImage() > return grabFramebuffer(); > } > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > - libcamera::Span<uint8_t> mem) > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, Image *image) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > @@ -121,7 +122,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, > if (buffer_) > renderComplete(buffer_); > > - data_ = mem.data(); > + data_ = image->data(0).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 3334549e0be4..7cd8ef3316b9 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, libcamera::Span<uint8_t> mem) override; > + void render(libcamera::FrameBuffer *buffer, Image *image) override; > void stop() override; > > QImage getCurrentImage() override; > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > index a0bf99b0b522..fef6d53eef5e 100644 > --- a/src/qcam/viewfinder_qt.cpp > +++ b/src/qcam/viewfinder_qt.cpp > @@ -19,6 +19,7 @@ > > #include <libcamera/formats.h> > > +#include "../cam/image.h" > #include "format_converter.h" > > static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats > @@ -78,15 +79,14 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, > return 0; > } > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, > - libcamera::Span<uint8_t> mem) > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, Image *image) > { > if (buffer->planes().size() != 1) { > qWarning() << "Multi-planar buffers are not supported"; > return; > } > > - unsigned char *memory = mem.data(); > + unsigned char *memory = image->data(0).data(); > size_t size = buffer->metadata().planes()[0].bytesused; > > { > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > index 1a569b9cee6e..6b48ef48a7d1 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, libcamera::Span<uint8_t> mem) override; > + void render(libcamera::FrameBuffer *buffer, Image *image) override; > void stop() override; > > QImage getCurrentImage() override; >
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 0a00b1001570..168dd5ce30e3 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -7,10 +7,9 @@ #include "main_window.h" +#include <assert.h> #include <iomanip> #include <string> -#include <sys/mman.h> -#include <unistd.h> #include <QComboBox> #include <QCoreApplication> @@ -29,6 +28,7 @@ #include <libcamera/camera_manager.h> #include <libcamera/version.h> +#include "../cam/image.h" #include "dng_writer.h" #ifndef QT_NO_OPENGL #include "viewfinder_gl.h" @@ -473,15 +473,10 @@ int MainWindow::startCapture() for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) { /* Map memory buffers and cache the mappings. */ - const FrameBuffer::Plane &plane = buffer->planes().front(); - size_t length = lseek(plane.fd.fd(), 0, SEEK_END); - void *memory = mmap(NULL, length, PROT_READ, MAP_SHARED, - plane.fd.fd(), 0); - - mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory), - plane.length }; - planeData_[buffer.get()] = { static_cast<uint8_t *>(memory) + plane.offset, - plane.length }; + std::unique_ptr<Image> image = + Image::fromFrameBuffer(buffer.get(), Image::MapMode::ReadOnly); + assert(image != nullptr); + mappedBuffers_[buffer.get()] = std::move(image); /* Store buffers on the free list. */ freeBuffers_[stream].enqueue(buffer.get()); @@ -543,12 +538,7 @@ error_disconnect: error: requests_.clear(); - for (auto &iter : mappedBuffers_) { - const Span<uint8_t> &buffer = iter.second; - munmap(buffer.data(), buffer.size()); - } mappedBuffers_.clear(); - planeData_.clear(); freeBuffers_.clear(); @@ -580,12 +570,7 @@ void MainWindow::stopCapture() camera_->requestCompleted.disconnect(this); - for (auto &iter : mappedBuffers_) { - const Span<uint8_t> &buffer = iter.second; - munmap(buffer.data(), buffer.size()); - } mappedBuffers_.clear(); - planeData_.clear(); requests_.clear(); freeQueue_.clear(); @@ -682,7 +667,7 @@ void MainWindow::processRaw(FrameBuffer *buffer, "DNG Files (*.dng)"); if (!filename.isEmpty()) { - uint8_t *memory = planeData_[buffer].data(); + uint8_t *memory = mappedBuffers_[buffer]->data(0).data(); DNGWriter::write(filename.toStdString().c_str(), camera_.get(), rawStream_->configuration(), metadata, buffer, memory); @@ -766,7 +751,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; /* Render the frame on the viewfinder. */ - viewfinder_->render(buffer, planeData_[buffer]); + viewfinder_->render(buffer, mappedBuffers_[buffer].get()); } void MainWindow::queueRequest(FrameBuffer *buffer) diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 28244bca58b2..a16bea09eadc 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -34,6 +34,7 @@ using namespace libcamera; class QAction; class QComboBox; +class Image; class HotplugEvent; enum { @@ -106,8 +107,7 @@ private: FrameBufferAllocator *allocator_; std::unique_ptr<CameraConfiguration> config_; - std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_; - std::map<FrameBuffer *, Span<uint8_t>> planeData_; + std::map<FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; /* Capture state, buffers queue and statistics */ bool isCapturing_; diff --git a/src/qcam/meson.build b/src/qcam/meson.build index 7d3621c93d41..c46f463130cd 100644 --- a/src/qcam/meson.build +++ b/src/qcam/meson.build @@ -15,6 +15,7 @@ endif qcam_enabled = true qcam_sources = files([ + '../cam/image.cpp', '../cam/options.cpp', '../cam/stream_options.cpp', 'format_converter.cpp', diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h index 42d40f1f33f0..fb462835fb5f 100644 --- a/src/qcam/viewfinder.h +++ b/src/qcam/viewfinder.h @@ -11,11 +11,11 @@ #include <QList> #include <QSize> -#include <libcamera/base/span.h> - #include <libcamera/formats.h> #include <libcamera/framebuffer.h> +class Image; + class ViewFinder { public: @@ -24,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, libcamera::Span<uint8_t> mem) = 0; + virtual void render(libcamera::FrameBuffer *buffer, Image *image) = 0; virtual void stop() = 0; virtual QImage getCurrentImage() = 0; diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp index d2ef036974f4..87e4fe03cb8d 100644 --- a/src/qcam/viewfinder_gl.cpp +++ b/src/qcam/viewfinder_gl.cpp @@ -13,6 +13,8 @@ #include <libcamera/formats.h> +#include "../cam/image.h" + static const QList<libcamera::PixelFormat> supportedFormats{ /* YUV - packed (single plane) */ libcamera::formats::UYVY, @@ -110,8 +112,7 @@ QImage ViewFinderGL::getCurrentImage() return grabFramebuffer(); } -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, - libcamera::Span<uint8_t> mem) +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, Image *image) { if (buffer->planes().size() != 1) { qWarning() << "Multi-planar buffers are not supported"; @@ -121,7 +122,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, if (buffer_) renderComplete(buffer_); - data_ = mem.data(); + data_ = image->data(0).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 3334549e0be4..7cd8ef3316b9 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, libcamera::Span<uint8_t> mem) override; + void render(libcamera::FrameBuffer *buffer, Image *image) override; void stop() override; QImage getCurrentImage() override; diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp index a0bf99b0b522..fef6d53eef5e 100644 --- a/src/qcam/viewfinder_qt.cpp +++ b/src/qcam/viewfinder_qt.cpp @@ -19,6 +19,7 @@ #include <libcamera/formats.h> +#include "../cam/image.h" #include "format_converter.h" static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats @@ -78,15 +79,14 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, return 0; } -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, - libcamera::Span<uint8_t> mem) +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, Image *image) { if (buffer->planes().size() != 1) { qWarning() << "Multi-planar buffers are not supported"; return; } - unsigned char *memory = mem.data(); + unsigned char *memory = image->data(0).data(); size_t size = buffer->metadata().planes()[0].bytesused; { diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h index 1a569b9cee6e..6b48ef48a7d1 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, libcamera::Span<uint8_t> mem) override; + void render(libcamera::FrameBuffer *buffer, Image *image) override; void stop() override; QImage getCurrentImage() override;