| Message ID | 20210823131221.1034059-5-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Mon, Aug 23, 2021 at 10:12:15PM +0900, Hirokazu Honda wrote: > This fixes the way of mapping FrameBuffer in FrameSink by > using offset. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > src/cam/file_sink.cpp | 22 +++++++++++++++++----- > src/cam/file_sink.h | 1 + > 2 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 2d30694a..26a60058 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -51,12 +51,24 @@ int FileSink::configure(const libcamera::CameraConfiguration &config) > > void FileSink::mapBuffer(FrameBuffer *buffer) > { > + /* \todo use MappedFrameBuffer. */ > for (const FrameBuffer::Plane &plane : buffer->planes()) { > - void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, > - plane.fd.fd(), 0); > + const int fd = plane.fd.fd(); > + if (mappedBuffers_.find(fd) == mappedBuffers_.end()) { > + /** > + * \todo Should we try to only map the portions of the > + * dmabuf that are used by planes ? > + */ Tabs for indentation. > + size_t length = lseek(fd, 0, SEEK_END); > + void *memory = mmap(NULL, plane.length, PROT_READ, > + MAP_SHARED, plane.fd.fd(), 0); > + mappedBuffers_[plane.fd.fd()] = > + std::make_pair(memory, length); You can use fd instead of plane.fd.fd(). > And you can drop this blank line. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > - mappedBuffers_[plane.fd.fd()] = > - std::make_pair(memory, plane.length); > + } > + > + void *memory = mappedBuffers_[fd].first; > + planeData_[&plane] = static_cast<uint8_t *>(memory) + plane.offset; > } > } > > @@ -102,7 +114,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > const FrameBuffer::Plane &plane = buffer->planes()[i]; > const FrameMetadata::Plane &meta = buffer->metadata().planes[i]; > > - void *data = mappedBuffers_[plane.fd.fd()].first; > + uint8_t *data = planeData_[&plane]; > unsigned int length = std::min(meta.bytesused, plane.length); > > if (meta.bytesused > plane.length) > diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h > index c3eb230a..c12325d9 100644 > --- a/src/cam/file_sink.h > +++ b/src/cam/file_sink.h > @@ -33,6 +33,7 @@ private: > std::map<const libcamera::Stream *, std::string> streamNames_; > std::string pattern_; > std::map<int, std::pair<void *, unsigned int>> mappedBuffers_; > + std::map<const libcamera::FrameBuffer::Plane *, uint8_t *> planeData_; > }; > > #endif /* __CAM_FILE_SINK_H__ */
diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp index 2d30694a..26a60058 100644 --- a/src/cam/file_sink.cpp +++ b/src/cam/file_sink.cpp @@ -51,12 +51,24 @@ int FileSink::configure(const libcamera::CameraConfiguration &config) void FileSink::mapBuffer(FrameBuffer *buffer) { + /* \todo use MappedFrameBuffer. */ for (const FrameBuffer::Plane &plane : buffer->planes()) { - void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, - plane.fd.fd(), 0); + const int fd = plane.fd.fd(); + if (mappedBuffers_.find(fd) == mappedBuffers_.end()) { + /** + * \todo Should we try to only map the portions of the + * dmabuf that are used by planes ? + */ + size_t length = lseek(fd, 0, SEEK_END); + void *memory = mmap(NULL, plane.length, PROT_READ, + MAP_SHARED, plane.fd.fd(), 0); + mappedBuffers_[plane.fd.fd()] = + std::make_pair(memory, length); - mappedBuffers_[plane.fd.fd()] = - std::make_pair(memory, plane.length); + } + + void *memory = mappedBuffers_[fd].first; + planeData_[&plane] = static_cast<uint8_t *>(memory) + plane.offset; } } @@ -102,7 +114,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) const FrameBuffer::Plane &plane = buffer->planes()[i]; const FrameMetadata::Plane &meta = buffer->metadata().planes[i]; - void *data = mappedBuffers_[plane.fd.fd()].first; + uint8_t *data = planeData_[&plane]; unsigned int length = std::min(meta.bytesused, plane.length); if (meta.bytesused > plane.length) diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h index c3eb230a..c12325d9 100644 --- a/src/cam/file_sink.h +++ b/src/cam/file_sink.h @@ -33,6 +33,7 @@ private: std::map<const libcamera::Stream *, std::string> streamNames_; std::string pattern_; std::map<int, std::pair<void *, unsigned int>> mappedBuffers_; + std::map<const libcamera::FrameBuffer::Plane *, uint8_t *> planeData_; }; #endif /* __CAM_FILE_SINK_H__ */
This fixes the way of mapping FrameBuffer in FrameSink by using offset. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- src/cam/file_sink.cpp | 22 +++++++++++++++++----- src/cam/file_sink.h | 1 + 2 files changed, 18 insertions(+), 5 deletions(-)