| Message ID | 20201026095356.442605-1-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi all, Gentle ping to get some feedback on this patch set when you get a moment. Regards, Naush On Mon, 26 Oct 2020 at 09:53, Naushir Patuck <naush@raspberrypi.com> wrote: > Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline > handler to use in the cases where the sensor does not fill it in. This > avoids the need to mmap and unmap on every frame. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 35 ++++++++++++++----- > 1 file changed, 27 insertions(+), 8 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index dd62dfc7..8817915b 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -24,6 +24,7 @@ > #include <linux/videodev2.h> > > #include "libcamera/internal/bayer_format.h" > +#include "libcamera/internal/buffer.h" > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/device_enumerator.h" > #include "libcamera/internal/ipa_manager.h" > @@ -165,6 +166,12 @@ public: > /* Stores the ids of the buffers mapped in the IPA. */ > std::unordered_set<unsigned int> ipaBuffers_; > > + /* > + * Map of (internal) mmaped embedded data buffers, to avoid having > to > + * map/unmap on every frame. > + */ > + std::map<unsigned int, MappedFrameBuffer> mappedEmbeddedBuffers_; > + > /* DMAHEAP allocation helper. */ > RPi::DmaHeap dmaHeap_; > FileDescriptor lsTable_; > @@ -1040,6 +1047,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera > *camera) > return ret; > } > > + if (!data->sensorMetadata_) { > + for (auto const &it : > data->unicam_[Unicam::Embedded].getBuffers()) { > + > data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct, > + > std::forward_as_tuple(it.first), > + > std::forward_as_tuple( > + > MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE))); > + } > + } > + > /* > * Pass the stats and embedded data buffers to the IPA. No other > * buffers need to be passed. > @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera) > std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), > data->ipaBuffers_.end()); > data->ipa_->unmapBuffers(ipaBuffers); > data->ipaBuffers_.clear(); > + data->mappedEmbeddedBuffers_.clear(); > > for (auto const stream : data->streams_) > stream->releaseBuffers(); > @@ -1310,14 +1327,16 @@ void > RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > * metadata buffer. > */ > if (!sensorMetadata_) { > - const FrameBuffer &fb = buffer->planes(); > - uint32_t *mem = static_cast<uint32_t > *>(::mmap(nullptr, fb.planes()[0].length, > - > PROT_READ | PROT_WRITE, > - > MAP_SHARED, > - > fb.planes()[0].fd.fd(), 0)); > - mem[0] = ctrl[V4L2_CID_EXPOSURE]; > - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > - munmap(mem, fb.planes()[0].length); > + unsigned int bufferId = > unicam_[Unicam::Embedded].getBufferId(buffer); > + auto it = mappedEmbeddedBuffers_.find(bufferId); > + if (it != mappedEmbeddedBuffers_.end()) { > + uint32_t *mem = reinterpret_cast<uint32_t > *>(it->second.maps()[0].data()); > + mem[0] = ctrl[V4L2_CID_EXPOSURE]; > + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > + } else { > + LOG(RPI, Warning) << "Failed to find > embedded buffer " > + << bufferId; > + } > } > } > > -- > 2.25.1 > >
Hi Naush, On 26/10/2020 09:53, Naushir Patuck wrote: > Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline > handler to use in the cases where the sensor does not fill it in. This > avoids the need to mmap and unmap on every frame. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 35 ++++++++++++++----- > 1 file changed, 27 insertions(+), 8 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index dd62dfc7..8817915b 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -24,6 +24,7 @@ > #include <linux/videodev2.h> > > #include "libcamera/internal/bayer_format.h" > +#include "libcamera/internal/buffer.h" > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/device_enumerator.h" > #include "libcamera/internal/ipa_manager.h" > @@ -165,6 +166,12 @@ public: > /* Stores the ids of the buffers mapped in the IPA. */ > std::unordered_set<unsigned int> ipaBuffers_; > > + /* > + * Map of (internal) mmaped embedded data buffers, to avoid having to > + * map/unmap on every frame. > + */ > + std::map<unsigned int, MappedFrameBuffer> mappedEmbeddedBuffers_; > + > /* DMAHEAP allocation helper. */ > RPi::DmaHeap dmaHeap_; > FileDescriptor lsTable_; > @@ -1040,6 +1047,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) > return ret; > } > > + if (!data->sensorMetadata_) { > + for (auto const &it : data->unicam_[Unicam::Embedded].getBuffers()) { > + data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct, > + std::forward_as_tuple(it.first), > + std::forward_as_tuple( > + MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE))); Does this need to use piecewise? Can't you just use: emplace(it.first, MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)); directly? Or is that then trying to use an invalid copy or move constructor? If you do need to piecewise, Does it make sense to shorten it to: emplace(std::piecewise_construct, std::forward_as_tuple(it.first), std::forward_as_tuple(it.second, PROT_READ | PROT_WRITE))); Although maybe it's more clear showing the MappedFrameBuffer type. Not really a problem what route is used as long as it works I think ;-) Just tried out a few options and they all seem to compile, so I'd try to remove the piecewise if it's not needed ... > + } > + } > + > /* > * Pass the stats and embedded data buffers to the IPA. No other > * buffers need to be passed. > @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera) > std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end()); > data->ipa_->unmapBuffers(ipaBuffers); > data->ipaBuffers_.clear(); > + data->mappedEmbeddedBuffers_.clear(); > > for (auto const stream : data->streams_) > stream->releaseBuffers(); > @@ -1310,14 +1327,16 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > * metadata buffer. > */ > if (!sensorMetadata_) { > - const FrameBuffer &fb = buffer->planes(); > - uint32_t *mem = static_cast<uint32_t *>(::mmap(nullptr, fb.planes()[0].length, > - PROT_READ | PROT_WRITE, > - MAP_SHARED, > - fb.planes()[0].fd.fd(), 0)); > - mem[0] = ctrl[V4L2_CID_EXPOSURE]; > - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > - munmap(mem, fb.planes()[0].length); Oh, yes, keeping these mappings around rather than mapping and unmapping each frame will definitely help. > + unsigned int bufferId = unicam_[Unicam::Embedded].getBufferId(buffer); > + auto it = mappedEmbeddedBuffers_.find(bufferId); > + if (it != mappedEmbeddedBuffers_.end()) { > + uint32_t *mem = reinterpret_cast<uint32_t *>(it->second.maps()[0].data()); I wonder if we should add a helper to MappedFrameBuffer to return a more friendly data type to avoid casts like this, but that's not for this patch. With the piecewise constructs either clarified that you want to keep them, or simplified out: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + mem[0] = ctrl[V4L2_CID_EXPOSURE]; > + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > + } else { > + LOG(RPI, Warning) << "Failed to find embedded buffer " > + << bufferId; > + } > } > } > >
Hi Kieran, On Thu, 12 Nov 2020 at 22:16, Kieran Bingham < kieran.bingham@ideasonboard.com> wrote: > Hi Naush, > > On 26/10/2020 09:53, Naushir Patuck wrote: > > Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline > > handler to use in the cases where the sensor does not fill it in. This > > avoids the need to mmap and unmap on every frame. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > .../pipeline/raspberrypi/raspberrypi.cpp | 35 ++++++++++++++----- > > 1 file changed, 27 insertions(+), 8 deletions(-) > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index dd62dfc7..8817915b 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -24,6 +24,7 @@ > > #include <linux/videodev2.h> > > > > #include "libcamera/internal/bayer_format.h" > > +#include "libcamera/internal/buffer.h" > > #include "libcamera/internal/camera_sensor.h" > > #include "libcamera/internal/device_enumerator.h" > > #include "libcamera/internal/ipa_manager.h" > > @@ -165,6 +166,12 @@ public: > > /* Stores the ids of the buffers mapped in the IPA. */ > > std::unordered_set<unsigned int> ipaBuffers_; > > > > + /* > > + * Map of (internal) mmaped embedded data buffers, to avoid having > to > > + * map/unmap on every frame. > > + */ > > + std::map<unsigned int, MappedFrameBuffer> mappedEmbeddedBuffers_; > > + > > /* DMAHEAP allocation helper. */ > > RPi::DmaHeap dmaHeap_; > > FileDescriptor lsTable_; > > @@ -1040,6 +1047,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera > *camera) > > return ret; > > } > > > > + if (!data->sensorMetadata_) { > > + for (auto const &it : > data->unicam_[Unicam::Embedded].getBuffers()) { > > + > data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct, > > + > std::forward_as_tuple(it.first), > > + > std::forward_as_tuple( > > + > MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE))); > > Does this need to use piecewise? > > Can't you just use: > > emplace(it.first, > MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)); > directly? > This is hitting the limits of my C++ knowledge :-) I thought using a piecewise_construct might be more efficient here, as you do not create a temporary object of MappedFrameBuffer on the stack and do a copy. Although, the compiler might possibly optimise that out anyway....? Not sure. Either way, this is in startup code, and any possible penalty of constructing and copying temporary objects is not really going to cause me to stay up at night. I'll change it to the above and post an update. > Or is that then trying to use an invalid copy or move constructor? > > If you do need to piecewise, Does it make sense to shorten it to: > > emplace(std::piecewise_construct, > std::forward_as_tuple(it.first), > std::forward_as_tuple(it.second, PROT_READ | PROT_WRITE))); > I did not think this would work, but admittedly never tried it! Regards, Naush > Although maybe it's more clear showing the MappedFrameBuffer type. > > Not really a problem what route is used as long as it works I think ;-) > > > Just tried out a few options and they all seem to compile, so I'd try to > remove the piecewise if it's not needed ... > > > + } > > + } > > + > > /* > > * Pass the stats and embedded data buffers to the IPA. No other > > * buffers need to be passed. > > @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera > *camera) > > std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), > data->ipaBuffers_.end()); > > data->ipa_->unmapBuffers(ipaBuffers); > > data->ipaBuffers_.clear(); > > + data->mappedEmbeddedBuffers_.clear(); > > > > for (auto const stream : data->streams_) > > stream->releaseBuffers(); > > @@ -1310,14 +1327,16 @@ void > RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > > * metadata buffer. > > */ > > if (!sensorMetadata_) { > > - const FrameBuffer &fb = buffer->planes(); > > - uint32_t *mem = static_cast<uint32_t > *>(::mmap(nullptr, fb.planes()[0].length, > > - > PROT_READ | PROT_WRITE, > > - > MAP_SHARED, > > - > fb.planes()[0].fd.fd(), 0)); > > - mem[0] = ctrl[V4L2_CID_EXPOSURE]; > > - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > > - munmap(mem, fb.planes()[0].length); > > Oh, yes, keeping these mappings around rather than mapping and unmapping > each frame will definitely help. > > > + unsigned int bufferId = > unicam_[Unicam::Embedded].getBufferId(buffer); > > + auto it = mappedEmbeddedBuffers_.find(bufferId); > > + if (it != mappedEmbeddedBuffers_.end()) { > > + uint32_t *mem = reinterpret_cast<uint32_t > *>(it->second.maps()[0].data()); > > I wonder if we should add a helper to MappedFrameBuffer to return a more > friendly data type to avoid casts like this, but that's not for this patch. > > > With the piecewise constructs either clarified that you want to keep > them, or simplified out: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > + mem[0] = ctrl[V4L2_CID_EXPOSURE]; > > + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > > + } else { > > + LOG(RPI, Warning) << "Failed to find > embedded buffer " > > + << bufferId; > > + } > > } > > } > > > > > > -- > Regards > -- > Kieran >
Hi Naush, On 12/11/2020 22:47, Naushir Patuck wrote: > Hi Kieran, > > > On Thu, 12 Nov 2020 at 22:16, Kieran Bingham > <kieran.bingham@ideasonboard.com > <mailto:kieran.bingham@ideasonboard.com>> wrote: > > Hi Naush, > > On 26/10/2020 09:53, Naushir Patuck wrote: > > Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline > > handler to use in the cases where the sensor does not fill it in. This > > avoids the need to mmap and unmap on every frame. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com > <mailto:naush@raspberrypi.com>> > > --- > > .../pipeline/raspberrypi/raspberrypi.cpp | 35 > ++++++++++++++----- > > 1 file changed, 27 insertions(+), 8 deletions(-) > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index dd62dfc7..8817915b 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -24,6 +24,7 @@ > > #include <linux/videodev2.h> > > > > #include "libcamera/internal/bayer_format.h" > > +#include "libcamera/internal/buffer.h" > > #include "libcamera/internal/camera_sensor.h" > > #include "libcamera/internal/device_enumerator.h" > > #include "libcamera/internal/ipa_manager.h" > > @@ -165,6 +166,12 @@ public: > > /* Stores the ids of the buffers mapped in the IPA. */ > > std::unordered_set<unsigned int> ipaBuffers_; > > > > + /* > > + * Map of (internal) mmaped embedded data buffers, to avoid > having to > > + * map/unmap on every frame. > > + */ > > + std::map<unsigned int, MappedFrameBuffer> > mappedEmbeddedBuffers_; > > + > > /* DMAHEAP allocation helper. */ > > RPi::DmaHeap dmaHeap_; > > FileDescriptor lsTable_; > > @@ -1040,6 +1047,15 @@ int > PipelineHandlerRPi::prepareBuffers(Camera *camera) > > return ret; > > } > > > > + if (!data->sensorMetadata_) { > > + for (auto const &it : > data->unicam_[Unicam::Embedded].getBuffers()) { > > + > data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct, > > + > std::forward_as_tuple(it.first), > > + > std::forward_as_tuple( > > + > MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE))); > > Does this need to use piecewise? > > Can't you just use: > > emplace(it.first, > MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)); > directly? > > > This is hitting the limits of my C++ knowledge :-) > piecewise requirements stretched mine a bit too. Back in January Laurent did some code simplification around this, and I didn't really understand what it was doing then so I (while sat next to him, quite the novelty) got him to write a long commit message explaining it more.. Not sure if it helps or not, but this is the relevant commit message there: https://git.libcamera.org/libcamera/libcamera.git/commit/?id=38dd90307ab2b0d25a0a233eae04455f769153b4 > I thought using a piecewise_construct might be more efficient here, as > you do not create a temporary object of MappedFrameBuffer on the stack > and do a copy. Although, the compiler might possibly optimise that out > anyway....? Not sure. Either way, this is in startup code, and any I think by doing the emplace we're already heading to that efficiency. The piecewise is needed when the thing being emplaced is more complex or when one of the things being emplaced needs more than one argument. > possible penalty of constructing and copying temporary objects is not > really going to cause me to stay up at night. I'll change it to the > above and post an update. I think emplace(std::piecewise_construct, std::forward_as_tuple(it.first), std::forward_as_tuple(it.second, PROT_READ | PROT_WRITE))); would ensure the performance gain, but I would fear: emplace(std::piecewise_construct, std::forward_as_tuple(it.first), std::forward_as_tuple(MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)))); would be invoking the MappedFrameBuffer constructor explicitly, forcing a copy or a move. But I'm not really sure what the compiler does here without going through and putting some prints in to see what gets invoked. As this construct is only at initialise I'm not worried by that, but I don't think the MappedFrameBuffers are copyable anyway, as that would lead to two objects that could unmap the same memory. I assume I handled that all in the MappedFrameBuffer class though ;-) Well - on the topic of not losing any sleep over this - I'm going to go and try to find some! hehe -- Kieran > > > Or is that then trying to use an invalid copy or move constructor? > > If you do need to piecewise, Does it make sense to shorten it to: > > emplace(std::piecewise_construct, > std::forward_as_tuple(it.first), > std::forward_as_tuple(it.second, PROT_READ | PROT_WRITE))); > > I did not think this would work, but admittedly never tried it! > > Regards, > Naush > > > > Although maybe it's more clear showing the MappedFrameBuffer type. > > Not really a problem what route is used as long as it works I think ;-) > > > Just tried out a few options and they all seem to compile, so I'd try to > remove the piecewise if it's not needed ... > > > + } > > + } > > + > > /* > > * Pass the stats and embedded data buffers to the IPA. No other > > * buffers need to be passed. > > @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera > *camera) > > std::vector<unsigned int> > ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end()); > > data->ipa_->unmapBuffers(ipaBuffers); > > data->ipaBuffers_.clear(); > > + data->mappedEmbeddedBuffers_.clear(); > > > > for (auto const stream : data->streams_) > > stream->releaseBuffers(); > > @@ -1310,14 +1327,16 @@ void > RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > > * metadata buffer. > > */ > > if (!sensorMetadata_) { > > - const FrameBuffer &fb = buffer->planes(); > > - uint32_t *mem = static_cast<uint32_t > *>(::mmap(nullptr, fb.planes()[0].length, > > - > PROT_READ | PROT_WRITE, > > - > MAP_SHARED, > > - > fb.planes()[0].fd.fd(), 0)); > > - mem[0] = ctrl[V4L2_CID_EXPOSURE]; > > - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > > - munmap(mem, fb.planes()[0].length); > > Oh, yes, keeping these mappings around rather than mapping and unmapping > each frame will definitely help. > > > + unsigned int bufferId = > unicam_[Unicam::Embedded].getBufferId(buffer); > > + auto it = mappedEmbeddedBuffers_.find(bufferId); > > + if (it != mappedEmbeddedBuffers_.end()) { > > + uint32_t *mem = > reinterpret_cast<uint32_t *>(it->second.maps()[0].data()); > > I wonder if we should add a helper to MappedFrameBuffer to return a more > friendly data type to avoid casts like this, but that's not for this > patch. > > > With the piecewise constructs either clarified that you want to keep > them, or simplified out: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com > <mailto:kieran.bingham@ideasonboard.com>> > > > > > + mem[0] = ctrl[V4L2_CID_EXPOSURE]; > > + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; > > + } else { > > + LOG(RPI, Warning) << "Failed to find > embedded buffer " > > + << bufferId; > > + } > > } > > } > > > > > > -- > Regards > -- > Kieran >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index dd62dfc7..8817915b 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -24,6 +24,7 @@ #include <linux/videodev2.h> #include "libcamera/internal/bayer_format.h" +#include "libcamera/internal/buffer.h" #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/ipa_manager.h" @@ -165,6 +166,12 @@ public: /* Stores the ids of the buffers mapped in the IPA. */ std::unordered_set<unsigned int> ipaBuffers_; + /* + * Map of (internal) mmaped embedded data buffers, to avoid having to + * map/unmap on every frame. + */ + std::map<unsigned int, MappedFrameBuffer> mappedEmbeddedBuffers_; + /* DMAHEAP allocation helper. */ RPi::DmaHeap dmaHeap_; FileDescriptor lsTable_; @@ -1040,6 +1047,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) return ret; } + if (!data->sensorMetadata_) { + for (auto const &it : data->unicam_[Unicam::Embedded].getBuffers()) { + data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct, + std::forward_as_tuple(it.first), + std::forward_as_tuple( + MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE))); + } + } + /* * Pass the stats and embedded data buffers to the IPA. No other * buffers need to be passed. @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera) std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end()); data->ipa_->unmapBuffers(ipaBuffers); data->ipaBuffers_.clear(); + data->mappedEmbeddedBuffers_.clear(); for (auto const stream : data->streams_) stream->releaseBuffers(); @@ -1310,14 +1327,16 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) * metadata buffer. */ if (!sensorMetadata_) { - const FrameBuffer &fb = buffer->planes(); - uint32_t *mem = static_cast<uint32_t *>(::mmap(nullptr, fb.planes()[0].length, - PROT_READ | PROT_WRITE, - MAP_SHARED, - fb.planes()[0].fd.fd(), 0)); - mem[0] = ctrl[V4L2_CID_EXPOSURE]; - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; - munmap(mem, fb.planes()[0].length); + unsigned int bufferId = unicam_[Unicam::Embedded].getBufferId(buffer); + auto it = mappedEmbeddedBuffers_.find(bufferId); + if (it != mappedEmbeddedBuffers_.end()) { + uint32_t *mem = reinterpret_cast<uint32_t *>(it->second.maps()[0].data()); + mem[0] = ctrl[V4L2_CID_EXPOSURE]; + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; + } else { + LOG(RPI, Warning) << "Failed to find embedded buffer " + << bufferId; + } } }
Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline handler to use in the cases where the sensor does not fill it in. This avoids the need to mmap and unmap on every frame. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- .../pipeline/raspberrypi/raspberrypi.cpp | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-)