[libcamera-devel] [PATCH v2 2/2] v4l2: v4l2_compat: Add V4L2 compatibility layer
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Dec 16 01:18:44 CET 2019
Hi Paul,
Thank you for the patch.
On Mon, Dec 09, 2019 at 11:30:13AM +0100, Jacopo Mondi wrote:
> On Sun, Dec 08, 2019 at 11:56:03PM -0500, Paul Elder wrote:
> > Add libcamera V4L2 compatibility layer.
> >
> > This initial implementation supports the minimal set of V4L2 operations,
> > which allows getting, setting, and enumerating formats, and streaming
> > frames from a video device. Some data about the wrapped V4L2 video
> > device are hardcoded.
> >
> > Add a build option named 'v4l2' and adjust the build system to
> > selectively compile the V4L2 compatibility layer.
> >
> > Note that until we have a way of mapping V4L2 device nodes to libcamera
> > cameras, the V4L2 compatibility layer will always selects and use the
s/selects/select/
> > first enumerated libcamera camera.
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> >
> > ---
> > Changes in v2:
> > - move all errno acrobatics to V4L2CameraProxy
> > - remove all mentions of dmabuf
> > - make V4L2CompatManager::getCamera return pointer rather than
> > shared_ptr
> > - match V4L2 device nodes to libcamera cameras using Camera::name()
> > compared to /sys/dev/char/maj:min/name (only works for UVC cameras)
> > - in V4L2CompatManager::getCameraIndex()
> > - add -fvisibility=hidden to v4l2 compat
> > - cache the results of V4L2CompatManager::imageSize() within V4L2Camera
> > (where V4L2Camera is interested)
> > - remove V4L2CompatManager::valid[fd|mmap], and where those methods were
> > used, check V4L2CompatManager::getCamera() != nullptr instead
> > - fixed V4L2CompatManager::drmToV4L2() mappings for DRM_FORMAT_BGR888
> > and DRM_FORMAT_RGB888
> > - other cosmetic changes
> > ---
> > meson_options.txt | 5 +
> > src/meson.build | 4 +
> > src/v4l2/meson.build | 30 ++
> > src/v4l2/v4l2_camera.cpp | 299 ++++++++++++++++++++
> > src/v4l2/v4l2_camera.h | 67 +++++
> > src/v4l2/v4l2_camera_proxy.cpp | 452 +++++++++++++++++++++++++++++++
> > src/v4l2/v4l2_camera_proxy.h | 63 +++++
> > src/v4l2/v4l2_compat.cpp | 81 ++++++
> > src/v4l2/v4l2_compat_manager.cpp | 382 ++++++++++++++++++++++++++
> > src/v4l2/v4l2_compat_manager.h | 86 ++++++
> > 10 files changed, 1469 insertions(+)
> > create mode 100644 src/v4l2/meson.build
> > create mode 100644 src/v4l2/v4l2_camera.cpp
> > create mode 100644 src/v4l2/v4l2_camera.h
> > create mode 100644 src/v4l2/v4l2_camera_proxy.cpp
> > create mode 100644 src/v4l2/v4l2_camera_proxy.h
> > create mode 100644 src/v4l2/v4l2_compat.cpp
> > create mode 100644 src/v4l2/v4l2_compat_manager.cpp
> > create mode 100644 src/v4l2/v4l2_compat_manager.h
> >
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 1a328045..b06dd494 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -10,3 +10,8 @@ option('documentation',
> > option('test',
> > type : 'boolean',
> > description: 'Compile and include the tests')
> > +
> > +option('v4l2',
> > + type : 'boolean',
> > + value : false,
> > + description : 'Compile libcamera with V4L2 compatibility layer')
How about 'Compile the V4L2 adaptation layer', as it's not part of
libcamera.so ?
> > diff --git a/src/meson.build b/src/meson.build
> > index 67ad20aa..5adcd61f 100644
> > --- a/src/meson.build
> > +++ b/src/meson.build
> > @@ -6,3 +6,7 @@ subdir('libcamera')
> > subdir('ipa')
> > subdir('cam')
> > subdir('qcam')
> > +
> > +if get_option('v4l2')
> > + subdir('v4l2')
> > +endif
> > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
> > new file mode 100644
> > index 00000000..1650048e
> > --- /dev/null
> > +++ b/src/v4l2/meson.build
> > @@ -0,0 +1,30 @@
> > +v4l2_compat_sources = files([
> > + 'v4l2_camera.cpp',
> > + 'v4l2_camera_proxy.cpp',
> > + 'v4l2_compat.cpp',
> > + 'v4l2_compat_manager.cpp',
> > +])
> > +
> > +v4l2_compat_includes = [
> > + libcamera_includes,
> > + libcamera_internal_includes,
> > +]
> > +
> > +v4l2_compat_cpp_args = [
> > + # Meson enables large file support unconditionally, which redirect file
> > + # operations to 64-bit versions. This results in some symbols being
> > + # renamed, for instance open() being renamed to open64(). As the V4L2
> > + # adaptation wrapper needs to provide both 32-bit and 64-bit versions of
> > + # file operations, disable transparent large file support.
> > + '-U_FILE_OFFSET_BITS',
As explained in my review of v1, I would prefer also defining
_FILE_OFFSET_BITS explicitly.
> > + '-fvisibility=hidden',
> > +]
> > +
> > +v4l2_compat = shared_library('v4l2-compat',
> > + v4l2_compat_sources,
> > + name_prefix : '',
>
> Do you need this ? I tried removing it and it succesfully compiled...
But does it then generate a shared object without the 'lib' prefix ?
> > + install : true,
> > + link_with : libcamera,
> > + include_directories : v4l2_compat_includes,
> > + dependencies : libcamera_deps,
> > + cpp_args : v4l2_compat_cpp_args)
[snip]
Skipping over V4L2Camera and V4L2CameraProxy, I'll get to those
tomorrow.
> > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp
> > new file mode 100644
> > index 00000000..3330e7bc
> > --- /dev/null
> > +++ b/src/v4l2/v4l2_compat.cpp
> > @@ -0,0 +1,81 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * v4l2_compat.cpp - V4L2 compatibility layer
"V4L2 adaptation layer" ?
> > + */
> > +
> > +#include "v4l2_compat_manager.h"
> > +
> > +#include <iostream>
You don't need this header.
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <linux/videodev2.h>
I don't think you need this header either.
> > +#include <stdarg.h>
> > +#include <sys/mman.h>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +
> > +#define LIBCAMERA_PUBLIC __attribute__((visibility("default")))
> > +
> > +using namespace libcamera;
> > +
> > +#define extract_va_arg(type, arg, last) \
> > +{ \
> > + va_list ap; \
> > + va_start(ap, last); \
> > + arg = va_arg(ap, type); \
> > + va_end(ap); \
> > +}
> > +
> > +extern "C" {
I would add a blank line here, as you add a blank line before the
corresponding }.
> > +LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...)
> > +{
> > + mode_t mode = 0;
> > + if (oflag & O_CREAT || oflag & O_TMPFILE)
> > + extract_va_arg(mode_t, mode, oflag);
> > +
> > + return openat(AT_FDCWD, path, oflag, mode);
You could call
return V4L2CompatManager::instance()->openat(AT_FDCWD, path, oflag,
mode);
to avoid one layer of indirection.
> > +}
> > +
> > +LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...)
> > +{
> > + mode_t mode = 0;
> > + if (oflag & O_CREAT || oflag & O_TMPFILE)
> > + extract_va_arg(mode_t, mode, oflag);
> > +
> > + return V4L2CompatManager::instance()->openat(dirfd, path, oflag, mode);
> > +}
> > +
> > +LIBCAMERA_PUBLIC int dup(int oldfd)
> > +{
> > + return V4L2CompatManager::instance()->dup(oldfd);
> > +}
> > +
> > +LIBCAMERA_PUBLIC int close(int fd)
> > +{
> > + return V4L2CompatManager::instance()->close(fd);
> > +}
> > +
> > +LIBCAMERA_PUBLIC void *mmap(void *addr, size_t length, int prot, int flags,
> > + int fd, off_t offset)
> > +{
> > + void *val = V4L2CompatManager::instance()->mmap(addr, length, prot, flags, fd, offset);
> > + return val;
Anything wrong with
return V4L2CompatManager::instance()->mmap(addr, length, prot, flags,
fd, offset);
?
> > +}
> > +
> > +LIBCAMERA_PUBLIC int munmap(void *addr, size_t length)
> > +{
> > + return V4L2CompatManager::instance()->munmap(addr, length);
> > +}
> > +
> > +LIBCAMERA_PUBLIC int ioctl(int fd, unsigned long request, ...)
> > +{
> > + void *arg;
> > + extract_va_arg(void *, arg, request);
> > +
> > + return V4L2CompatManager::instance()->ioctl(fd, request, arg);
> > +}
> > +
> > +}
> > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> > new file mode 100644
> > index 00000000..90416b35
> > --- /dev/null
> > +++ b/src/v4l2/v4l2_compat_manager.cpp
> > @@ -0,0 +1,382 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * v4l2_compat_manager.cpp - V4L2 compatibility manager
> > + */
Missing blank line.
> > +#include "v4l2_compat_manager.h"
> > +
> > +#include <dlfcn.h>
> > +#include <fcntl.h>
> > +#include <fstream>
> > +#include <iostream>
I don't think you need iostream here either.
> > +#include <linux/drm_fourcc.h>
> > +#include <linux/videodev2.h>
> > +#include <map>
> > +#include <stdarg.h>
> > +#include <string.h>
> > +#include <sys/eventfd.h>
> > +#include <sys/mman.h>
> > +#include <sys/stat.h>
> > +#include <sys/sysmacros.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include <libcamera/camera.h>
> > +#include <libcamera/camera_manager.h>
> > +#include <libcamera/stream.h>
> > +
> > +#include "log.h"
> > +
> > +using namespace libcamera;
> > +
> > +LOG_DEFINE_CATEGORY(V4L2Compat)
> > +
> > +V4L2CompatManager::V4L2CompatManager()
> > + : cm_(nullptr)
> > +{
> > + openat_func_ = (openat_func_t )dlsym(RTLD_NEXT, "openat");
> > + dup_func_ = (dup_func_t )dlsym(RTLD_NEXT, "dup");
> > + close_func_ = (close_func_t )dlsym(RTLD_NEXT, "close");
> > + ioctl_func_ = (ioctl_func_t )dlsym(RTLD_NEXT, "ioctl");
> > + mmap_func_ = (mmap_func_t )dlsym(RTLD_NEXT, "mmap");
> > + munmap_func_ = (munmap_func_t )dlsym(RTLD_NEXT, "munmap");
You can remove one space before the ) in each of those lines.
> > +}
> > +
> > +V4L2CompatManager::~V4L2CompatManager()
> > +{
> > + devices_.clear();
> > +
> > + if (isRunning()) {
> > + exit(0);
> > + /* \todo Wait with a timeout, just in case. */
> > + wait();
> > + }
> > +}
> > +
> > +int V4L2CompatManager::init()
> > +{
> > + start();
> > +
> > + MutexLocker locker(mutex_);
> > + cv_.wait(locker);
There's a risk of a race condition here, where the new thread could
notify the condition variable before you start waiting on it. The wait
would then never complete. You need to add an initialized_ member to the
class, initialize it to false in the constructor, set it to true before
calling cv_.notify_one() in run(), and call
cv_.wait(locker, []{ return initialized_; });
here.
> > +
> > + return 0;
> > +}
> > +
> > +void V4L2CompatManager::run()
> > +{
> > + cm_ = new CameraManager();
> > +
> > + int ret = cm_->start();
> > + if (ret) {
> > + LOG(V4L2Compat, Error) << "Failed to start camera manager: "
> > + << strerror(-ret);
> > + return;
> > + }
> > +
> > + LOG(V4L2Compat, Debug) << "Started camera manager";
> > +
> > + /*
> > + * For each Camera registered in the system, a V4L2CameraProxy
> > + * gets created here to wraps a camera device.
s/wraps/wrap/
and you can go up to 80 columns before wrapping.
> > + */
> > + unsigned int index = 0;
> > + for (auto &camera : cm_->cameras()) {
> > + V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);
> > + proxies_.emplace_back(proxy);
> > + ++index;
> > + }
> > +
> > + /*
> > + * libcamera has been initialized. Unlock the init() caller
> > + * as we're now ready to handle calls from the framework.
You can go up to 80 columns before wrapping here too.
> > + */
mutex_.lock();
initialized_ = true;
mutex_.unlock();
> > + cv_.notify_one();
> > +
> > + /* Now start processing events and messages. */
> > + exec();
> > +
> > + cm_->stop();
> > + delete cm_;
> > + cm_ = nullptr;
> > +}
> > +
> > +V4L2CompatManager *V4L2CompatManager::instance()
> > +{
> > + static V4L2CompatManager v4l2CompatManager;
> > + return &v4l2CompatManager;
s/v4l2CompatManager/instance/ ?
> > +}
> > +
> > +V4L2CameraProxy *V4L2CompatManager::getCamera(int fd)
> > +{
> > + auto device = devices_.find(fd);
> > + if (device == devices_.end())
> > + return nullptr;
> > +
> > + return device->second.get();
> > +}
> > +
> > +V4L2CameraProxy *V4L2CompatManager::getCamera(void *addr)
> > +{
> > + auto map = mmaps_.find(addr);
> > + if (map == mmaps_.end())
> > + return nullptr;
> > +
> > + return devices_.at(map->second).get();
> > +}
> > +
> > +int V4L2CompatManager::getCameraIndex(int fd)
> > +{
> > + struct stat statbuf;
> > + fstat(fd, &statbuf);
> > + unsigned int dev_major = major(statbuf.st_rdev);
> > + unsigned int dev_minor = minor(statbuf.st_rdev);
devMajor and devMinor.
> > +
> > + std::string name;
> > + std::ifstream nameFile;
> > + nameFile.open("/sys/dev/char/" + std::to_string(dev_major) + ":" +
> > + std::to_string(dev_minor) + "/name");
> > + if (!nameFile)
> > + return -1;
> > +
> > + std::getline(nameFile, name);
> > + nameFile.close();
> > +
> > + unsigned int index = 0;
> > + for (auto &camera : cm_->cameras()) {
> > + if (!camera->name().compare(name))
> > + break;
> > + ++index;
> > + }
As commented on v1, matching on the video node name isn't a good idea,
as we want to support all cameras, and not all cameras are named based
on the video node name. I wrote
"One option would be to add a method to the camera manager to retrieve a
camera by device node path. This wouldn't expose any V4L2-specific
information towards applications, the map of device nodes to cameras
would be private."
but I don't think that's right, we should instead retrieve a camera by
major:minor, as multiple device nodes in the system could
> > +
> > + if (index >= cm_->cameras().size())
> > + return -1;
> > +
> > + return index;
> > +}
> > +
> > +int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mode)
> > +{
> > + int fd = openat_func_(dirfd, path, oflag, mode);
> > + if (fd < 0)
> > + return fd;
> > +
> > + if (std::string(path).find("video") == std::string::npos)
> > + return fd;
So opening "/home/user/myvideo.mp4" will match :-)
You can't match by name here, you need to fstat() and check that the
file is a character device node whose major is equal to the V4L2 major.
Or, if you add a method to the camera manager to retrieve a camera by
major:minor, I think you could even skip this check completely as it
would be handled in CameraManager (but the fstat is needed nonetheless).
> > +
> > + if (!isRunning())
> > + init();
> > +
> > + int ret = getCameraIndex(fd);
> > + if (ret < 0) {
> > + LOG(V4L2Compat, Error) << "No camera found for " << path;
I'd turn that into an Info or Debug message as it's not necessarily an
error, not all cameras are supported by libcamera.
> > + return fd;
> > + }
You need to close_func_(fd) here.
> > +
> > + unsigned int camera_index = static_cast<unsigned int>(ret);
> > +
> > + std::shared_ptr<V4L2CameraProxy> proxy = proxies_[camera_index];
>
> You here share the ownership of the managed object owned by
> proxies_[camera_index] with a local variable, increasing its
> reference count
>
> > + ret = proxy->open(mode & O_NONBLOCK);
> > + if (ret < 0)
> > + return ret;
> > +
> > + int efd = eventfd(0, (mode & O_CLOEXEC) | (mode & O_NONBLOCK));
> > + if (efd < 0)
> > + return efd;
>
> I'm still not convinced about this, I'll go read your reply to v1
> again.
What's wrong with it ? We need an fd that applications can use in a
select() or poll() call, so we can't return the fd for the video node,
we need an eventfd that we can signal when a request completes.
> > +
> > + devices_.emplace(efd, proxy);
>
> And here you store it permanently in a map, making its reference count
> permanently = 2 (it goes up to 3, then the local variable goes out of
> scope and gest back to 2).
I don't think this is a big issue, and reference counting is needed to
support hot-unplug. As we don't support it yet we could turn proxies_
into a std::vector<std::unique_ptr<V4L2CameraProxy>> and devices_ into a
std::map<int, V4L2CameraProxy *>>, knowing that the device won't be
deleted until the V4L2CompatManager itself is deleted. The above lookup
in proxies_ would then become
V4L2CameraProxy *proxy = proxies_[camera_index].get();
I think the mmaps_ could then map from void * to V4L2CameraProxy *,
which would avoid double look-ups in munmap().
> > +
> > + return efd;
> > +}
> > +
> > +int V4L2CompatManager::dup(int oldfd)
> > +{
> > + int newfd = dup_func_(oldfd);
> > + if (getCamera(oldfd))
> > + devices_[newfd] = devices_[oldfd];
>
> Then here you create a new copy, with reference count = 3
>
> At delete time you clear all the entries in devices_, causing the
> shared pointer stored in proxies_ to get back to 1, then proxies_ goes
> out of scope, and the managed object gets deleted.
>
> Why all this dance ? Is the ownership of the CameraProxy shared
> between different componentes? Is it created or destoryed outside of
> this class ? I would save all this complexity and store
> V4L2CameraProxy instances in a vector, and point to them in the map at
> openat() time.
>
> The only thing you care about is to increase the reference count to
> the Camera, but you have a shared_ptr<> class instance in V4L2Camera,
> so that's fine.
To support hot-unplug we'll likely need shared pointers, for now we can
do without them and use unique_ptr as explained above.
> > +
> > + return newfd;
> > +}
> > +
> > +int V4L2CompatManager::close(int fd)
> > +{
> > + V4L2CameraProxy *proxy = getCamera(fd);
> > + if (proxy)
> > + return proxy->close();
This will break if the fd has been dup'ed. Closing one of the two fds
will close the proxy, the second fd will then be unusable. You need to
refcount opens in V4L2CameraProxy.
You also need to remove fd from the devices_ map here.
> > +
> > + int ret = close_func_(fd);
> > + return ret;
return close_func_(fd);
> > +}
> > +
> > +void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
> > + int fd, off_t offset)
>
> Align to open ( please
it is aligned :-)
> Mostly stylistic comments, so I think we're almost there, at least
> from my side!
>
> > +{
> > + V4L2CameraProxy *proxy = getCamera(fd);
> > + if (!proxy)
> > + return mmap_func_(addr, length, prot, flags, fd, offset);
> > +
> > + void *map = proxy->mmap(addr, length, prot, flags, offset);
> > + if (map == MAP_FAILED)
> > + return map;
> > +
> > + mmaps_[addr] = fd;
addr is a hint, there's no guarantee that the buffer will be mapped at
addr, and it is often set to NULL by the caller. The correct index for
mmaps_ is map, not addr.
> > + return map;
> > +}
> > +
> > +int V4L2CompatManager::munmap(void *addr, size_t length)
> > +{
> > + V4L2CameraProxy *proxy = getCamera(addr);
> > + if (!proxy)
> > + return munmap_func_(addr, length);
As getCamera(void *) is only called in this method, I would inline it:
auto map = mmaps_.find(addr);
if (map == mmaps_.end())
return munmap_func_(addr, length);
V4L2CameraProxy *proxy = devices_.at(map->second).get();
> > +
> > + int ret = proxy->munmap(addr, length);
> > + if (ret < 0)
> > + return ret;
> > +
> > + mmaps_.erase(addr);
And here you could then call
mmaps_.erase(map);
> > + addr = nullptr;
This isn't needed.
> > +
> > + return 0;
> > +}
> > +
> > +int V4L2CompatManager::ioctl(int fd, unsigned long request, void *arg)
> > +{
> > + V4L2CameraProxy *proxy = getCamera(fd);
> > + if (!proxy)
> > + return ioctl_func_(fd, request, arg);
> > +
> > + return proxy->ioctl(request, arg);
> > +}
> > +
> > +/* \todo make libcamera export these */
> > +int V4L2CompatManager::bplMultiplier(unsigned int format)
> > +{
> > + switch (format) {
> > + case V4L2_PIX_FMT_NV12:
> > + case V4L2_PIX_FMT_NV21:
> > + case V4L2_PIX_FMT_NV16:
> > + case V4L2_PIX_FMT_NV61:
> > + case V4L2_PIX_FMT_NV24:
> > + case V4L2_PIX_FMT_NV42:
> > + return 1;
> > + case V4L2_PIX_FMT_BGR24:
> > + case V4L2_PIX_FMT_RGB24:
> > + return 3;
> > + case V4L2_PIX_FMT_ARGB32:
> > + return 4;
> > + case V4L2_PIX_FMT_VYUY:
> > + case V4L2_PIX_FMT_YVYU:
> > + case V4L2_PIX_FMT_UYVY:
> > + case V4L2_PIX_FMT_YUYV:
> > + return 2;
> > + default:
> > + return 0;
> > + };
> > +}
> > +
> > +int V4L2CompatManager::imageSize(unsigned int format,
> > + unsigned int width, unsigned int height)
> > +{
> > + switch (format) {
> > + case V4L2_PIX_FMT_NV12:
> > + case V4L2_PIX_FMT_NV21:
> > + return width * height + width * height / 2;
> > + case V4L2_PIX_FMT_NV16:
> > + case V4L2_PIX_FMT_NV61:
> > + return width * height * 2;
> > + case V4L2_PIX_FMT_NV24:
> > + case V4L2_PIX_FMT_NV42:
> > + return width * height * 3;
> > + case V4L2_PIX_FMT_BGR24:
> > + case V4L2_PIX_FMT_RGB24:
> > + return width * height * 3;
> > + case V4L2_PIX_FMT_ARGB32:
> > + return width * height * 4;
> > + case V4L2_PIX_FMT_VYUY:
> > + case V4L2_PIX_FMT_YVYU:
> > + case V4L2_PIX_FMT_UYVY:
> > + case V4L2_PIX_FMT_YUYV:
> > + return width * height * 2;
> > + default:
> > + return 0;
> > + };
> > +}
> > +
> > +unsigned int V4L2CompatManager::v4l2ToDrm(unsigned int pixelformat)
> > +{
> > + switch (pixelformat) {
> > + /* RGB formats. */
> > + case V4L2_PIX_FMT_RGB24:
> > + return DRM_FORMAT_BGR888;
> > + case V4L2_PIX_FMT_BGR24:
> > + return DRM_FORMAT_RGB888;
> > + case V4L2_PIX_FMT_ARGB32:
> > + return DRM_FORMAT_BGRA8888;
> > +
> > + /* YUV packed formats. */
> > + case V4L2_PIX_FMT_YUYV:
> > + return DRM_FORMAT_YUYV;
> > + case V4L2_PIX_FMT_YVYU:
> > + return DRM_FORMAT_YVYU;
> > + case V4L2_PIX_FMT_UYVY:
> > + return DRM_FORMAT_UYVY;
> > + case V4L2_PIX_FMT_VYUY:
> > + return DRM_FORMAT_VYUY;
> > +
> > + /* YUY planar formats. */
> > + case V4L2_PIX_FMT_NV16:
> > + return DRM_FORMAT_NV16;
> > + case V4L2_PIX_FMT_NV61:
> > + return DRM_FORMAT_NV61;
> > + case V4L2_PIX_FMT_NV12:
> > + return DRM_FORMAT_NV12;
> > + case V4L2_PIX_FMT_NV21:
> > + return DRM_FORMAT_NV21;
> > + case V4L2_PIX_FMT_NV24:
> > + return DRM_FORMAT_NV24;
> > + case V4L2_PIX_FMT_NV42:
> > + return DRM_FORMAT_NV42;
> > + default:
> > + return pixelformat;
> > + };
> > +}
> > +
> > +unsigned int V4L2CompatManager::drmToV4L2(unsigned int pixelformat)
> > +{
> > + switch (pixelformat) {
> > + /* RGB formats. */
> > + case DRM_FORMAT_BGR888:
> > + return V4L2_PIX_FMT_RGB24;
> > + case DRM_FORMAT_RGB888:
> > + return V4L2_PIX_FMT_BGR24;
> > + case DRM_FORMAT_BGRA8888:
> > + return V4L2_PIX_FMT_ARGB32;
> > +
> > + /* YUV packed formats. */
> > + case DRM_FORMAT_YUYV:
> > + return V4L2_PIX_FMT_YUYV;
> > + case DRM_FORMAT_YVYU:
> > + return V4L2_PIX_FMT_YVYU;
> > + case DRM_FORMAT_UYVY:
> > + return V4L2_PIX_FMT_UYVY;
> > + case DRM_FORMAT_VYUY:
> > + return V4L2_PIX_FMT_VYUY;
> > +
> > + /* YUY planar formats. */
> > + case DRM_FORMAT_NV16:
> > + return V4L2_PIX_FMT_NV16;
> > + case DRM_FORMAT_NV61:
> > + return V4L2_PIX_FMT_NV61;
> > + case DRM_FORMAT_NV12:
> > + return V4L2_PIX_FMT_NV12;
> > + case DRM_FORMAT_NV21:
> > + return V4L2_PIX_FMT_NV21;
> > + case DRM_FORMAT_NV24:
> > + return V4L2_PIX_FMT_NV24;
> > + case DRM_FORMAT_NV42:
> > + return V4L2_PIX_FMT_NV42;
> > + default:
> > + return pixelformat;
> > + }
> > +}
> > diff --git a/src/v4l2/v4l2_compat_manager.h b/src/v4l2/v4l2_compat_manager.h
> > new file mode 100644
> > index 00000000..d5ae7810
> > --- /dev/null
> > +++ b/src/v4l2/v4l2_compat_manager.h
> > @@ -0,0 +1,86 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * v4l2_compat_manager.h - V4L2 compatibility manager
> > + */
> > +#ifndef __V4L2_COMPAT_MANAGER_H__
> > +#define __V4L2_COMPAT_MANAGER_H__
> > +
> > +#include <condition_variable>
> > +#include <linux/videodev2.h>
videodev2.h isn't needed as far as I can tell.
> > +#include <map>
> > +#include <mutex>
> > +#include <queue>
I don't think you use std::queue here.
> > +#include <sys/syscall.h>
Is this needed here ?
> > +#include <unistd.h>
I don't think unistd.h is needed either, but you need fcntl.h for
mode_t, and sys/types.h for off_t and size_t.
> > +#include <vector>
> > +
> > +#include <libcamera/camera.h>
> > +#include <libcamera/camera_manager.h>
> > +#include <libcamera/stream.h>
> > +
> > +#include "thread.h"
> > +#include "v4l2_camera_proxy.h"
> > +
> > +using namespace libcamera;
> > +
> > +class V4L2CompatManager : public Thread
> > +{
> > +public:
> > + static V4L2CompatManager *instance();
> > +
> > + int init();
> > +
> > + int openat(int dirfd, const char *path, int oflag, mode_t mode);
> > +
> > + V4L2CameraProxy *getCamera(int fd);
> > + V4L2CameraProxy *getCamera(void *addr);
I would move those two methods either before openat() or after ioctl()
in order to keep all the implementations of the C functions you
intercept grouped together.
> > +
> > + int dup(int oldfd);
> > + int close(int fd);
> > + void *mmap(void *addr, size_t length, int prot, int flags,
> > + int fd, off_t offset);
> > + int munmap(void *addr, size_t length);
> > + int ioctl(int fd, unsigned long request, void *arg);
> > +
> > + static int bplMultiplier(unsigned int format);
> > + static int imageSize(unsigned int format, unsigned int width,
> > + unsigned int height);
> > +
> > + static unsigned int v4l2ToDrm(unsigned int pixelformat);
> > + static unsigned int drmToV4L2(unsigned int pixelformat);
> > +
> > +private:
> > + V4L2CompatManager();
> > + ~V4L2CompatManager();
> > +
> > + void run() override;
> > + int getCameraIndex(int fd);
> > +
> > + typedef int (*openat_func_t)(int dirfd, const char *path, int oflag, ...);
> > + typedef int (*dup_func_t)(int oldfd);
> > + typedef int (*close_func_t)(int fd);
> > + typedef int (*ioctl_func_t)(int fd, unsigned long request, ...);
> > + typedef void *(*mmap_func_t)(void *addr, size_t length, int prot,
> > + int flags, int fd, off_t offset);
> > + typedef int (*munmap_func_t)(void *addr, size_t length);
> > +
> > + openat_func_t openat_func_;
> > + dup_func_t dup_func_;
> > + close_func_t close_func_;
> > + ioctl_func_t ioctl_func_;
> > + mmap_func_t mmap_func_;
> > + munmap_func_t munmap_func_;
> > +
> > + CameraManager *cm_;
> > +
> > + std::mutex mutex_;
> > + std::condition_variable cv_;
> > +
> > + std::vector<std::shared_ptr<V4L2CameraProxy>> proxies_;
> > + std::map<int, std::shared_ptr<V4L2CameraProxy>> devices_;
> > + std::map<void *, int> mmaps_;
> > +};
> > +
> > +#endif /* __V4L2_COMPAT_MANAGER_H__ */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list