| Message ID | 20250611013245.133785-8-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Bryan, Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > Add MappedFrameBuffer::getPlaneFD() which takes a plane index and returns > the file descriptor associated with it. > > This fd will be used to feed into eglCreateImageKHR for both texture > creation on upload and directly render-to-texture where the texture buffer > comes from the fd given to eglCreateImageKHR. So it's for future improvements not present in the current version, right? > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > include/libcamera/internal/mapped_framebuffer.h | 1 + > src/libcamera/mapped_framebuffer.cpp | 5 +++++ > 2 files changed, 6 insertions(+) > > diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h > index 75ac2c8f..9a5355c7 100644 > --- a/include/libcamera/internal/mapped_framebuffer.h > +++ b/include/libcamera/internal/mapped_framebuffer.h > @@ -55,6 +55,7 @@ public: > using MapFlags = Flags<MapFlag>; > > MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags); > + int getPlaneFD(int plane); The argument shouldn't be negative -- should it be unsigned int? > private: > const FrameBuffer *buffer_; > diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp > index f5ee770c..d5f347d4 100644 > --- a/src/libcamera/mapped_framebuffer.cpp > +++ b/src/libcamera/mapped_framebuffer.cpp > @@ -242,4 +242,9 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags) > buffer_ = buffer; > } > > +int MappedFrameBuffer::getPlaneFD(int plane) > +{ > + return buffer_->planes()[plane].fd.get(); > +} > + > } /* namespace libcamera */
On 16/06/2025 18:22, Milan Zamazal wrote: >> comes from the fd given to eglCreateImageKHR. > So it's for future improvements not present in the current version, > right? Right. >> Signed-off-by: Bryan O'Donoghue<bryan.odonoghue@linaro.org> >> --- >> include/libcamera/internal/mapped_framebuffer.h | 1 + >> src/libcamera/mapped_framebuffer.cpp | 5 +++++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h >> index 75ac2c8f..9a5355c7 100644 >> --- a/include/libcamera/internal/mapped_framebuffer.h >> +++ b/include/libcamera/internal/mapped_framebuffer.h >> @@ -55,6 +55,7 @@ public: >> using MapFlags = Flags<MapFlag>; >> >> MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags); >> + int getPlaneFD(int plane); > The argument shouldn't be negative -- should it be unsigned int? Ah maybe, I thought I was returning the type "int" of the fd handle. I'll check again for the next version. --- bod
Bryan O'Donoghue <bod.linux@nxsw.ie> writes: > On 16/06/2025 18:22, Milan Zamazal wrote: >>> comes from the fd given to eglCreateImageKHR. >> So it's for future improvements not present in the current version, >> right? > > Right. > >>> Signed-off-by: Bryan O'Donoghue<bryan.odonoghue@linaro.org> >>> --- >>> include/libcamera/internal/mapped_framebuffer.h | 1 + >>> src/libcamera/mapped_framebuffer.cpp | 5 +++++ >>> 2 files changed, 6 insertions(+) >>> >>> diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h >>> index 75ac2c8f..9a5355c7 100644 >>> --- a/include/libcamera/internal/mapped_framebuffer.h >>> +++ b/include/libcamera/internal/mapped_framebuffer.h >>> @@ -55,6 +55,7 @@ public: >>> using MapFlags = Flags<MapFlag>; >>> >>> MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags); >>> + int getPlaneFD(int plane); >> The argument shouldn't be negative -- should it be unsigned int? > > Ah maybe, I thought I was returning the type "int" of the fd handle. To avoid contingent confusion: I'm talking about `plane' type, not about the return value of the function. > I'll check again for the next version. > > --- > bod
Quoting Bryan O'Donoghue (2025-06-11 02:32:17) > Add MappedFrameBuffer::getPlaneFD() which takes a plane index and returns > the file descriptor associated with it. > > This fd will be used to feed into eglCreateImageKHR for both texture > creation on upload and directly render-to-texture where the texture buffer > comes from the fd given to eglCreateImageKHR. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > include/libcamera/internal/mapped_framebuffer.h | 1 + > src/libcamera/mapped_framebuffer.cpp | 5 +++++ > 2 files changed, 6 insertions(+) > > diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h > index 75ac2c8f..9a5355c7 100644 > --- a/include/libcamera/internal/mapped_framebuffer.h > +++ b/include/libcamera/internal/mapped_framebuffer.h > @@ -55,6 +55,7 @@ public: > using MapFlags = Flags<MapFlag>; > > MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags); > + int getPlaneFD(int plane); > > private: > const FrameBuffer *buffer_; > diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp > index f5ee770c..d5f347d4 100644 > --- a/src/libcamera/mapped_framebuffer.cpp > +++ b/src/libcamera/mapped_framebuffer.cpp > @@ -242,4 +242,9 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags) > buffer_ = buffer; > } > > +int MappedFrameBuffer::getPlaneFD(int plane) > +{ > + return buffer_->planes()[plane].fd.get(); Given the intrinsic usage of buffer_ here and how buffer_ isn't used in the previous patch - I think I'd ask to merge these two patches to keep the rationale together. There should also probably be a if plane > buffer_->planes().size() return -EINVAL; too, though I expect that shouldn't ever get hit. And one reason for merging the two patches is I'm wondering here, will the lifetime of buffer_ be sufficiently tied to the MappedFrameBuffer to ensure it's accessible and valid? I suspect so - but I haven't checked it through. It feels a bit odd that we're actually passing through the underlying internals of the FrameBuffer here through the MappedFrameBuffer ... but I presume that's because at the stage you are operating on a MappedFrameBuffer - you do not have a direct reference to the underlying FrameBuffer? > +} > + > } /* namespace libcamera */ > -- > 2.49.0 >
diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h index 75ac2c8f..9a5355c7 100644 --- a/include/libcamera/internal/mapped_framebuffer.h +++ b/include/libcamera/internal/mapped_framebuffer.h @@ -55,6 +55,7 @@ public: using MapFlags = Flags<MapFlag>; MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags); + int getPlaneFD(int plane); private: const FrameBuffer *buffer_; diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp index f5ee770c..d5f347d4 100644 --- a/src/libcamera/mapped_framebuffer.cpp +++ b/src/libcamera/mapped_framebuffer.cpp @@ -242,4 +242,9 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags) buffer_ = buffer; } +int MappedFrameBuffer::getPlaneFD(int plane) +{ + return buffer_->planes()[plane].fd.get(); +} + } /* namespace libcamera */
Add MappedFrameBuffer::getPlaneFD() which takes a plane index and returns the file descriptor associated with it. This fd will be used to feed into eglCreateImageKHR for both texture creation on upload and directly render-to-texture where the texture buffer comes from the fd given to eglCreateImageKHR. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- include/libcamera/internal/mapped_framebuffer.h | 1 + src/libcamera/mapped_framebuffer.cpp | 5 +++++ 2 files changed, 6 insertions(+)