Message ID | 20191028022525.796995-10-niklas.soderlund@ragnatech.se |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Niklas, Thank you for the patch. On Mon, Oct 28, 2019 at 03:25:22AM +0100, Niklas Söderlund wrote: > In preparation for the new buffer handling add a class which will deal > with keeping the cache between dmafds and V4L2 video device buffer > indexes. Previously this responsibility have been split between multiple s/have been/was/ > classes. > > This initial implement ensures that no hot association is lost while its > eviction strategy could be improved in the future. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/v4l2_videodevice.h | 19 ++++++++++ > src/libcamera/v4l2_videodevice.cpp | 47 ++++++++++++++++++++++++ > 2 files changed, 66 insertions(+) > > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > index 4b8cf9394eb9516f..5b178339d0ce7e2c 100644 > --- a/src/libcamera/include/v4l2_videodevice.h > +++ b/src/libcamera/include/v4l2_videodevice.h > @@ -104,6 +104,25 @@ struct V4L2Capability final : v4l2_capability { > } > }; > > +class V4L2BufferCache > +{ > +public: > + V4L2BufferCache(unsigned int size); > + > + void populate(unsigned int index, const std::array<int, 3> &fds); > + > + int pop(const std::array<int, 3> &fds); > + void push(unsigned int index); I understand why there's no doc at this stage, but you then need to name methods in a self-documenting way :-) Jokes aside, I think better names are needed here. I also fail to see why you need both populate() and pop(), but maybe I'll figure it out in a later patch. > + > +private: > + struct CacheInfo { > + bool free; > + std::array<int, 3> last; > + }; > + > + std::map<unsigned int, CacheInfo> cache_; As indices start at zero and are consecutive, how about using an std::vector<CacheInfo> instead ? > +}; > + > class V4L2DeviceFormat > { > public: > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index a2a9eab2bcc0d7e8..8bc2e439e4faeb68 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -132,6 +132,53 @@ LOG_DECLARE_CATEGORY(V4L2) > * \return True if the video device provides Streaming I/O IOCTLs > */ > > +V4L2BufferCache::V4L2BufferCache(unsigned int size) > +{ > + for (unsigned int i = 0; i < size; i++) > + cache_[i] = { .free = true, .last = { -1, -1, -1 } }; If you turn cache_ into a vector, this could just become cache_.resize(size, { .free = true, .last = { -1, -1, -1 } }); > +} > + > +void V4L2BufferCache::populate(unsigned int index, const std::array<int, 3> &fds) > +{ > + ASSERT(index < cache_.size()); > + cache_[index].last = fds; > +} > + > +int V4L2BufferCache::pop(const std::array<int, 3> &fds) > +{ > + int use = -1; > + > + for (auto &it : cache_) { > + int index = it.first; > + CacheInfo &info = it.second; > + > + if (!info.free) > + continue; > + > + if (use < 0) > + use = index; > + > + if (info.last == fds) { > + use = index; > + break; > + } > + } Or maybe cache_ should be a map from fds to { index, free } to optimize this ? > + > + if (use < 0) > + return -ENOENT; > + > + cache_[use].free = false; > + cache_[use].last = fds; > + > + return use; > +} > + > +void V4L2BufferCache::push(unsigned int index) > +{ > + ASSERT(index < cache_.size()); > + cache_[index].free = true; > +} > + > /** > * \class V4L2DeviceFormat > * \brief The V4L2 video device image format and sizes
diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 4b8cf9394eb9516f..5b178339d0ce7e2c 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -104,6 +104,25 @@ struct V4L2Capability final : v4l2_capability { } }; +class V4L2BufferCache +{ +public: + V4L2BufferCache(unsigned int size); + + void populate(unsigned int index, const std::array<int, 3> &fds); + + int pop(const std::array<int, 3> &fds); + void push(unsigned int index); + +private: + struct CacheInfo { + bool free; + std::array<int, 3> last; + }; + + std::map<unsigned int, CacheInfo> cache_; +}; + class V4L2DeviceFormat { public: diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index a2a9eab2bcc0d7e8..8bc2e439e4faeb68 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -132,6 +132,53 @@ LOG_DECLARE_CATEGORY(V4L2) * \return True if the video device provides Streaming I/O IOCTLs */ +V4L2BufferCache::V4L2BufferCache(unsigned int size) +{ + for (unsigned int i = 0; i < size; i++) + cache_[i] = { .free = true, .last = { -1, -1, -1 } }; +} + +void V4L2BufferCache::populate(unsigned int index, const std::array<int, 3> &fds) +{ + ASSERT(index < cache_.size()); + cache_[index].last = fds; +} + +int V4L2BufferCache::pop(const std::array<int, 3> &fds) +{ + int use = -1; + + for (auto &it : cache_) { + int index = it.first; + CacheInfo &info = it.second; + + if (!info.free) + continue; + + if (use < 0) + use = index; + + if (info.last == fds) { + use = index; + break; + } + } + + if (use < 0) + return -ENOENT; + + cache_[use].free = false; + cache_[use].last = fds; + + return use; +} + +void V4L2BufferCache::push(unsigned int index) +{ + ASSERT(index < cache_.size()); + cache_[index].free = true; +} + /** * \class V4L2DeviceFormat * \brief The V4L2 video device image format and sizes
In preparation for the new buffer handling add a class which will deal with keeping the cache between dmafds and V4L2 video device buffer indexes. Previously this responsibility have been split between multiple classes. This initial implement ensures that no hot association is lost while its eviction strategy could be improved in the future. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/include/v4l2_videodevice.h | 19 ++++++++++ src/libcamera/v4l2_videodevice.cpp | 47 ++++++++++++++++++++++++ 2 files changed, 66 insertions(+)