| Message ID | 20200623190836.53446-3-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Wed, Jun 24, 2020 at 04:08:16AM +0900, Paul Elder wrote: > Prepare for using the V4L2CameraFile as a container for file-specific > information in the V4L2 compatibility layer by making it a required > argument for all V4L2CameraProxy calls that are directed from > V4L2CompatManager, which are intercepted via LD_PRELOAD. Change > V4L2CameraFile accordingly. > > Also change V4L2CompatManager accordingly. Instead of keeping a map of > file descriptors to V4L2CameraProxy instances, we keep a map of > V4L2CameraFile instances to V4L2CameraProxy instances. When the > proxy methods are called, feed the file as a parameter. > > The dup function is also modified, in that it is removed from > V4L2CameraProxy, and is handled completely in V4L2CompatManager, as a > map from file descriptors to V4L2CameraFile instances. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > New in v3 > - split from "v4l2: v4l2_compat: Support multiple open" > --- > src/v4l2/v4l2_camera_file.cpp | 4 +- > src/v4l2/v4l2_camera_proxy.cpp | 89 +++++++++++++++----------------- > src/v4l2/v4l2_camera_proxy.h | 31 ++++++----- > src/v4l2/v4l2_compat_manager.cpp | 69 +++++++++++-------------- > src/v4l2/v4l2_compat_manager.h | 4 +- > 5 files changed, 90 insertions(+), 107 deletions(-) > > diff --git a/src/v4l2/v4l2_camera_file.cpp b/src/v4l2/v4l2_camera_file.cpp > index d3232ab..a07679b 100644 > --- a/src/v4l2/v4l2_camera_file.cpp > +++ b/src/v4l2/v4l2_camera_file.cpp > @@ -17,10 +17,10 @@ V4L2CameraFile::V4L2CameraFile(int efd, bool nonBlocking, V4L2CameraProxy *proxy > : proxy_(proxy), nonBlocking_(nonBlocking), efd_(efd), > priority_(V4L2_PRIORITY_DEFAULT) > { > - proxy_->open(nonBlocking); > + proxy_->open(this); > } > > V4L2CameraFile::~V4L2CameraFile() > { > - proxy_->close(); > + proxy_->close(this); > } > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > index bf47aa7..c1ee1be 100644 > --- a/src/v4l2/v4l2_camera_proxy.cpp > +++ b/src/v4l2/v4l2_camera_proxy.cpp > @@ -23,6 +23,7 @@ > #include "libcamera/internal/utils.h" > > #include "v4l2_camera.h" > +#include "v4l2_camera_file.h" > #include "v4l2_compat_manager.h" > > #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c)) > @@ -39,9 +40,9 @@ V4L2CameraProxy::V4L2CameraProxy(unsigned int index, > querycap(camera); > } > > -int V4L2CameraProxy::open(bool nonBlocking) > +int V4L2CameraProxy::open(V4L2CameraFile *file) > { > - LOG(V4L2Compat, Debug) << "Servicing open"; > + LOG(V4L2Compat, Debug) << "Servicing open fd = " << file->efd(); > > int ret = vcam_->open(); > if (ret < 0) { > @@ -49,8 +50,6 @@ int V4L2CameraProxy::open(bool nonBlocking) > return -1; > } > > - nonBlocking_ = nonBlocking; > - > vcam_->getStreamConfig(&streamConfig_); > setFmtFromConfig(streamConfig_); > sizeimage_ = calculateSizeImage(streamConfig_); > @@ -60,14 +59,10 @@ int V4L2CameraProxy::open(bool nonBlocking) > return 0; > } > > -void V4L2CameraProxy::dup() > +void V4L2CameraProxy::close(V4L2CameraFile *file) > { > - refcount_++; > -} > + LOG(V4L2Compat, Debug) << "Servicing close fd = " << file->efd(); > > -void V4L2CameraProxy::close() > -{ > - LOG(V4L2Compat, Debug) << "Servicing close"; > > if (--refcount_ > 0) > return; > @@ -221,9 +216,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg) > return 0; > } > > -int V4L2CameraProxy::vidioc_enum_fmt(struct v4l2_fmtdesc *arg) > +int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_fmt"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_fmt fd = " << file->efd(); > > if (!validateBufferType(arg->type) || > arg->index >= streamConfig_.formats().pixelformats().size()) > @@ -237,9 +232,9 @@ int V4L2CameraProxy::vidioc_enum_fmt(struct v4l2_fmtdesc *arg) > return 0; > } > > -int V4L2CameraProxy::vidioc_g_fmt(struct v4l2_format *arg) > +int V4L2CameraProxy::vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_g_fmt"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_g_fmt fd = " << file->efd(); > > if (!validateBufferType(arg->type)) > return -EINVAL; > @@ -275,9 +270,9 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg) > arg->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB; > } > > -int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg) > +int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_s_fmt"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_s_fmt fd = " << file->efd(); > > if (!validateBufferType(arg->type)) > return -EINVAL; > @@ -302,9 +297,9 @@ int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg) > return 0; > } > > -int V4L2CameraProxy::vidioc_try_fmt(struct v4l2_format *arg) > +int V4L2CameraProxy::vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_try_fmt"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_try_fmt fd = " << file->efd(); > > if (!validateBufferType(arg->type)) > return -EINVAL; > @@ -329,11 +324,9 @@ int V4L2CameraProxy::freeBuffers() > return 0; > } > > -int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg) > +int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg) > { > - int ret; > - > - LOG(V4L2Compat, Debug) << "Servicing vidioc_reqbufs"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_reqbufs fd = " << file->efd(); > > if (!validateBufferType(arg->type) || > !validateMemoryType(arg->memory)) > @@ -347,9 +340,9 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg) > return freeBuffers(); > > Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height); > - ret = vcam_->configure(&streamConfig_, size, > - v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat), > - arg->count); > + int ret = vcam_->configure(&streamConfig_, size, > + v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat), > + arg->count); > if (ret < 0) > return -EINVAL; > > @@ -396,9 +389,9 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg) > return 0; > } > > -int V4L2CameraProxy::vidioc_querybuf(struct v4l2_buffer *arg) > +int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_querybuf"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_querybuf fd = " << file->efd(); > > if (!validateBufferType(arg->type) || > arg->index >= bufferCount_) > @@ -411,10 +404,10 @@ int V4L2CameraProxy::vidioc_querybuf(struct v4l2_buffer *arg) > return 0; > } > > -int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg) > +int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) > { > LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = " > - << arg->index; > + << arg->index << " fd = " << file->efd(); > > if (!validateBufferType(arg->type) || > !validateMemoryType(arg->memory) || > @@ -431,15 +424,15 @@ int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg) > return ret; > } > > -int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg) > +int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_dqbuf"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_dqbuf fd = " << file->efd(); > > if (!validateBufferType(arg->type) || > !validateMemoryType(arg->memory)) > return -EINVAL; > > - if (!nonBlocking_) > + if (!(file->nonBlocking())) No need for parentheses around file->nonBlock(). > vcam_->bufferSema_.acquire(); > else if (!vcam_->bufferSema_.tryAcquire()) > return -EAGAIN; > @@ -455,16 +448,16 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg) > currentBuf_ = (currentBuf_ + 1) % bufferCount_; > > uint64_t data; > - int ret = ::read(efd_, &data, sizeof(data)); > + int ret = ::read(file->efd(), &data, sizeof(data)); > if (ret != sizeof(data)) > LOG(V4L2Compat, Error) << "Failed to clear eventfd POLLIN"; > > return 0; > } > > -int V4L2CameraProxy::vidioc_streamon(int *arg) > +int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_streamon"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_streamon fd = " << file->efd(); > > if (!validateBufferType(*arg)) > return -EINVAL; > @@ -474,9 +467,9 @@ int V4L2CameraProxy::vidioc_streamon(int *arg) > return vcam_->streamOn(); > } > > -int V4L2CameraProxy::vidioc_streamoff(int *arg) > +int V4L2CameraProxy::vidioc_streamoff(V4L2CameraFile *file, int *arg) > { > - LOG(V4L2Compat, Debug) << "Servicing vidioc_streamoff"; > + LOG(V4L2Compat, Debug) << "Servicing vidioc_streamoff fd = " << file->efd(); > > if (!validateBufferType(*arg)) > return -EINVAL; > @@ -489,7 +482,7 @@ int V4L2CameraProxy::vidioc_streamoff(int *arg) > return ret; > } > > -int V4L2CameraProxy::ioctl(unsigned long request, void *arg) > +int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *arg) > { > int ret; > switch (request) { > @@ -497,34 +490,34 @@ int V4L2CameraProxy::ioctl(unsigned long request, void *arg) > ret = vidioc_querycap(static_cast<struct v4l2_capability *>(arg)); > break; > case VIDIOC_ENUM_FMT: > - ret = vidioc_enum_fmt(static_cast<struct v4l2_fmtdesc *>(arg)); > + ret = vidioc_enum_fmt(file, static_cast<struct v4l2_fmtdesc *>(arg)); > break; > case VIDIOC_G_FMT: > - ret = vidioc_g_fmt(static_cast<struct v4l2_format *>(arg)); > + ret = vidioc_g_fmt(file, static_cast<struct v4l2_format *>(arg)); > break; > case VIDIOC_S_FMT: > - ret = vidioc_s_fmt(static_cast<struct v4l2_format *>(arg)); > + ret = vidioc_s_fmt(file, static_cast<struct v4l2_format *>(arg)); > break; > case VIDIOC_TRY_FMT: > - ret = vidioc_try_fmt(static_cast<struct v4l2_format *>(arg)); > + ret = vidioc_try_fmt(file, static_cast<struct v4l2_format *>(arg)); > break; > case VIDIOC_REQBUFS: > - ret = vidioc_reqbufs(static_cast<struct v4l2_requestbuffers *>(arg)); > + ret = vidioc_reqbufs(file, static_cast<struct v4l2_requestbuffers *>(arg)); > break; > case VIDIOC_QUERYBUF: > - ret = vidioc_querybuf(static_cast<struct v4l2_buffer *>(arg)); > + ret = vidioc_querybuf(file, static_cast<struct v4l2_buffer *>(arg)); > break; > case VIDIOC_QBUF: > - ret = vidioc_qbuf(static_cast<struct v4l2_buffer *>(arg)); > + ret = vidioc_qbuf(file, static_cast<struct v4l2_buffer *>(arg)); > break; > case VIDIOC_DQBUF: > - ret = vidioc_dqbuf(static_cast<struct v4l2_buffer *>(arg)); > + ret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg)); > break; > case VIDIOC_STREAMON: > - ret = vidioc_streamon(static_cast<int *>(arg)); > + ret = vidioc_streamon(file, static_cast<int *>(arg)); > break; > case VIDIOC_STREAMOFF: > - ret = vidioc_streamoff(static_cast<int *>(arg)); > + ret = vidioc_streamoff(file, static_cast<int *>(arg)); > break; > default: > ret = -ENOTTY; > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h > index 27d3e50..b2197ef 100644 > --- a/src/v4l2/v4l2_camera_proxy.h > +++ b/src/v4l2/v4l2_camera_proxy.h > @@ -21,20 +21,20 @@ > > using namespace libcamera; > > +class V4L2CameraFile; > + > class V4L2CameraProxy > { > public: > V4L2CameraProxy(unsigned int index, std::shared_ptr<Camera> camera); > > - int open(bool nonBlocking); > - void dup(); > - void close(); > + int open(V4L2CameraFile *file); > + void close(V4L2CameraFile *file); > void *mmap(void *addr, size_t length, int prot, int flags, off64_t offset); > int munmap(void *addr, size_t length); > > - int ioctl(unsigned long request, void *arg); > - > void bind(int fd); > + int ioctl(V4L2CameraFile *file, unsigned long request, void *arg); > > private: > bool validateBufferType(uint32_t type); > @@ -47,16 +47,16 @@ private: > int freeBuffers(); > > int vidioc_querycap(struct v4l2_capability *arg); > - int vidioc_enum_fmt(struct v4l2_fmtdesc *arg); > - int vidioc_g_fmt(struct v4l2_format *arg); > - int vidioc_s_fmt(struct v4l2_format *arg); > - int vidioc_try_fmt(struct v4l2_format *arg); > - int vidioc_reqbufs(struct v4l2_requestbuffers *arg); > - int vidioc_querybuf(struct v4l2_buffer *arg); > - int vidioc_qbuf(struct v4l2_buffer *arg); > - int vidioc_dqbuf(struct v4l2_buffer *arg); > - int vidioc_streamon(int *arg); > - int vidioc_streamoff(int *arg); > + int vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *arg); > + int vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg); > + int vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg); > + int vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *arg); > + int vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg); > + int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg); > + int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg); > + int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg); > + int vidioc_streamon(V4L2CameraFile *file, int *arg); > + int vidioc_streamoff(V4L2CameraFile *file, int *arg); > > static unsigned int bplMultiplier(uint32_t format); > static unsigned int imageSize(uint32_t format, unsigned int width, > @@ -67,7 +67,6 @@ private: > > unsigned int refcount_; > unsigned int index_; > - bool nonBlocking_; > > struct v4l2_format curV4L2Format_; > StreamConfiguration streamConfig_; > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp > index 8da3316..8118170 100644 > --- a/src/v4l2/v4l2_compat_manager.cpp > +++ b/src/v4l2/v4l2_compat_manager.cpp > @@ -24,6 +24,8 @@ > > #include "libcamera/internal/log.h" > > +#include "v4l2_camera_file.h" > + > using namespace libcamera; > > LOG_DEFINE_CATEGORY(V4L2Compat) > @@ -49,7 +51,7 @@ V4L2CompatManager::V4L2CompatManager() > > V4L2CompatManager::~V4L2CompatManager() > { > - devices_.clear(); > + files_.clear(); > mmaps_.clear(); > > if (cm_) { > @@ -95,13 +97,13 @@ V4L2CompatManager *V4L2CompatManager::instance() > return &instance; > } > > -V4L2CameraProxy *V4L2CompatManager::getProxy(int fd) > +std::shared_ptr<V4L2CameraFile> V4L2CompatManager::cameraFile(int fd) > { > - auto device = devices_.find(fd); > - if (device == devices_.end()) > - return nullptr; > + auto file = files_.find(fd); > + if (file == files_.end()) > + return std::shared_ptr<V4L2CameraFile>(); I think you can return nullptr directly as the corresponding std::shared_ptr constructor is not explicit: constexpr shared_ptr( std::nullptr_t ) noexcept; (https://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr) > > - return device->second; > + return file->second; > } > > int V4L2CompatManager::getCameraIndex(int fd) > @@ -148,25 +150,15 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod > > fops_.close(fd); > > - unsigned int camera_index = static_cast<unsigned int>(ret); > - > - V4L2CameraProxy *proxy = proxies_[camera_index].get(); > - ret = proxy->open(oflag & O_NONBLOCK); > - if (ret < 0) > - return ret; > - > int efd = eventfd(0, EFD_SEMAPHORE | > ((oflag & O_CLOEXEC) ? EFD_CLOEXEC : 0) | > ((oflag & O_NONBLOCK) ? EFD_NONBLOCK : 0)); > - if (efd < 0) { > - int err = errno; > - proxy->close(); > - errno = err; > + if (efd < 0) > return efd; > - } > > - proxy->bind(efd); > - devices_.emplace(efd, proxy); > + unsigned int index = static_cast<unsigned int>(ret); > + V4L2CameraProxy *proxy = proxies_[index].get(); Can you use ret directly and remove the index variable ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + files_.emplace(efd, std::make_shared<V4L2CameraFile>(efd, oflag & O_NONBLOCK, proxy)); > > return efd; > } > @@ -177,40 +169,39 @@ int V4L2CompatManager::dup(int oldfd) > if (newfd < 0) > return newfd; > > - auto device = devices_.find(oldfd); > - if (device != devices_.end()) { > - V4L2CameraProxy *proxy = device->second; > - devices_[newfd] = proxy; > - proxy->dup(); > - } > + auto file = files_.find(oldfd); > + if (file != files_.end()) > + files_[newfd] = file->second; > > return newfd; > } > > int V4L2CompatManager::close(int fd) > { > - V4L2CameraProxy *proxy = getProxy(fd); > - if (proxy) { > - proxy->close(); > - devices_.erase(fd); > - return 0; > - } > + auto file = files_.find(fd); > + if (file != files_.end()) > + files_.erase(file); > > + /* We still need to close the eventfd. */ > return fops_.close(fd); > } > > void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags, > int fd, off64_t offset) > { > - V4L2CameraProxy *proxy = getProxy(fd); > - if (!proxy) > + std::shared_ptr<V4L2CameraFile> file = cameraFile(fd); > + if (!file) > return fops_.mmap(addr, length, prot, flags, fd, offset); > > - void *map = proxy->mmap(addr, length, prot, flags, offset); > + void *map = file->proxy()->mmap(addr, length, prot, flags, offset); > if (map == MAP_FAILED) > return map; > > - mmaps_[map] = proxy; > + /* > + * Map to V4L2CameraProxy directly to prevent adding more references > + * to V4L2CameraFile. > + */ > + mmaps_[map] = file->proxy(); > return map; > } > > @@ -233,9 +224,9 @@ int V4L2CompatManager::munmap(void *addr, size_t length) > > int V4L2CompatManager::ioctl(int fd, unsigned long request, void *arg) > { > - V4L2CameraProxy *proxy = getProxy(fd); > - if (!proxy) > + std::shared_ptr<V4L2CameraFile> file = cameraFile(fd); > + if (!file) > return fops_.ioctl(fd, request, arg); > > - return proxy->ioctl(request, arg); > + return file->proxy()->ioctl(file.get(), request, arg); > } > diff --git a/src/v4l2/v4l2_compat_manager.h b/src/v4l2/v4l2_compat_manager.h > index 3d4e512..bc548ab 100644 > --- a/src/v4l2/v4l2_compat_manager.h > +++ b/src/v4l2/v4l2_compat_manager.h > @@ -44,7 +44,6 @@ public: > > static V4L2CompatManager *instance(); > > - V4L2CameraProxy *getProxy(int fd); > const FileOperations &fops() const { return fops_; } > > int openat(int dirfd, const char *path, int oflag, mode_t mode); > @@ -62,13 +61,14 @@ private: > > int start(); > int getCameraIndex(int fd); > + std::shared_ptr<V4L2CameraFile> cameraFile(int fd); > > FileOperations fops_; > > CameraManager *cm_; > > std::vector<std::unique_ptr<V4L2CameraProxy>> proxies_; > - std::map<int, V4L2CameraProxy *> devices_; > + std::map<int, std::shared_ptr<V4L2CameraFile>> files_; > std::map<void *, V4L2CameraProxy *> mmaps_; > }; >
diff --git a/src/v4l2/v4l2_camera_file.cpp b/src/v4l2/v4l2_camera_file.cpp index d3232ab..a07679b 100644 --- a/src/v4l2/v4l2_camera_file.cpp +++ b/src/v4l2/v4l2_camera_file.cpp @@ -17,10 +17,10 @@ V4L2CameraFile::V4L2CameraFile(int efd, bool nonBlocking, V4L2CameraProxy *proxy : proxy_(proxy), nonBlocking_(nonBlocking), efd_(efd), priority_(V4L2_PRIORITY_DEFAULT) { - proxy_->open(nonBlocking); + proxy_->open(this); } V4L2CameraFile::~V4L2CameraFile() { - proxy_->close(); + proxy_->close(this); } diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index bf47aa7..c1ee1be 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -23,6 +23,7 @@ #include "libcamera/internal/utils.h" #include "v4l2_camera.h" +#include "v4l2_camera_file.h" #include "v4l2_compat_manager.h" #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c)) @@ -39,9 +40,9 @@ V4L2CameraProxy::V4L2CameraProxy(unsigned int index, querycap(camera); } -int V4L2CameraProxy::open(bool nonBlocking) +int V4L2CameraProxy::open(V4L2CameraFile *file) { - LOG(V4L2Compat, Debug) << "Servicing open"; + LOG(V4L2Compat, Debug) << "Servicing open fd = " << file->efd(); int ret = vcam_->open(); if (ret < 0) { @@ -49,8 +50,6 @@ int V4L2CameraProxy::open(bool nonBlocking) return -1; } - nonBlocking_ = nonBlocking; - vcam_->getStreamConfig(&streamConfig_); setFmtFromConfig(streamConfig_); sizeimage_ = calculateSizeImage(streamConfig_); @@ -60,14 +59,10 @@ int V4L2CameraProxy::open(bool nonBlocking) return 0; } -void V4L2CameraProxy::dup() +void V4L2CameraProxy::close(V4L2CameraFile *file) { - refcount_++; -} + LOG(V4L2Compat, Debug) << "Servicing close fd = " << file->efd(); -void V4L2CameraProxy::close() -{ - LOG(V4L2Compat, Debug) << "Servicing close"; if (--refcount_ > 0) return; @@ -221,9 +216,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg) return 0; } -int V4L2CameraProxy::vidioc_enum_fmt(struct v4l2_fmtdesc *arg) +int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_fmt"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_fmt fd = " << file->efd(); if (!validateBufferType(arg->type) || arg->index >= streamConfig_.formats().pixelformats().size()) @@ -237,9 +232,9 @@ int V4L2CameraProxy::vidioc_enum_fmt(struct v4l2_fmtdesc *arg) return 0; } -int V4L2CameraProxy::vidioc_g_fmt(struct v4l2_format *arg) +int V4L2CameraProxy::vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_g_fmt"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_g_fmt fd = " << file->efd(); if (!validateBufferType(arg->type)) return -EINVAL; @@ -275,9 +270,9 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg) arg->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB; } -int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg) +int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_s_fmt"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_s_fmt fd = " << file->efd(); if (!validateBufferType(arg->type)) return -EINVAL; @@ -302,9 +297,9 @@ int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg) return 0; } -int V4L2CameraProxy::vidioc_try_fmt(struct v4l2_format *arg) +int V4L2CameraProxy::vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_try_fmt"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_try_fmt fd = " << file->efd(); if (!validateBufferType(arg->type)) return -EINVAL; @@ -329,11 +324,9 @@ int V4L2CameraProxy::freeBuffers() return 0; } -int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg) +int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg) { - int ret; - - LOG(V4L2Compat, Debug) << "Servicing vidioc_reqbufs"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_reqbufs fd = " << file->efd(); if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory)) @@ -347,9 +340,9 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg) return freeBuffers(); Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height); - ret = vcam_->configure(&streamConfig_, size, - v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat), - arg->count); + int ret = vcam_->configure(&streamConfig_, size, + v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat), + arg->count); if (ret < 0) return -EINVAL; @@ -396,9 +389,9 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg) return 0; } -int V4L2CameraProxy::vidioc_querybuf(struct v4l2_buffer *arg) +int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_querybuf"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_querybuf fd = " << file->efd(); if (!validateBufferType(arg->type) || arg->index >= bufferCount_) @@ -411,10 +404,10 @@ int V4L2CameraProxy::vidioc_querybuf(struct v4l2_buffer *arg) return 0; } -int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg) +int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) { LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = " - << arg->index; + << arg->index << " fd = " << file->efd(); if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory) || @@ -431,15 +424,15 @@ int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg) return ret; } -int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg) +int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_dqbuf"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_dqbuf fd = " << file->efd(); if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory)) return -EINVAL; - if (!nonBlocking_) + if (!(file->nonBlocking())) vcam_->bufferSema_.acquire(); else if (!vcam_->bufferSema_.tryAcquire()) return -EAGAIN; @@ -455,16 +448,16 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg) currentBuf_ = (currentBuf_ + 1) % bufferCount_; uint64_t data; - int ret = ::read(efd_, &data, sizeof(data)); + int ret = ::read(file->efd(), &data, sizeof(data)); if (ret != sizeof(data)) LOG(V4L2Compat, Error) << "Failed to clear eventfd POLLIN"; return 0; } -int V4L2CameraProxy::vidioc_streamon(int *arg) +int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_streamon"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_streamon fd = " << file->efd(); if (!validateBufferType(*arg)) return -EINVAL; @@ -474,9 +467,9 @@ int V4L2CameraProxy::vidioc_streamon(int *arg) return vcam_->streamOn(); } -int V4L2CameraProxy::vidioc_streamoff(int *arg) +int V4L2CameraProxy::vidioc_streamoff(V4L2CameraFile *file, int *arg) { - LOG(V4L2Compat, Debug) << "Servicing vidioc_streamoff"; + LOG(V4L2Compat, Debug) << "Servicing vidioc_streamoff fd = " << file->efd(); if (!validateBufferType(*arg)) return -EINVAL; @@ -489,7 +482,7 @@ int V4L2CameraProxy::vidioc_streamoff(int *arg) return ret; } -int V4L2CameraProxy::ioctl(unsigned long request, void *arg) +int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *arg) { int ret; switch (request) { @@ -497,34 +490,34 @@ int V4L2CameraProxy::ioctl(unsigned long request, void *arg) ret = vidioc_querycap(static_cast<struct v4l2_capability *>(arg)); break; case VIDIOC_ENUM_FMT: - ret = vidioc_enum_fmt(static_cast<struct v4l2_fmtdesc *>(arg)); + ret = vidioc_enum_fmt(file, static_cast<struct v4l2_fmtdesc *>(arg)); break; case VIDIOC_G_FMT: - ret = vidioc_g_fmt(static_cast<struct v4l2_format *>(arg)); + ret = vidioc_g_fmt(file, static_cast<struct v4l2_format *>(arg)); break; case VIDIOC_S_FMT: - ret = vidioc_s_fmt(static_cast<struct v4l2_format *>(arg)); + ret = vidioc_s_fmt(file, static_cast<struct v4l2_format *>(arg)); break; case VIDIOC_TRY_FMT: - ret = vidioc_try_fmt(static_cast<struct v4l2_format *>(arg)); + ret = vidioc_try_fmt(file, static_cast<struct v4l2_format *>(arg)); break; case VIDIOC_REQBUFS: - ret = vidioc_reqbufs(static_cast<struct v4l2_requestbuffers *>(arg)); + ret = vidioc_reqbufs(file, static_cast<struct v4l2_requestbuffers *>(arg)); break; case VIDIOC_QUERYBUF: - ret = vidioc_querybuf(static_cast<struct v4l2_buffer *>(arg)); + ret = vidioc_querybuf(file, static_cast<struct v4l2_buffer *>(arg)); break; case VIDIOC_QBUF: - ret = vidioc_qbuf(static_cast<struct v4l2_buffer *>(arg)); + ret = vidioc_qbuf(file, static_cast<struct v4l2_buffer *>(arg)); break; case VIDIOC_DQBUF: - ret = vidioc_dqbuf(static_cast<struct v4l2_buffer *>(arg)); + ret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg)); break; case VIDIOC_STREAMON: - ret = vidioc_streamon(static_cast<int *>(arg)); + ret = vidioc_streamon(file, static_cast<int *>(arg)); break; case VIDIOC_STREAMOFF: - ret = vidioc_streamoff(static_cast<int *>(arg)); + ret = vidioc_streamoff(file, static_cast<int *>(arg)); break; default: ret = -ENOTTY; diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h index 27d3e50..b2197ef 100644 --- a/src/v4l2/v4l2_camera_proxy.h +++ b/src/v4l2/v4l2_camera_proxy.h @@ -21,20 +21,20 @@ using namespace libcamera; +class V4L2CameraFile; + class V4L2CameraProxy { public: V4L2CameraProxy(unsigned int index, std::shared_ptr<Camera> camera); - int open(bool nonBlocking); - void dup(); - void close(); + int open(V4L2CameraFile *file); + void close(V4L2CameraFile *file); void *mmap(void *addr, size_t length, int prot, int flags, off64_t offset); int munmap(void *addr, size_t length); - int ioctl(unsigned long request, void *arg); - void bind(int fd); + int ioctl(V4L2CameraFile *file, unsigned long request, void *arg); private: bool validateBufferType(uint32_t type); @@ -47,16 +47,16 @@ private: int freeBuffers(); int vidioc_querycap(struct v4l2_capability *arg); - int vidioc_enum_fmt(struct v4l2_fmtdesc *arg); - int vidioc_g_fmt(struct v4l2_format *arg); - int vidioc_s_fmt(struct v4l2_format *arg); - int vidioc_try_fmt(struct v4l2_format *arg); - int vidioc_reqbufs(struct v4l2_requestbuffers *arg); - int vidioc_querybuf(struct v4l2_buffer *arg); - int vidioc_qbuf(struct v4l2_buffer *arg); - int vidioc_dqbuf(struct v4l2_buffer *arg); - int vidioc_streamon(int *arg); - int vidioc_streamoff(int *arg); + int vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *arg); + int vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg); + int vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg); + int vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *arg); + int vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg); + int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg); + int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg); + int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg); + int vidioc_streamon(V4L2CameraFile *file, int *arg); + int vidioc_streamoff(V4L2CameraFile *file, int *arg); static unsigned int bplMultiplier(uint32_t format); static unsigned int imageSize(uint32_t format, unsigned int width, @@ -67,7 +67,6 @@ private: unsigned int refcount_; unsigned int index_; - bool nonBlocking_; struct v4l2_format curV4L2Format_; StreamConfiguration streamConfig_; diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp index 8da3316..8118170 100644 --- a/src/v4l2/v4l2_compat_manager.cpp +++ b/src/v4l2/v4l2_compat_manager.cpp @@ -24,6 +24,8 @@ #include "libcamera/internal/log.h" +#include "v4l2_camera_file.h" + using namespace libcamera; LOG_DEFINE_CATEGORY(V4L2Compat) @@ -49,7 +51,7 @@ V4L2CompatManager::V4L2CompatManager() V4L2CompatManager::~V4L2CompatManager() { - devices_.clear(); + files_.clear(); mmaps_.clear(); if (cm_) { @@ -95,13 +97,13 @@ V4L2CompatManager *V4L2CompatManager::instance() return &instance; } -V4L2CameraProxy *V4L2CompatManager::getProxy(int fd) +std::shared_ptr<V4L2CameraFile> V4L2CompatManager::cameraFile(int fd) { - auto device = devices_.find(fd); - if (device == devices_.end()) - return nullptr; + auto file = files_.find(fd); + if (file == files_.end()) + return std::shared_ptr<V4L2CameraFile>(); - return device->second; + return file->second; } int V4L2CompatManager::getCameraIndex(int fd) @@ -148,25 +150,15 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod fops_.close(fd); - unsigned int camera_index = static_cast<unsigned int>(ret); - - V4L2CameraProxy *proxy = proxies_[camera_index].get(); - ret = proxy->open(oflag & O_NONBLOCK); - if (ret < 0) - return ret; - int efd = eventfd(0, EFD_SEMAPHORE | ((oflag & O_CLOEXEC) ? EFD_CLOEXEC : 0) | ((oflag & O_NONBLOCK) ? EFD_NONBLOCK : 0)); - if (efd < 0) { - int err = errno; - proxy->close(); - errno = err; + if (efd < 0) return efd; - } - proxy->bind(efd); - devices_.emplace(efd, proxy); + unsigned int index = static_cast<unsigned int>(ret); + V4L2CameraProxy *proxy = proxies_[index].get(); + files_.emplace(efd, std::make_shared<V4L2CameraFile>(efd, oflag & O_NONBLOCK, proxy)); return efd; } @@ -177,40 +169,39 @@ int V4L2CompatManager::dup(int oldfd) if (newfd < 0) return newfd; - auto device = devices_.find(oldfd); - if (device != devices_.end()) { - V4L2CameraProxy *proxy = device->second; - devices_[newfd] = proxy; - proxy->dup(); - } + auto file = files_.find(oldfd); + if (file != files_.end()) + files_[newfd] = file->second; return newfd; } int V4L2CompatManager::close(int fd) { - V4L2CameraProxy *proxy = getProxy(fd); - if (proxy) { - proxy->close(); - devices_.erase(fd); - return 0; - } + auto file = files_.find(fd); + if (file != files_.end()) + files_.erase(file); + /* We still need to close the eventfd. */ return fops_.close(fd); } void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags, int fd, off64_t offset) { - V4L2CameraProxy *proxy = getProxy(fd); - if (!proxy) + std::shared_ptr<V4L2CameraFile> file = cameraFile(fd); + if (!file) return fops_.mmap(addr, length, prot, flags, fd, offset); - void *map = proxy->mmap(addr, length, prot, flags, offset); + void *map = file->proxy()->mmap(addr, length, prot, flags, offset); if (map == MAP_FAILED) return map; - mmaps_[map] = proxy; + /* + * Map to V4L2CameraProxy directly to prevent adding more references + * to V4L2CameraFile. + */ + mmaps_[map] = file->proxy(); return map; } @@ -233,9 +224,9 @@ int V4L2CompatManager::munmap(void *addr, size_t length) int V4L2CompatManager::ioctl(int fd, unsigned long request, void *arg) { - V4L2CameraProxy *proxy = getProxy(fd); - if (!proxy) + std::shared_ptr<V4L2CameraFile> file = cameraFile(fd); + if (!file) return fops_.ioctl(fd, request, arg); - return proxy->ioctl(request, arg); + return file->proxy()->ioctl(file.get(), request, arg); } diff --git a/src/v4l2/v4l2_compat_manager.h b/src/v4l2/v4l2_compat_manager.h index 3d4e512..bc548ab 100644 --- a/src/v4l2/v4l2_compat_manager.h +++ b/src/v4l2/v4l2_compat_manager.h @@ -44,7 +44,6 @@ public: static V4L2CompatManager *instance(); - V4L2CameraProxy *getProxy(int fd); const FileOperations &fops() const { return fops_; } int openat(int dirfd, const char *path, int oflag, mode_t mode); @@ -62,13 +61,14 @@ private: int start(); int getCameraIndex(int fd); + std::shared_ptr<V4L2CameraFile> cameraFile(int fd); FileOperations fops_; CameraManager *cm_; std::vector<std::unique_ptr<V4L2CameraProxy>> proxies_; - std::map<int, V4L2CameraProxy *> devices_; + std::map<int, std::shared_ptr<V4L2CameraFile>> files_; std::map<void *, V4L2CameraProxy *> mmaps_; };
Prepare for using the V4L2CameraFile as a container for file-specific information in the V4L2 compatibility layer by making it a required argument for all V4L2CameraProxy calls that are directed from V4L2CompatManager, which are intercepted via LD_PRELOAD. Change V4L2CameraFile accordingly. Also change V4L2CompatManager accordingly. Instead of keeping a map of file descriptors to V4L2CameraProxy instances, we keep a map of V4L2CameraFile instances to V4L2CameraProxy instances. When the proxy methods are called, feed the file as a parameter. The dup function is also modified, in that it is removed from V4L2CameraProxy, and is handled completely in V4L2CompatManager, as a map from file descriptors to V4L2CameraFile instances. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v3 - split from "v4l2: v4l2_compat: Support multiple open" --- src/v4l2/v4l2_camera_file.cpp | 4 +- src/v4l2/v4l2_camera_proxy.cpp | 89 +++++++++++++++----------------- src/v4l2/v4l2_camera_proxy.h | 31 ++++++----- src/v4l2/v4l2_compat_manager.cpp | 69 +++++++++++-------------- src/v4l2/v4l2_compat_manager.h | 4 +- 5 files changed, 90 insertions(+), 107 deletions(-)