| Message ID | 20210816043138.957984-5-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Mon, Aug 16, 2021 at 01:31:32PM +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 | 5 ++++- > src/cam/file_sink.h | 1 + > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 2d30694a..92b2d53d 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -51,12 +51,15 @@ 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); > > mappedBuffers_[plane.fd.fd()] = > std::make_pair(memory, plane.length); > + planeData_[plane.fd.fd()] = > + static_cast<unsigned int *>(memory) + plane.offset; I don't think this will work. With an NV12 buffer, for instance, plane.fd.fd() will be the same for both planes, so you'll overwrite planeData_ of the first plane with the second plane. It's fine in this patch, as we don't create FrameBuffer instances with multiple planes, but it will break as soon as we do. Should we instead store the addresses in a std::map<std::pair<Buffer *, unsigned int>, void *> (with the unsigned int being the plane index) ? Of maybe just a std::map<FrameBuffer::Plane *, void *> ? > } > } > > @@ -102,7 +105,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; > + void *data = planeData_[plane.fd.fd()]; > 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..d236d6d8 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<int, void *> planeData_; > }; > > #endif /* __CAM_FILE_SINK_H__ */
diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp index 2d30694a..92b2d53d 100644 --- a/src/cam/file_sink.cpp +++ b/src/cam/file_sink.cpp @@ -51,12 +51,15 @@ 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); mappedBuffers_[plane.fd.fd()] = std::make_pair(memory, plane.length); + planeData_[plane.fd.fd()] = + static_cast<unsigned int *>(memory) + plane.offset; } } @@ -102,7 +105,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; + void *data = planeData_[plane.fd.fd()]; 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..d236d6d8 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<int, void *> 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 | 5 ++++- src/cam/file_sink.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)