[libcamera-devel] [PATCH 2/2] v4l2: v4l2_compat: Add V4L2 compatibility layer
Jacopo Mondi
jacopo at jmondi.org
Fri Dec 6 17:12:03 CET 2019
Hi Paul,
On Fri, Dec 06, 2019 at 04:26:54AM -0500, Paul Elder wrote:
> Add libcamera V4L2 compatibility layer.
Maybe just me, but having this patch split in components would have
helped not having to digest 1445 lines of patch in one go
I know Niklas agrees :)
>
> 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
> first enumerated libcamera camera.
That's not trivial indeed...
For simple devices, we could easily collect the video nodes a camera
uses and match them with the one the v4l2 application tries to use.
For complex devices, where the DMA output node could be multiplexed by
different cameras depending on the ISP configuration, this is not
easy.
I presume v4l2 application are mostly meant to be used with simple
devices, so adding a list of V4L2Videodevices to the Camera (while a
bit of an hack) is totally doable. What do others think ?
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> meson_options.txt | 5 +
> src/meson.build | 4 +
> src/v4l2/meson.build | 30 +++
> src/v4l2/v4l2_camera.cpp | 312 ++++++++++++++++++++++
> src/v4l2/v4l2_camera.h | 68 +++++
> src/v4l2/v4l2_camera_proxy.cpp | 438 +++++++++++++++++++++++++++++++
> src/v4l2/v4l2_camera_proxy.h | 63 +++++
> src/v4l2/v4l2_compat.cpp | 81 ++++++
> src/v4l2/v4l2_compat_manager.cpp | 353 +++++++++++++++++++++++++
> src/v4l2/v4l2_compat_manager.h | 91 +++++++
> 10 files changed, 1445 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')
> 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..d96db4ff
> --- /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',
> + '-D_FILE_OFFSET_BITS=32',
Seems it is enough to undefine the _FILE_OFFSET_BITS macro to use the
32-bit interface ?
https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html
If _FILE_OFFSET_BITS is undefined, or if it is defined to the value
32, nothing changes. The 32 bit interface is used and types like off_t
have a size of 32 bits on 32 bit systems.
> +]
> +
> +v4l2_compat = shared_library('v4l2-compat',
> + v4l2_compat_sources,
> + name_prefix : '',
> + install : true,
> + link_with : libcamera,
> + include_directories : v4l2_compat_includes,
> + dependencies : libcamera_deps,
> + cpp_args : v4l2_compat_cpp_args)
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> new file mode 100644
> index 00000000..c6c84ef3
> --- /dev/null
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -0,0 +1,312 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * v4l2_camera.cpp - V4L2 compatibility camera
> + */
> +#include "v4l2_camera.h"
> +
> +#include <errno.h>
> +#include <linux/videodev2.h>
> +#include <sys/mman.h>
> +#include <sys/syscall.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +#include "log.h"
> +#include "v4l2_compat_manager.h"
> +
> +using namespace libcamera;
> +
> +LOG_DECLARE_CATEGORY(V4L2Compat);
> +
> +V4L2Camera::V4L2Camera(std::shared_ptr<Camera> camera)
> + : camera_(camera), bufferCount_(0), isRunning_(false), sequence_(0),
> + buffer_sema_(new Semaphore())
Can't this be a class member instead of a pointer ?
Also, I would name it differently, or at least make it cameraCase_
> +{
> + camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);
> +};
> +
> +V4L2Camera::~V4L2Camera()
> +{
> + while (!pendingRequests_.empty()) {
> + delete pendingRequests_.front();
> + pendingRequests_.pop();
> + }
> +
> + buffer_lock_.lock();
> + while (!completedBuffers_.empty()) {
> + delete completedBuffers_.front();
> + completedBuffers_.pop();
> + }
> + buffer_lock_.unlock();
> +
> + delete buffer_sema_;
you would save deleting it
> +
> + camera_->release();
> +}
> +
> +void V4L2Camera::open(int *ret, bool nonblock)
> +{
> + nonblock_ = nonblock;
> +
> + if (camera_->acquire() < 0) {
> + LOG(V4L2Compat, Error) << "Failed to acquire camera";
> + *ret = -EINVAL;
> + return;
> + }
> +
> + config_ = camera_->generateConfiguration({ StreamRole::Viewfinder });
> + if (config_ == nullptr) {
> + *ret = -EINVAL;
> + return;
> + }
> +
> + *ret = 0;
> +}
> +
> +void V4L2Camera::close(int *ret)
> +{
> + *ret = camera_->release();
> +}
> +
> +void V4L2Camera::getStreamConfig(StreamConfiguration *ret)
Maybe *streaConfig ?
> +{
> + *ret = config_->at(0);
> +}
> +
> +void V4L2Camera::requestComplete(Request *request)
> +{
> + if (request->status() == Request::RequestCancelled) {
> + LOG(V4L2Compat, Error) << "Request not succesfully completed: "
> + << request->status();
> + return;
> + }
> +
> + /* We only have one stream at the moment. */
> + buffer_lock_.lock();
> + Buffer *buffer = request->buffers().begin()->second;
> + completedBuffers_.push(buffer);
> + buffer_lock_.unlock();
> +
> + buffer_sema_->release();
> +}
> +
> +void V4L2Camera::configure(int *ret, struct v4l2_format *arg,
> + unsigned int bufferCount)
> +{
> + config_->at(0).size.width = arg->fmt.pix.width;
Nit: can you use a reference to config_->at(0) ?
> + config_->at(0).size.height = arg->fmt.pix.height;
> + config_->at(0).pixelFormat =
> + V4L2CompatManager::v4l2_to_drm(arg->fmt.pix.pixelformat);
> + bufferCount_ = bufferCount;
> + config_->at(0).bufferCount = bufferCount_;
> + /* \todo memoryType (interval vs external) */
> +
> + CameraConfiguration::Status validation = config_->validate();
> + if (validation == CameraConfiguration::Invalid) {
> + LOG(V4L2Compat, Info) << "Configuration invalid";
that's probably an error, isn't it ?
> + *ret = -EINVAL;
> + return;
> + }
> + if (validation == CameraConfiguration::Adjusted)
> + LOG(V4L2Compat, Info) << "Configuration adjusted";
> +
> + LOG(V4L2Compat, Info) << "Validated configuration is: "
> + << config_->at(0).toString();
> +
> + *ret = camera_->configure(config_.get());
> + if (*ret < 0)
> + return;
> +
> + bufferCount_ = config_->at(0).bufferCount;
> +
> + *ret = 0;
> +}
> +
> +void V4L2Camera::mmap(void **ret, void *addr, size_t length, int prot,
> + int flags, off_t offset)
Do we need to check flags ?
> +{
> + LOG(V4L2Compat, Debug) << "Servicing MMAP";
> +
> + if (prot != (PROT_READ | PROT_WRITE)) {
> + errno = EINVAL;
This function (actually all V4L2Camera functions) is called through a method
invocation and has a long call stack before getting back to the
caller. I wonder if errno does not get overwritten along that route.
Also, from man mmap:
ENOTSUP
MAP_FIXED or MAP_PRIVATE was specified in the flags argument and the
implementation does not support this functionality.
The implementation does not support the combination of accesses
requested in the prot argument.
> + *ret = MAP_FAILED;
> + return;
> + }
> +
> + StreamConfiguration &streamConfig = config_->at(0);
> + unsigned int sizeimage =
> + V4L2CompatManager::image_size(
> + V4L2CompatManager::drm_to_v4l2(streamConfig.pixelFormat),
> + streamConfig.size.width,
> + streamConfig.size.height);
> + if (sizeimage == 0) {
> + errno = EINVAL;
> + *ret = MAP_FAILED;
> + return;
> + }
> +
> + unsigned int index = offset / sizeimage;
> + if (index * sizeimage != offset || length != sizeimage) {
> + errno = EINVAL;
Is EINVAL the right error here?
from man mmap:
ENOMEM
MAP_FIXED was specified, and the range [addr,addr+len)
exceeds that allowed for the address space of a process; or, if
MAP_FIXED was not specified and there is insufficient room in the
address space to effect the map‐ ping
EOVERFLOW
The file is a regular file and the value of off plus len exceeds the
offset maximum established in the open file description associated
with fildes.
?
However I'm not sure we should care about the mmap errors at this
level, as those should be returned when the actual mapping is
performed. What do you think ?
> + *ret = MAP_FAILED;
> + return;
> + }
> +
> + Stream *stream = *camera_->streams().begin();
> + *ret = stream->buffers()[index].planes()[0].mem();
> +}
> +
> +void V4L2Camera::munmap(int *ret, void *addr, size_t length)
> +{
> + StreamConfiguration &streamConfig = config_->at(0);
> + unsigned int sizeimage =
> + V4L2CompatManager::image_size(streamConfig.pixelFormat,
> + streamConfig.size.width,
> + streamConfig.size.height);
> +
> + if (length != sizeimage) {
> + errno = EINVAL;
Here EINVAL seems to be appropriate
> + *ret = -1;
> + return;
> + }
> +
> + *ret = 0;
> +}
> +
> +void V4L2Camera::validStreamType(bool *ret, uint32_t type)
> +{
> + *ret = (type == V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +}
> +
> +void V4L2Camera::validMemoryType(bool *ret, uint32_t memory)
> +{
> + *ret = (memory == V4L2_MEMORY_MMAP);
> +}
> +
> +void V4L2Camera::allocBuffers(int *ret, unsigned int count)
> +{
> + LOG(V4L2Compat, Debug) << "Allocating libcamera bufs";
> + *ret = camera_->allocateBuffers();
I fear the buffer rework would required a big rebase of this series :(
> + if (*ret == -EACCES)
> + *ret = -EBUSY;
> +}
> +
> +/* \todo implement allocDMABuffers */
> +void V4L2Camera::allocDMABuffers(int *ret, unsigned int count)
> +{
> + *ret = -ENOTTY;
> +}
> +
> +void V4L2Camera::freeBuffers()
> +{
> + camera_->freeBuffers();
> + bufferCount_ = 0;
> +}
> +
> +void V4L2Camera::streamOn(int *ret)
> +{
> + if (isRunning_) {
> + *ret = 0;
> + return;
> + }
> +
> + sequence_ = 0;
> +
> + *ret = camera_->start();
> + if (*ret < 0)
> + return;
errno ?
> + isRunning_ = true;
> +
> + while (!pendingRequests_.empty()) {
> + *ret = camera_->queueRequest(pendingRequests_.front());
> + pendingRequests_.pop();
> + if (*ret < 0)
> + return;
> + }
> +
> + *ret = 0;
> +}
> +
> +void V4L2Camera::streamOff(int *ret)
> +{
> + /* \todo restore buffers to reqbufs state? */
> + if (!isRunning_) {
> + *ret = 0;
> + return;
> + }
> +
> + *ret = camera_->stop();
> + if (*ret < 0)
> + return;
> + isRunning_ = false;
> +
> + *ret = 0;
Here and in streamOn you could set *ret = 0 at the beginning of the
function.
> +}
> +
> +void V4L2Camera::qbuf(int *ret, struct v4l2_buffer *arg)
> +{
> + Stream *stream = config_->at(0).stream();
> + std::unique_ptr<Buffer> buffer = stream->createBuffer(arg->index);
> + if (!buffer) {
> + LOG(V4L2Compat, Error) << "Can't create buffer";
> + *ret = -ENOMEM;
> + return;
> + }
> +
> + Request *request = camera_->createRequest();
paranoid: createRequest() can return nullptr
> + *ret = request->addBuffer(std::move(buffer));
> + if (*ret < 0) {
> + LOG(V4L2Compat, Error) << "Can't set buffer for request";
> + return;
> + }
> +
> + if (!isRunning_) {
> + pendingRequests_.push(request);
> + } else {
> + *ret = camera_->queueRequest(request);
> + if (*ret < 0) {
> + LOG(V4L2Compat, Error) << "Can't queue request";
> + if (*ret == -EACCES)
> + *ret = -EBUSY;
> + return;
> + }
> + }
> +
> + arg->flags |= V4L2_BUF_FLAG_QUEUED;
> + arg->flags |= V4L2_BUF_FLAG_MAPPED;
> + arg->flags &= ~V4L2_BUF_FLAG_DONE;
> + *ret = 0;
> +}
> +
> +int V4L2Camera::dqbuf(struct v4l2_buffer *arg)
> +{
> + if (nonblock_ && !buffer_sema_->tryAcquire())
> + return -EAGAIN;
> + else
> + buffer_sema_->acquire();
> +
> + buffer_lock_.lock();
> + Buffer *buffer = completedBuffers_.front();
> + completedBuffers_.pop();
> + buffer_lock_.unlock();
> +
> + arg->bytesused = buffer->bytesused();
> + arg->field = V4L2_FIELD_NONE;
> + arg->timestamp.tv_sec = buffer->timestamp() / 1000000000;
> + arg->timestamp.tv_usec = buffer->timestamp() / 1000;
> + arg->sequence = sequence_++;
Don't we have sequence in Buffer ?
> +
> + arg->flags &= ~V4L2_BUF_FLAG_QUEUED;
> + arg->flags &= ~V4L2_BUF_FLAG_DONE;
> +
> + StreamConfiguration &streamConfig = config_->at(0);
> + unsigned int sizeimage =
> + V4L2CompatManager::image_size(streamConfig.pixelFormat,
> + streamConfig.size.width,
> + streamConfig.size.height);
> + arg->length = sizeimage;
You can save the sizeimage variable.
I wonder if this needs to be re-calculated everytime... nothing big
however...
> +
> + return 0;
> +}
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> new file mode 100644
> index 00000000..3d3cd8ff
> --- /dev/null
> +++ b/src/v4l2/v4l2_camera.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * v4l2_camera.h - V4L2 compatibility camera
> + */
> +#ifndef __V4L2_CAMERA_H__
> +#define __V4L2_CAMERA_H__
> +
> +#include <linux/videodev2.h>
> +#include <mutex>
> +#include <queue>
> +
> +#include <libcamera/camera.h>
> +#include "semaphore.h"
> +
> +using namespace libcamera;
> +
> +class V4L2Camera : public Object
> +{
> +public:
> + V4L2Camera(std::shared_ptr<Camera> camera);
> + ~V4L2Camera();
> +
> + void open(int *ret, bool nonblock);
> + void close(int *ret);
> + void getStreamConfig(StreamConfiguration *ret);
> + void requestComplete(Request *request);
> +
> + void mmap(void **ret, void *addr, size_t length, int prot, int flags,
> + off_t offset);
> + void munmap(int *ret, void *addr, size_t length);
> +
> + void configure(int *ret, struct v4l2_format *arg,
> + unsigned int bufferCount);
> +
> + void validStreamType(bool *ret, uint32_t type);
> + void validMemoryType(bool *ret, uint32_t memory);
> +
> + void allocBuffers(int *ret, unsigned int count);
> + void allocDMABuffers(int *ret, unsigned int count);
> + void freeBuffers();
> + void streamOn(int *ret);
> + void streamOff(int *ret);
> +
> + void qbuf(int *ret, struct v4l2_buffer *arg);
> + int dqbuf(struct v4l2_buffer *arg);
> +
> +private:
> + void initDefaultV4L2Format();
> +
> + std::shared_ptr<Camera> camera_;
> + std::unique_ptr<CameraConfiguration> config_;
> +
> + unsigned int bufferCount_;
> + bool isRunning_;
> + bool nonblock_;
> +
> + unsigned int sequence_;
> +
> + Semaphore *buffer_sema_;
> + std::mutex buffer_lock_;
> +
> + std::queue<Request *> pendingRequests_;
> + std::queue<Buffer *> completedBuffers_;
> +};
> +
> +#endif /* __V4L2_CAMERA_H__ */
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> new file mode 100644
> index 00000000..1dd2c363
> --- /dev/null
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -0,0 +1,438 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * v4l2_camera_proxy.cpp - Proxy to V4L2 compatibility camera
> + */
> +#include "v4l2_camera_proxy.h"
> +
> +#include <algorithm>
> +#include <linux/videodev2.h>
> +#include <string.h>
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/object.h>
> +
> +#include "log.h"
> +#include "utils.h"
> +#include "v4l2_camera.h"
> +#include "v4l2_compat_manager.h"
> +
> +#define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))
> +
> +using namespace libcamera;
> +
> +LOG_DECLARE_CATEGORY(V4L2Compat);
> +
> +V4L2CameraProxy::V4L2CameraProxy(unsigned int index,
> + std::shared_ptr<Camera> camera)
> + : index_(index), bufferCount_(0), currentBuf_(0),
> + vcam_(utils::make_unique<V4L2Camera>(camera))
> +{
> + querycap(camera);
> +}
> +
> +V4L2CameraProxy::~V4L2CameraProxy()
> +{
> + vcam_.reset();
Not sure it is necessary to reset a unique_ptr<> at destruction time,
the managed pointer will be destroyed anyway.
> +}
> +
> +int V4L2CameraProxy::open(bool nonblock)
> +{
> + int ret;
> + vcam_->invokeMethod(&V4L2Camera::open, ConnectionTypeBlocking,
> + &ret, nonblock);
> + if (ret < 0)
> + return ret;
> +
> + vcam_->invokeMethod(&V4L2Camera::getStreamConfig,
> + ConnectionTypeBlocking, &streamConfig_);
> + setFmtFromConfig();
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::close()
> +{
> + int ret;
> + vcam_->invokeMethod(&V4L2Camera::close, ConnectionTypeBlocking, &ret);
> + return ret;
> +}
> +
> +void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
> + off_t offset)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing MMAP";
> +
> + void *val;
> + vcam_->invokeMethod(&V4L2Camera::mmap, ConnectionTypeBlocking,
> + &val, addr, length, prot, flags, offset);
> + return val;
> +}
> +
> +int V4L2CameraProxy::munmap(void *addr, size_t length)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing MUNMAP";
> +
> + int ret;
> + vcam_->invokeMethod(&V4L2Camera::munmap, ConnectionTypeBlocking,
> + &ret, addr, length);
> + return ret;
> +}
> +
> +bool V4L2CameraProxy::hasPixelFormat(unsigned int format)
> +{
> + const std::vector<PixelFormat> &formats =
> + streamConfig_.formats().pixelformats();
> + return std::find(formats.begin(), formats.end(), format) != formats.end();
> +}
> +
> +/* \todo getDeviceCaps? getMemoryCaps? */
> +
> +bool V4L2CameraProxy::hasSize(unsigned int format, Size size)
> +{
> + int len = streamConfig_.formats().sizes(format).size();
> + for (int i = 0; i < len; i++)
> + if (streamConfig_.formats().sizes(format)[i] == size)
> + return true;
Can't you find() on the vector<Size> returned by
streamConfig_.formats().sizes(format) ?
> +
> + return false;
> +}
> +
> +bool V4L2CameraProxy::validateStreamType(uint32_t type)
> +{
> + bool valid;
> + vcam_->invokeMethod(&V4L2Camera::validStreamType,
> + ConnectionTypeBlocking, &valid, type);
> + if (!valid)
> + return false;
> +
> + return true;
> +}
> +
> +bool V4L2CameraProxy::validateMemoryType(uint32_t memory)
> +{
> + bool valid;
> + vcam_->invokeMethod(&V4L2Camera::validMemoryType,
> + ConnectionTypeBlocking, &valid, memory);
> + if (!valid)
> + return false;
> +
> + return true;
In this two functions you can here just return 'valid'
> +}
> +
> +void V4L2CameraProxy::setFmtFromConfig()
> +{
> + curV4L2Format_.fmt.pix.width = streamConfig_.size.width;
> + curV4L2Format_.fmt.pix.height = streamConfig_.size.height;
> + curV4L2Format_.fmt.pix.pixelformat =
> + V4L2CompatManager::drm_to_v4l2(streamConfig_.pixelFormat);
> + curV4L2Format_.fmt.pix.field = V4L2_FIELD_NONE;
> + curV4L2Format_.fmt.pix.bytesperline =
> + V4L2CompatManager::bpl_multiplier(
> + curV4L2Format_.fmt.pix.pixelformat) *
> + curV4L2Format_.fmt.pix.width;
> + curV4L2Format_.fmt.pix.sizeimage =
> + V4L2CompatManager::image_size(curV4L2Format_.fmt.pix.pixelformat,
> + curV4L2Format_.fmt.pix.width,
> + curV4L2Format_.fmt.pix.height);
> + curV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> +}
> +
> +void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
> +{
> + std::string driver = "libcamera";
> + std::string bus_info = driver + ":" + std::to_string(index_);
> +
> + memcpy(capabilities_.driver, driver.c_str(),
> + sizeof(capabilities_.driver));
> + memcpy(capabilities_.card, camera->name().c_str(),
> + sizeof(capabilities_.card));
> + memcpy(capabilities_.bus_info, bus_info.c_str(),
> + sizeof(capabilities_.bus_info));
> + capabilities_.version = KERNEL_VERSION(5, 2, 0);
> + capabilities_.device_caps = V4L2_CAP_VIDEO_CAPTURE;
Are we single planar only ? I guess so, it's fine :)
> + capabilities_.capabilities =
> + capabilities_.device_caps | V4L2_CAP_DEVICE_CAPS;
> + memset(capabilities_.reserved, 0, sizeof(capabilities_.reserved));
> +}
> +
> +
> +int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_QUERYCAP";
> +
> + memcpy(arg, &capabilities_, sizeof(*arg));
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::vidioc_enum_fmt(struct v4l2_fmtdesc *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_ENUM_FMT";
> +
> + if (!validateStreamType(arg->type))
> + return -EINVAL;
> + if (arg->index > streamConfig_.formats().pixelformats().size())
> + return -EINVAL;
> +
> + memcpy(arg->description, "asdf", 5);
That's a real nice format name! :D
Do we need a map of formats to descriptions ?
> + arg->pixelformat =
> + V4L2CompatManager::drm_to_v4l2(
> + streamConfig_.formats().pixelformats()[arg->index]);
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::vidioc_g_fmt(struct v4l2_format *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_G_FMT";
> +
> + if (!validateStreamType(arg->type))
> + return -EINVAL;
> +
> + arg->fmt.pix.width = curV4L2Format_.fmt.pix.width;
> + arg->fmt.pix.height = curV4L2Format_.fmt.pix.height;
> + arg->fmt.pix.pixelformat = curV4L2Format_.fmt.pix.pixelformat;
> + arg->fmt.pix.field = curV4L2Format_.fmt.pix.field;
> + arg->fmt.pix.bytesperline = curV4L2Format_.fmt.pix.bytesperline;
> + arg->fmt.pix.sizeimage = curV4L2Format_.fmt.pix.sizeimage;
> + arg->fmt.pix.colorspace = curV4L2Format_.fmt.pix.colorspace;
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_S_FMT";
> +
> + int ret = vidioc_try_fmt(arg);
> + if (ret < 0)
> + return ret;
> +
> + vcam_->invokeMethod(&V4L2Camera::configure, ConnectionTypeBlocking,
> + &ret, arg, bufferCount_);
> + if (ret < 0)
> + return -EINVAL;
> +
> + vcam_->invokeMethod(&V4L2Camera::getStreamConfig,
> + ConnectionTypeBlocking, &streamConfig_);
> + setFmtFromConfig();
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::vidioc_try_fmt(struct v4l2_format *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_TRY_FMT";
> + if (!validateStreamType(arg->type))
> + return -EINVAL;
> +
> + unsigned int format = arg->fmt.pix.pixelformat;
> + if (!hasPixelFormat(format))
> + format = streamConfig_.formats().pixelformats()[0];
> +
> + Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> + if (!hasSize(format, size))
> + size = streamConfig_.formats().sizes(format)[0];
> +
> + arg->fmt.pix.width = size.width;
> + arg->fmt.pix.height = size.height;
> + arg->fmt.pix.pixelformat = format;
> + arg->fmt.pix.field = V4L2_FIELD_NONE;
> + arg->fmt.pix.bytesperline =
> + V4L2CompatManager::bpl_multiplier(
> + V4L2CompatManager::drm_to_v4l2(format)) *
> + arg->fmt.pix.width;
> + arg->fmt.pix.sizeimage =
> + V4L2CompatManager::image_size(
> + V4L2CompatManager::drm_to_v4l2(format),
> + arg->fmt.pix.width, arg->fmt.pix.height);
> + arg->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_REQBUFS";
> + if (!validateStreamType(arg->type))
> + return -EINVAL;
> + if (!validateMemoryType(arg->memory))
> + return -EINVAL;
> +
> + LOG(V4L2Compat, Debug) << arg->count << " bufs requested ";
> +
> + arg->capabilities = V4L2_BUF_CAP_SUPPORTS_MMAP;
> +
> + if (arg->count == 0) {
> + LOG(V4L2Compat, Debug) << "Freeing libcamera bufs";
> + int ret;
> + vcam_->invokeMethod(&V4L2Camera::streamOff,
> + ConnectionTypeBlocking, &ret);
> + vcam_->invokeMethod(&V4L2Camera::freeBuffers,
> + ConnectionTypeBlocking);
> + bufferCount_ = 0;
> + return 0;
> + }
> +
> + int ret;
as ret is used function-wise, I would its declaration to the beginning
> + vcam_->invokeMethod(&V4L2Camera::configure, ConnectionTypeBlocking,
> + &ret, &curV4L2Format_, arg->count);
> + if (ret < 0)
> + return -EINVAL;
> + arg->count = streamConfig_.bufferCount;
> + bufferCount_ = arg->count;
> +
> + ret = -EINVAL;
> + if (arg->memory == V4L2_MEMORY_MMAP)
> + vcam_->invokeMethod(&V4L2Camera::allocBuffers,
> + ConnectionTypeBlocking, &ret, arg->count);
> + else if (arg->memory == V4L2_MEMORY_DMABUF)
Do we even claim support for this ?
> + vcam_->invokeMethod(&V4L2Camera::allocDMABuffers,
> + ConnectionTypeBlocking, &ret, arg->count);
> +
> + if (ret < 0) {
> + arg->count = 0;
> + return ret == -EACCES ? -EBUSY : ret;
> + }
> +
> + LOG(V4L2Compat, Debug) << "Allocated " << arg->count << " buffers";
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::vidioc_querybuf(struct v4l2_buffer *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_QUERYBUF";
> + Stream *stream = streamConfig_.stream();
> +
> + if (!validateStreamType(arg->type))
> + return -EINVAL;
> + if (arg->index >= stream->buffers().size())
> + return -EINVAL;
> +
> + unsigned int index = arg->index;
> + memset(arg, 0, sizeof(*arg));
> + arg->index = index;
> + arg->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> + arg->length = curV4L2Format_.fmt.pix.sizeimage;
> + arg->memory = V4L2_MEMORY_MMAP;
> + arg->m.offset = arg->index * curV4L2Format_.fmt.pix.sizeimage;
> +
> + return 0;
> +}
> +
> +int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_QBUF, index = "
> + << arg->index;
> +
> + Stream *stream = streamConfig_.stream();
> +
> + if (!validateStreamType(arg->type))
> + return -EINVAL;
> + if (!validateMemoryType(arg->memory))
> + return -EINVAL;
> + if (arg->index >= stream->buffers().size())
> + return -EINVAL;
> +
> + int ret;
> + vcam_->invokeMethod(&V4L2Camera::qbuf, ConnectionTypeBlocking,
> + &ret, arg);
> + return ret;
> +}
> +
> +int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_DQBUF";
> +
> + if (!validateStreamType(arg->type))
> + return -EINVAL;
> + if (!validateMemoryType(arg->memory))
> + return -EINVAL;
> +
> + arg->index = currentBuf_;
> + currentBuf_ = (currentBuf_ + 1) % bufferCount_;
> +
> + return vcam_->dqbuf(arg);
Is dqbuf special ?
I know it could return immediately if nonblock_ is set, but
invokeMethod checks that invoked method is called, if it returns
immediately, that's fine. Or have I missed some other reason why this
is called directly.
> +}
> +
> +int V4L2CameraProxy::vidioc_streamon(int *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_STREAMON";
> +
> + if (!validateStreamType(*arg))
> + return -EINVAL;
> +
> + int ret;
> + vcam_->invokeMethod(&V4L2Camera::streamOn,
> + ConnectionTypeBlocking, &ret);
> + return ret;
> +}
> +
> +int V4L2CameraProxy::vidioc_streamoff(int *arg)
> +{
> + LOG(V4L2Compat, Debug) << "Servicing VIDIOC_STREAMOFF";
> +
> + if (!validateStreamType(*arg))
> + return -EINVAL;
> +
> + int ret;
> + vcam_->invokeMethod(&V4L2Camera::streamOff,
> + ConnectionTypeBlocking, &ret);
> + return ret;
> +}
> +
> +int V4L2CameraProxy::ioctl(unsigned long request, void *arg)
> +{
> + int ret;
> + switch (request) {
> + case VIDIOC_QUERYCAP:
> + ret = vidioc_querycap(static_cast<struct v4l2_capability *>(arg));
camelCase for method names as well ?
> + break;
> + case VIDIOC_ENUM_FMT:
> + ret = vidioc_enum_fmt(static_cast<struct v4l2_fmtdesc *>(arg));
> + break;
> + case VIDIOC_G_FMT:
> + ret = vidioc_g_fmt(static_cast<struct v4l2_format *>(arg));
> + break;
> + case VIDIOC_S_FMT:
> + ret = vidioc_s_fmt(static_cast<struct v4l2_format *>(arg));
> + break;
> + case VIDIOC_TRY_FMT:
> + ret = vidioc_try_fmt(static_cast<struct v4l2_format *>(arg));
> + break;
> + case VIDIOC_REQBUFS:
> + ret = vidioc_reqbufs(static_cast<struct v4l2_requestbuffers *>(arg));
> + break;
> + case VIDIOC_QUERYBUF:
> + ret = vidioc_querybuf(static_cast<struct v4l2_buffer *>(arg));
> + break;
> + case VIDIOC_QBUF:
> + ret = vidioc_qbuf(static_cast<struct v4l2_buffer *>(arg));
> + break;
> + case VIDIOC_DQBUF:
> + ret = vidioc_dqbuf(static_cast<struct v4l2_buffer *>(arg));
> + break;
> + case VIDIOC_STREAMON:
> + ret = vidioc_streamon(static_cast<int *>(arg));
> + break;
> + case VIDIOC_STREAMOFF:
> + ret = vidioc_streamoff(static_cast<int *>(arg));
> + break;
> + case VIDIOC_EXPBUF:
> + case VIDIOC_ENUM_FRAMESIZES:
> + default:
> + ret = -ENOTTY;
> + }
> +
> + if (ret < 0) {
> + errno = ret;
> + return -1;
> + }
> +
> + errno = 0;
> + return ret;
> +
> +}
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> new file mode 100644
> index 00000000..64c7aadd
> --- /dev/null
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * v4l2_camera_proxy.h - Proxy to V4L2 compatibility camera
> + */
> +#ifndef __V4L2_CAMERA_PROXY_H__
> +#define __V4L2_CAMERA_PROXY_H__
> +
> +#include <linux/videodev2.h>
> +
> +#include <libcamera/camera.h>
> +
> +#include "v4l2_camera.h"
> +
> +using namespace libcamera;
> +
> +class V4L2CameraProxy
> +{
> +public:
> + V4L2CameraProxy(unsigned int index, std::shared_ptr<Camera> camera);
> + ~V4L2CameraProxy();
> +
> + int open(bool nonblock);
> + int close();
> + void *mmap(void *addr, size_t length, int prot, int flags,
> + off_t offset);
> + int munmap(void *addr, size_t length);
> +
> + int ioctl(unsigned long request, void *arg);
> +
> +private:
> + bool hasPixelFormat(unsigned int format);
> + bool hasSize(unsigned int format, Size size);
> + bool validateStreamType(uint32_t type);
> + bool validateMemoryType(uint32_t memory);
> + void setFmtFromConfig();
> + void querycap(std::shared_ptr<Camera> camera);
> +
> + 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);
> +
> + unsigned int index_;
> +
> + struct v4l2_format curV4L2Format_;
> + StreamConfiguration streamConfig_;
> + struct v4l2_capability capabilities_;
> + unsigned int bufferCount_;
> + unsigned int currentBuf_;
> +
> + std::unique_ptr<V4L2Camera> vcam_;
> +};
> +
> +#endif /* __V4L2_CAMERA_PROXY_H__ */
> 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
> + */
> +
> +#include "v4l2_compat_manager.h"
> +
> +#include <iostream>
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/videodev2.h>
> +#include <stdarg.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +
> +#define LIBCAMERA_PUBLIC __attribute__((visibility("default")))
Am I wrong or this makes sense only if we compile with
-fvisiblity=hidden ?
https://gcc.gnu.org/wiki/Visibility
I welcome this change, but then probably you should add that
compilation flag to the v4l2 compat library, it I have not
mis-interpreted the above wiki page
> +
> +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" {
> +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);
> +}
> +
> +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;
> +}
> +
> +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..4eeb714f
> --- /dev/null
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -0,0 +1,353 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * v4l2_compat_manager.cpp - V4L2 compatibility manager
> + */
> +#include "v4l2_compat_manager.h"
> +
> +#include <dlfcn.h>
> +#include <fcntl.h>
> +#include <iostream>
> +#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()
> + : cameraManagerRunning_(false), 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 seem to be mixing cameraCase and snake_case here and there in
variable declaration. Was this intentional ?
> +}
> +
> +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);
> +
> + 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.
> + */
> + // \todo map v4l2 devices to libcamera cameras; minor device node?
> + 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.
> + */
> + cv_.notify_one();
> +
> + /* Now start processing events and messages. */
> + exec();
> +
> + cm_->stop();
> + delete cm_;
> + cm_ = nullptr;
> +}
> +
> +V4L2CompatManager *V4L2CompatManager::instance()
> +{
> + static V4L2CompatManager v4l2CompatManager;
> + return &v4l2CompatManager;
> +}
> +
> +bool V4L2CompatManager::validfd(int fd)
> +{
> + return devices_.find(fd) != devices_.end();
> +}
> +
> +bool V4L2CompatManager::validmmap(void *addr)
> +{
> + return mmaps_.find(addr) != mmaps_.end();
> +}
> +
> +std::shared_ptr<V4L2CameraProxy> V4L2CompatManager::getCamera(int fd)
> +{
> + if (!validfd(fd))
> + return nullptr;
> +
> + return devices_.at(fd);
This is safe, but it's a double lookup, same as below. This is minor
indeed, but probably using the iterator returned from find() directly
is slightly more efficient.
Then you can inline the valid[fd|mmap}() methods, probably
> +}
> +
> +std::shared_ptr<V4L2CameraProxy> V4L2CompatManager::getCamera(void *addr)
> +{
> + if (!validmmap(addr))
> + return nullptr;
> +
> + return devices_.at(mmaps_.at(addr));
> +}
> +
> +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;
> +
> + if (!isRunning())
> + init();
> +
> + /* \todo map v4l2 devnodes to libcamera cameras */
> + unsigned int camera_index = 0;
> +
> + std::shared_ptr<V4L2CameraProxy> proxy = proxies_[camera_index];
> + int 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 errno;
I'm not sure I get this usage of the file descriptor returned by
eventfd...
It is used as index in the map, possibly replaced by dup(). I would
have expected the fd returned by openat() to be used as index in the
map... What am I missing ?
> +
> + devices_.emplace(efd, proxy);
> +
> + return efd;
> +}
> +
> +int V4L2CompatManager::dup(int oldfd)
> +{
> + int newfd = dup_func_(oldfd);
> + if (validfd(oldfd))
Shouldn't you return an error if the fd to duplicate is not valid, ad
dup() only if it is ?
> + devices_[newfd] = devices_[oldfd];
> +
> + return newfd;
> +}
> +
> +int V4L2CompatManager::close(int fd)
> +{
> + if (validfd(fd) && devices_[fd]->close() < 0)
I would return -EIO if !validfd() and propagate the error up if
close() fails.
> + return -EIO;
> +
> + return close_func_(fd);
> +}
> +
> +void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
> + int fd, off_t offset)
> +{
> + if (!validfd(fd))
> + return mmap_func_(addr, length, prot, flags, fd, offset);
> +
> + void *map = getCamera(fd)->mmap(addr, length, prot, flags, offset);
> + if (map == MAP_FAILED) {
> + errno = EINVAL;
> + return map;
> + }
> +
> + mmaps_[addr] = fd;
> + return map;
> +}
> +
> +int V4L2CompatManager::munmap(void *addr, size_t length)
> +{
> + if (!validmmap(addr))
> + return munmap_func_(addr, length);
> +
> + int ret = getCamera(addr)->munmap(addr, length);
> + if (ret < 0)
> + return ret;
> +
> + mmaps_.erase(addr);
> + addr = nullptr;
> +
> + return 0;
> +}
> +
> +int V4L2CompatManager::ioctl(int fd, unsigned long request, void *arg)
> +{
> + std::shared_ptr<V4L2CameraProxy> proxy = getCamera(fd);
> + if (!proxy)
> + return ioctl_func_(fd, request, arg);
> +
What is the use case for bypassing the proxy ? That might be my
limited knowledge of how v4l2 application behave
> + return proxy->ioctl(request, arg);
> +}
> +
> +/* \todo make libcamera export these */
mmm, V4L2VideoDevice has very similar format conversion routines... we
should really centralize them somehow.. maybe not in scope for this
patch, but replicating cose which is tricky to get right due to the
different format definition between DRM and V4L2 is not a good idea.
> +int V4L2CompatManager::bpl_multiplier(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::image_size(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::v4l2_to_drm(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::drm_to_v4l2(unsigned int pixelformat)
> +{
> + switch (pixelformat) {
> + /* RGB formats. */
> + case DRM_FORMAT_BGR888:
> + return V4L2_PIX_FMT_BGR24;
This in example does not match the one we have in V4L2Videodevice.
DRM_FORMAT_BGR24 = V4L2_PIX_FMT_RGB24 not BGR24.
I know, it's a pain.
I think the other version is correct, as we validated them using
conversion routines from kernel drivers...
There might be more below (and possibly above)
> + case DRM_FORMAT_RGB888:
> + return V4L2_PIX_FMT_RGB24;
> + 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..492f97fd
> --- /dev/null
> +++ b/src/v4l2/v4l2_compat_manager.h
> @@ -0,0 +1,91 @@
> +/* 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>
> +#include <map>
> +#include <mutex>
> +#include <queue>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +#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);
> +
> + std::shared_ptr<V4L2CameraProxy> getCamera(int fd);
> + std::shared_ptr<V4L2CameraProxy> getCamera(void *addr);
Every time you return a shared_ptr by value, you increase the
reference count for no good reason. I would either return by reference
or return V4L2CameraProxy and use shared pointers only in the proxies_
array and not in the rest of the code base.
Overall this is a very good first submission and I'm happy to see it
posted to the list \o/
I admit in this first pass I didn't go into extreme detail on the way
the v4l2 operations semantic is handled, but it seems sane to me!
Thanks
j
> +
> + 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 bpl_multiplier(unsigned int format);
> + static int image_size(unsigned int format, unsigned int width,
> + unsigned int height);
> +
> + static unsigned int v4l2_to_drm(unsigned int pixelformat);
> + static unsigned int drm_to_v4l2(unsigned int pixelformat);
> +
> +private:
> + V4L2CompatManager();
> + ~V4L2CompatManager();
> +
> + void run() override;
> +
> + bool validfd(int fd);
> + bool validmmap(void *addr);
> +
> + int default_ioctl(int fd, unsigned long request, void *arg);
> +
> + 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_;
> +
> + bool cameraManagerRunning_;
> + 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__ */
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191206/02b380eb/attachment-0001.sig>
More information about the libcamera-devel
mailing list