[libcamera-devel] [RFC PATCH v2 06/10] qcam: main_window: Use offset mapping FrameBuffer

[Hirokazu Honda]

Hi Laurent,

On Tue, Aug 24, 2021 at 8:05 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> Thank you for the patch.
>
> On Mon, Aug 23, 2021 at 10:12:17PM +0900, Hirokazu Honda wrote:
> > FrameBuffer::Plane has offset info now. This uses the offset
> > in mapping FrameBuffer in MainWindow.
> >
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/qcam/main_window.cpp | 16 +++++++++++-----
> >  src/qcam/main_window.h   |  1 +
> >  2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 39d034de..7fdec4a8 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -10,6 +10,7 @@
> >  #include <iomanip>
> >  #include <string>
> >  #include <sys/mman.h>
> > +#include <unistd.h>
> >
> >  #include <QComboBox>
> >  #include <QCoreApplication>
> > @@ -472,9 +473,12 @@ 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();
> > -                     void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> > +                     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()] = { memory, plane.length };
> > +                     mappedBuffers_[buffer.get()] = { memory, length };
> > +                     planeData_[buffer.get()] = { static_cast<uint8_t *>(memory) + plane.offset,
> > +                                                  plane.length };
> >
> >                       /* Store buffers on the free list. */
> >                       freeBuffers_[stream].enqueue(buffer.get());
> > @@ -541,6 +545,7 @@ error:
> >               munmap(buffer.memory, buffer.size);
> >       }
> >       mappedBuffers_.clear();
> > +     planeData_.clear();
> >
> >       freeBuffers_.clear();
> >
> > @@ -577,6 +582,7 @@ void MainWindow::stopCapture()
> >               munmap(buffer.memory, buffer.size);
> >       }
> >       mappedBuffers_.clear();
> > +     planeData_.clear();
> >
> >       requests_.clear();
> >       freeQueue_.clear();
> > @@ -673,10 +679,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,
> >                                                       "DNG Files (*.dng)");
> >
> >       if (!filename.isEmpty()) {
> > -             const MappedBuffer &mapped = mappedBuffers_[buffer];
> > +             void *memory = planeData_[buffer].data();
> >               DNGWriter::write(filename.toStdString().c_str(), camera_.get(),
> >                                rawStream_->configuration(), metadata, buffer,
> > -                              mapped.memory);
> > +                              memory);
> >       }
> >  #endif
> >
> > @@ -753,7 +759,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
> >               << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
> >
> >       /* Render the frame on the viewfinder. */
> > -     viewfinder_->render(buffer, &mappedBuffers_[buffer]);
> > +     // HACK HACK HACK viewfinder_->render(buffer, &planeData_[buffer]);
>
> I'm afraid this needs a fix :-) Could it be as simple as
>
>         viewfinder_->render(buffer, planeData_[buffer]);
>
> ?

Yeah, I commented out while I was waiting for your Span patch. :-)

-Hiro
>
> >  }
> >
> >  void MainWindow::queueRequest(FrameBuffer *buffer)
> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> > index 85d56ce4..f35cce36 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -107,6 +107,7 @@ private:
> >
> >       std::unique_ptr<CameraConfiguration> config_;
> >       std::map<FrameBuffer *, MappedBuffer> mappedBuffers_;
> > +     std::map<FrameBuffer *, Span<uint8_t>> planeData_;
> >
> >       /* Capture state, buffers queue and statistics */
> >       bool isCapturing_;
>
> --
> Regards,
>
> Laurent Pinchart