Message ID | 20200720224232.153717-4-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:27PM +0100, Kieran Bingham wrote: > Remove the local mapping code, and utilise the libcamera buffer map > helper class. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > > Working towards a more generic MappedBuffer base interface of the newly > introduced MappedFrameBuffer hits a snag that the viewfinder in qcam > already defines a type which conflicts. > > Remove it by converting qcam to use the new MappedFrameBuffer types. I'd drop this as I think we should start with MappedFrameBuffer being an internal class. > src/qcam/main_window.cpp | 27 +++++++++++---------------- > src/qcam/main_window.h | 2 +- > src/qcam/viewfinder.cpp | 4 ++-- > src/qcam/viewfinder.h | 7 +------ > 4 files changed, 15 insertions(+), 25 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 7bc13603774e..b25260ba7b28 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -446,10 +446,14 @@ 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, > - plane.fd.fd(), 0); > - mappedBuffers_[buffer.get()] = { memory, plane.length }; > + MappedFrameBuffer mapped(buffer.get(), PROT_READ); > + if (!mapped.isValid()) { > + qWarning() << "Failed to map buffer"; > + ret = mapped.error(); > + goto error; > + } > + > + mappedBuffers_.insert({buffer.get(), std::move(mapped)}); > > /* Store buffers on the free list. */ > freeBuffers_[stream].enqueue(buffer.get()); > @@ -512,12 +516,7 @@ error: > for (Request *request : requests) > delete request; > > - for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > - } > mappedBuffers_.clear(); > - > freeBuffers_.clear(); > > delete allocator_; > @@ -548,10 +547,6 @@ void MainWindow::stopCapture() > > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); > > - for (auto &iter : mappedBuffers_) { > - const MappedBuffer &buffer = iter.second; > - munmap(buffer.memory, buffer.size); > - } > mappedBuffers_.clear(); > > delete allocator_; > @@ -643,10 +638,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, const ControlList &metadata) > "DNG Files (*.dng)"); > > if (!filename.isEmpty()) { > - const MappedBuffer &mapped = mappedBuffers_[buffer]; > + const MappedFrameBuffer &mapped = mappedBuffers_.at(buffer); > DNGWriter::write(filename.toStdString().c_str(), camera_.get(), > rawStream_->configuration(), metadata, buffer, > - mapped.memory); > + mapped.maps()[0].address); > } > #endif > > @@ -720,7 +715,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_.at(buffer)); > } > > void MainWindow::queueRequest(FrameBuffer *buffer) > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index 4606fe487ad4..ddf51ca01111 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -119,7 +119,7 @@ private: > FrameBufferAllocator *allocator_; > > std::unique_ptr<CameraConfiguration> config_; > - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; > + std::map<FrameBuffer *, MappedFrameBuffer> mappedBuffers_; > > /* Capture state, buffers queue and statistics */ > bool isCapturing_; > diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp > index dcf8a15d2df6..591d26eae87c 100644 > --- a/src/qcam/viewfinder.cpp > +++ b/src/qcam/viewfinder.cpp > @@ -78,14 +78,14 @@ int ViewFinder::setFormat(const libcamera::PixelFormat &format, > return 0; > } > > -void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) > +void ViewFinder::render(libcamera::FrameBuffer *buffer, libcamera::MappedFrameBuffer *map) > { > 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 = static_cast<unsigned char *>(map->maps()[0].address); > size_t size = buffer->metadata().planes[0].bytesused; > > { > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h > index 26a1320537d2..c4cc51f14dda 100644 > --- a/src/qcam/viewfinder.h > +++ b/src/qcam/viewfinder.h > @@ -23,11 +23,6 @@ > > class QImage; > > -struct MappedBuffer { > - void *memory; > - size_t size; > -}; > - > class ViewFinder : public QWidget > { > Q_OBJECT > @@ -39,7 +34,7 @@ public: > const QList<libcamera::PixelFormat> &nativeFormats() const; > > int setFormat(const libcamera::PixelFormat &format, const QSize &size); > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map); > + void render(libcamera::FrameBuffer *buffer, libcamera::MappedFrameBuffer *map); > void stop(); > > QImage getCurrentImage();
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 7bc13603774e..b25260ba7b28 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -446,10 +446,14 @@ 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, - plane.fd.fd(), 0); - mappedBuffers_[buffer.get()] = { memory, plane.length }; + MappedFrameBuffer mapped(buffer.get(), PROT_READ); + if (!mapped.isValid()) { + qWarning() << "Failed to map buffer"; + ret = mapped.error(); + goto error; + } + + mappedBuffers_.insert({buffer.get(), std::move(mapped)}); /* Store buffers on the free list. */ freeBuffers_[stream].enqueue(buffer.get()); @@ -512,12 +516,7 @@ error: for (Request *request : requests) delete request; - for (auto &iter : mappedBuffers_) { - const MappedBuffer &buffer = iter.second; - munmap(buffer.memory, buffer.size); - } mappedBuffers_.clear(); - freeBuffers_.clear(); delete allocator_; @@ -548,10 +547,6 @@ void MainWindow::stopCapture() camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete); - for (auto &iter : mappedBuffers_) { - const MappedBuffer &buffer = iter.second; - munmap(buffer.memory, buffer.size); - } mappedBuffers_.clear(); delete allocator_; @@ -643,10 +638,10 @@ void MainWindow::processRaw(FrameBuffer *buffer, const ControlList &metadata) "DNG Files (*.dng)"); if (!filename.isEmpty()) { - const MappedBuffer &mapped = mappedBuffers_[buffer]; + const MappedFrameBuffer &mapped = mappedBuffers_.at(buffer); DNGWriter::write(filename.toStdString().c_str(), camera_.get(), rawStream_->configuration(), metadata, buffer, - mapped.memory); + mapped.maps()[0].address); } #endif @@ -720,7 +715,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_.at(buffer)); } void MainWindow::queueRequest(FrameBuffer *buffer) diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 4606fe487ad4..ddf51ca01111 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -119,7 +119,7 @@ private: FrameBufferAllocator *allocator_; std::unique_ptr<CameraConfiguration> config_; - std::map<FrameBuffer *, MappedBuffer> mappedBuffers_; + std::map<FrameBuffer *, MappedFrameBuffer> mappedBuffers_; /* Capture state, buffers queue and statistics */ bool isCapturing_; diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp index dcf8a15d2df6..591d26eae87c 100644 --- a/src/qcam/viewfinder.cpp +++ b/src/qcam/viewfinder.cpp @@ -78,14 +78,14 @@ int ViewFinder::setFormat(const libcamera::PixelFormat &format, return 0; } -void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) +void ViewFinder::render(libcamera::FrameBuffer *buffer, libcamera::MappedFrameBuffer *map) { 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 = static_cast<unsigned char *>(map->maps()[0].address); size_t size = buffer->metadata().planes[0].bytesused; { diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h index 26a1320537d2..c4cc51f14dda 100644 --- a/src/qcam/viewfinder.h +++ b/src/qcam/viewfinder.h @@ -23,11 +23,6 @@ class QImage; -struct MappedBuffer { - void *memory; - size_t size; -}; - class ViewFinder : public QWidget { Q_OBJECT @@ -39,7 +34,7 @@ public: const QList<libcamera::PixelFormat> &nativeFormats() const; int setFormat(const libcamera::PixelFormat &format, const QSize &size); - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map); + void render(libcamera::FrameBuffer *buffer, libcamera::MappedFrameBuffer *map); void stop(); QImage getCurrentImage();
Remove the local mapping code, and utilise the libcamera buffer map helper class. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- Working towards a more generic MappedBuffer base interface of the newly introduced MappedFrameBuffer hits a snag that the viewfinder in qcam already defines a type which conflicts. Remove it by converting qcam to use the new MappedFrameBuffer types. src/qcam/main_window.cpp | 27 +++++++++++---------------- src/qcam/main_window.h | 2 +- src/qcam/viewfinder.cpp | 4 ++-- src/qcam/viewfinder.h | 7 +------ 4 files changed, 15 insertions(+), 25 deletions(-)