[libcamera-devel] [PATCH v2 25/27] qcam: Use Image class to access pixel data
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Sep 6 13:45:35 CEST 2021
Hi Jean-Michel,
On Mon, Sep 06, 2021 at 11:36:10AM +0200, Jean-Michel Hautbois wrote:
> On 06/09/2021 04:00, 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 at 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',
>
> I never realized we were referencing as much "../cam" !
>
> > '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"
>
> Where should Image class be in the end, once stable enough ?
I expect it to move to include/libcamera/ then.
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>
> > +
> > 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;
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list