[libcamera-devel] [PATCH 14/14] libcamera: v4l2: Use Object::invokeMethod() return value
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Jan 7 20:25:04 CET 2020
Hi Laurent,
Thank for your work.
On 2020-01-04 07:09:47 +0200, Laurent Pinchart wrote:
> Now that Object::invokeMethod() supports returning a value, use it and
> drop the return value method argument.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Looks much nicer,
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/v4l2/v4l2_camera.cpp | 109 +++++++++++++++------------------
> src/v4l2/v4l2_camera.h | 18 +++---
> src/v4l2/v4l2_camera_proxy.cpp | 51 ++++++++-------
> 3 files changed, 82 insertions(+), 96 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 3590730fde08..4545483cf1c7 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -34,23 +34,21 @@ V4L2Camera::~V4L2Camera()
> camera_->release();
> }
>
> -void V4L2Camera::open(int *ret)
> +int V4L2Camera::open()
> {
> /* \todo Support multiple open. */
> if (camera_->acquire() < 0) {
> LOG(V4L2Compat, Error) << "Failed to acquire camera";
> - *ret = -EINVAL;
> - return;
> + return -EINVAL;
> }
>
> config_ = camera_->generateConfiguration({ StreamRole::Viewfinder });
> if (!config_) {
> camera_->release();
> - *ret = -EINVAL;
> - return;
> + return -EINVAL;
> }
>
> - *ret = 0;
> + return 0;
> }
>
> void V4L2Camera::close()
> @@ -92,9 +90,9 @@ void V4L2Camera::requestComplete(Request *request)
> bufferSema_.release();
> }
>
> -void V4L2Camera::configure(int *ret, StreamConfiguration *streamConfigOut,
> - const Size &size, PixelFormat pixelformat,
> - unsigned int bufferCount)
> +int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
> + const Size &size, PixelFormat pixelformat,
> + unsigned int bufferCount)
> {
> StreamConfiguration &streamConfig = config_->at(0);
> streamConfig.size.width = size.width;
> @@ -106,8 +104,7 @@ void V4L2Camera::configure(int *ret, StreamConfiguration *streamConfigOut,
> CameraConfiguration::Status validation = config_->validate();
> if (validation == CameraConfiguration::Invalid) {
> LOG(V4L2Compat, Debug) << "Configuration invalid";
> - *ret = -EINVAL;
> - return;
> + return -EINVAL;
> }
> if (validation == CameraConfiguration::Adjusted)
> LOG(V4L2Compat, Debug) << "Configuration adjusted";
> @@ -115,24 +112,25 @@ void V4L2Camera::configure(int *ret, StreamConfiguration *streamConfigOut,
> LOG(V4L2Compat, Debug) << "Validated configuration is: "
> << streamConfig.toString();
>
> - *ret = camera_->configure(config_.get());
> - if (*ret < 0)
> - return;
> + int ret = camera_->configure(config_.get());
> + if (ret < 0)
> + return ret;
>
> *streamConfigOut = config_->at(0);
> +
> + return 0;
> }
>
> -void V4L2Camera::mmap(void **ret, unsigned int index)
> +void *V4L2Camera::mmap(unsigned int index)
> {
> Stream *stream = *camera_->streams().begin();
> - *ret = stream->buffers()[index].planes()[0].mem();
> + return stream->buffers()[index].planes()[0].mem();
> }
>
> -void V4L2Camera::allocBuffers(int *ret, unsigned int count)
> +int V4L2Camera::allocBuffers(unsigned int count)
> {
> - *ret = camera_->allocateBuffers();
> - if (*ret == -EACCES)
> - *ret = -EBUSY;
> + int ret = camera_->allocateBuffers();
> + return ret == -EACCES ? -EBUSY : ret;
> }
>
> void V4L2Camera::freeBuffers()
> @@ -140,85 +138,76 @@ void V4L2Camera::freeBuffers()
> camera_->freeBuffers();
> }
>
> -void V4L2Camera::streamOn(int *ret)
> +int V4L2Camera::streamOn()
> {
> - *ret = 0;
> -
> if (isRunning_)
> - return;
> + return 0;
> +
> + int ret = camera_->start();
> + if (ret < 0)
> + return ret == -EACCES ? -EBUSY : ret;
>
> - *ret = camera_->start();
> - if (*ret < 0) {
> - if (*ret == -EACCES)
> - *ret = -EBUSY;
> - return;
> - }
> isRunning_ = true;
>
> for (std::unique_ptr<Request> &req : pendingRequests_) {
> /* \todo What should we do if this returns -EINVAL? */
> - *ret = camera_->queueRequest(req.release());
> - if (*ret < 0) {
> - if (*ret == -EACCES)
> - *ret = -EBUSY;
> - return;
> - }
> + ret = camera_->queueRequest(req.release());
> + if (ret < 0)
> + return ret == -EACCES ? -EBUSY : ret;
> }
>
> pendingRequests_.clear();
> +
> + return 0;
> }
>
> -void V4L2Camera::streamOff(int *ret)
> +int V4L2Camera::streamOff()
> {
> - *ret = 0;
> -
> /* \todo Restore buffers to reqbufs state? */
> if (!isRunning_)
> - return;
> + return 0;
> +
> + int ret = camera_->stop();
> + if (ret < 0)
> + return ret == -EACCES ? -EBUSY : ret;
>
> - *ret = camera_->stop();
> - if (*ret < 0) {
> - if (*ret == -EACCES)
> - *ret = -EBUSY;
> - return;
> - }
> isRunning_ = false;
> +
> + return 0;
> }
>
> -void V4L2Camera::qbuf(int *ret, unsigned int index)
> +int V4L2Camera::qbuf(unsigned int index)
> {
> Stream *stream = config_->at(0).stream();
> std::unique_ptr<Buffer> buffer = stream->createBuffer(index);
> if (!buffer) {
> LOG(V4L2Compat, Error) << "Can't create buffer";
> - *ret = -ENOMEM;
> - return;
> + return -ENOMEM;
> }
>
> std::unique_ptr<Request> request =
> std::unique_ptr<Request>(camera_->createRequest());
> if (!request) {
> LOG(V4L2Compat, Error) << "Can't create request";
> - *ret = -ENOMEM;
> - return;
> + return -ENOMEM;
> }
>
> - *ret = request->addBuffer(std::move(buffer));
> - if (*ret < 0) {
> + int ret = request->addBuffer(std::move(buffer));
> + if (ret < 0) {
> LOG(V4L2Compat, Error) << "Can't set buffer for request";
> - *ret = -ENOMEM;
> - return;
> + return -ENOMEM;
> }
>
> if (!isRunning_) {
> pendingRequests_.push_back(std::move(request));
> - return;
> + return 0;
> }
>
> - *ret = camera_->queueRequest(request.release());
> - if (*ret < 0) {
> + ret = camera_->queueRequest(request.release());
> + if (ret < 0) {
> LOG(V4L2Compat, Error) << "Can't queue request";
> - if (*ret == -EACCES)
> - *ret = -EBUSY;
> + return ret == -EACCES ? -EBUSY : ret;
> }
> +
> + return 0;
> }
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index d24dbca6aaf4..5a889efdb4a2 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -48,23 +48,23 @@ public:
> V4L2Camera(std::shared_ptr<Camera> camera);
> ~V4L2Camera();
>
> - void open(int *ret);
> + int open();
> void close();
> void getStreamConfig(StreamConfiguration *streamConfig);
> std::vector<FrameMetadata> completedBuffers();
>
> - void mmap(void **ret, unsigned int index);
> + void *mmap(unsigned int index);
>
> - void configure(int *ret, StreamConfiguration *streamConfigOut,
> - const Size &size, PixelFormat pixelformat,
> - unsigned int bufferCount);
> + int configure(StreamConfiguration *streamConfigOut,
> + const Size &size, PixelFormat pixelformat,
> + unsigned int bufferCount);
>
> - void allocBuffers(int *ret, unsigned int count);
> + int allocBuffers(unsigned int count);
> void freeBuffers();
> - void streamOn(int *ret);
> - void streamOff(int *ret);
> + int streamOn();
> + int streamOff();
>
> - void qbuf(int *ret, unsigned int index);
> + int qbuf(unsigned int index);
>
> Semaphore bufferSema_;
>
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 559caba69270..2eeb12396d90 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -40,9 +40,8 @@ int V4L2CameraProxy::open(bool nonBlocking)
> {
> LOG(V4L2Compat, Debug) << "Servicing open";
>
> - int ret;
> - vcam_->invokeMethod(&V4L2Camera::open, ConnectionTypeBlocking,
> - &ret);
> + int ret = vcam_->invokeMethod(&V4L2Camera::open,
> + ConnectionTypeBlocking);
> if (ret < 0) {
> errno = -ret;
> return -1;
> @@ -91,9 +90,8 @@ void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)
> return MAP_FAILED;
> }
>
> - void *val;
> - vcam_->invokeMethod(&V4L2Camera::mmap, ConnectionTypeBlocking,
> - &val, index);
> + void *val = vcam_->invokeMethod(&V4L2Camera::mmap,
> + ConnectionTypeBlocking, index);
>
> buffers_[index].flags |= V4L2_BUF_FLAG_MAPPED;
> mmaps_[val] = index;
> @@ -245,9 +243,11 @@ int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg)
> return ret;
>
> Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> - vcam_->invokeMethod(&V4L2Camera::configure, ConnectionTypeBlocking,
> - &ret, &streamConfig_, size,
> - v4l2ToDrm(arg->fmt.pix.pixelformat), bufferCount_);
> + ret = vcam_->invokeMethod(&V4L2Camera::configure,
> + ConnectionTypeBlocking,
> + &streamConfig_, size,
> + v4l2ToDrm(arg->fmt.pix.pixelformat),
> + bufferCount_);
> if (ret < 0)
> return -EINVAL;
>
> @@ -297,9 +297,8 @@ int V4L2CameraProxy::freeBuffers()
> {
> LOG(V4L2Compat, Debug) << "Freeing libcamera bufs";
>
> - int ret;
> - vcam_->invokeMethod(&V4L2Camera::streamOff,
> - ConnectionTypeBlocking, &ret);
> + int ret = vcam_->invokeMethod(&V4L2Camera::streamOff,
> + ConnectionTypeBlocking);
> if (ret < 0) {
> LOG(V4L2Compat, Error) << "Failed to stop stream";
> return ret;
> @@ -327,10 +326,11 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)
> return freeBuffers();
>
> Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
> - vcam_->invokeMethod(&V4L2Camera::configure, ConnectionTypeBlocking,
> - &ret, &streamConfig_, size,
> - v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),
> - arg->count);
> + ret = vcam_->invokeMethod(&V4L2Camera::configure,
> + ConnectionTypeBlocking,
> + &streamConfig_, size,
> + v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),
> + arg->count);
> if (ret < 0)
> return -EINVAL;
>
> @@ -343,8 +343,8 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)
> arg->count = streamConfig_.bufferCount;
> bufferCount_ = arg->count;
>
> - vcam_->invokeMethod(&V4L2Camera::allocBuffers,
> - ConnectionTypeBlocking, &ret, arg->count);
> + ret = vcam_->invokeMethod(&V4L2Camera::allocBuffers,
> + ConnectionTypeBlocking, arg->count);
> if (ret < 0) {
> arg->count = 0;
> return ret;
> @@ -392,9 +392,8 @@ int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg)
> arg->index >= bufferCount_)
> return -EINVAL;
>
> - int ret;
> - vcam_->invokeMethod(&V4L2Camera::qbuf, ConnectionTypeBlocking,
> - &ret, arg->index);
> + int ret = vcam_->invokeMethod(&V4L2Camera::qbuf, ConnectionTypeBlocking,
> + arg->index);
> if (ret < 0)
> return ret;
>
> @@ -437,9 +436,8 @@ int V4L2CameraProxy::vidioc_streamon(int *arg)
> if (!validateBufferType(*arg))
> return -EINVAL;
>
> - int ret;
> - vcam_->invokeMethod(&V4L2Camera::streamOn,
> - ConnectionTypeBlocking, &ret);
> + int ret = vcam_->invokeMethod(&V4L2Camera::streamOn,
> + ConnectionTypeBlocking);
>
> return ret;
> }
> @@ -451,9 +449,8 @@ int V4L2CameraProxy::vidioc_streamoff(int *arg)
> if (!validateBufferType(*arg))
> return -EINVAL;
>
> - int ret;
> - vcam_->invokeMethod(&V4L2Camera::streamOff,
> - ConnectionTypeBlocking, &ret);
> + int ret = vcam_->invokeMethod(&V4L2Camera::streamOff,
> + ConnectionTypeBlocking);
>
> for (struct v4l2_buffer &buf : buffers_)
> buf.flags &= ~(V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_DONE);
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list